Issue 6514 - IOToggle Custom Element

css/io-toggle.scss

-io-toggle {

[Thomas Greiner, 2018/04/25 17:29:14]

Coding style: "Opening braces always go on their own line."

[a.giammarchi, 2018/04/26 10:44:51]

have we published this eyeo style thing somewhere so I can fix this stuff
automatically?

[Thomas Greiner, 2018/04/26 17:00:52]

Yep, two weeks ago we've published the stylelint-config-eyeo package on npm so
we are finally able to use that to lint our CSS code.

+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-present eyeo GmbH
+ *
+ * Adblock Plus is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Adblock Plus is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+io-toggle
+{
   --width: 30px;

[saroyanm, 2018/04/25 17:05:01]

While we are using SASS, wouldn't that make more sense to use SASS variables
instead of the CSS variables ?
SCSS variables are still powerful I think because we can still define them in
the separate file and reuse them in modules.

If you folks agree, maybe we can switch to using SASS variables instead ?

[a.giammarchi, 2018/04/26 10:44:51]

I think the whole point of dropping Edge 14 was to be able to use native CSS
variables.

These variables can be manipulated, if needed, by JS, while SASS variables
cannot.

However, I agree if/when we have the need to share immutable variables across
components we can prefer SASS variables over CSS variables.

Forcing all variables to be SASS variables seems unnecessary seppuku for the
future of the UI.

[Thomas Greiner, 2018/04/26 17:00:51]

I'd argue that we want to rely on web standards as much as possible to avoid any
unnecessary "magic" in between the developer and the user.

That being said, SASS has the benefit of allowing us to generate CSS files at
build time which is useful for modularization. Furthermore, if SASS supports a
feature we'd expect to become part of the standard, we could also use it as a
polyfill.

[a.giammarchi, 2018/04/27 10:59:04]

this doesn't address the fact CSS variables are already out, supported, and the
standard, and at this point more useful/powerful than SASS variables.

We can use CSS variables also in a file a part, configured under `:root { ... }`
to share colors.

If we are pursuing standards, CSS variables are already available to our UI and
are more powerful, 'cause accessible through JS.

This is very important since we started seeing UI implementing animations, where
these performs always better through CSS rather than JS.

Accordingly, if we have to chose between standards and SASS variables *only*,
then I'd vote standard, 'cause I don't see any advantage in SASS since we don't
need pre processing anything with just CSS.

Actually, once we have a way to import CSS per page, if we ever need that, we
won't need SASS at all for the kind of style we usually write.

[Thomas Greiner, 2018/05/02 11:53:28]

That's also what I'm saying that we should always prefer web standards over SASS
and I also agree that as soon as we don't need SASS anymore that we should drop
it.

   --height: 8px;
   --translateY: -4px;
   --translateX: 14px;
   display: inline-block;
   width: var(--width);
   height: var(--height);
   border-radius: 4px;
   background-color: #9b9b9b;

[saroyanm, 2018/04/25 17:05:01]

Having colors set separately will make it easy to keep it in sync with the
specifications I think ->
https://gitlab.com/eyeo/specs/spec/blob/master/spec/abp/options-page-style-gu...


We might want to update the colors and even better separate components in the
specifications and apply new colors defined here if they are already agreed with
Jeen.

[a.giammarchi, 2018/04/26 10:44:51]

Yeah. Colors might be one of those immutable shared variables, but we have to
agree on where/how to use them across all UI.

   transition: background .2s ease-out;

[Thomas Greiner, 2018/04/25 17:29:13]

Technically, we only expect "background-color" to change so we could restrict
"transition" and "will-change" to refer to "background-color".

[Thomas Greiner, 2018/04/25 17:29:13]

Coding style: "Don't omit the optional leading 0 for decimal numbers."

[a.giammarchi, 2018/04/26 10:44:51]

AFAIK that doesn't work. will change wants background, same as margin, or
padding.

[Thomas Greiner, 2018/04/26 17:00:51]

Interesting. I wasn't aware of that. No problem with keeping it as is then.

   will-change: background;
   cursor: pointer;
 }
 
-io-toggle[checked] {
+io-toggle[checked]
+{
   background-color: #92d3ea;

[Thomas Greiner, 2018/04/25 17:29:13]

How did you choose the colors? At least this one isn't listed at
https://gitlab.com/eyeo/specs/spec/blob/master/spec/abp/options-page-style-gu....

[a.giammarchi, 2018/04/26 10:44:51]

I have downloaded specs images and picked color through GIMP color picker.

[Thomas Greiner, 2018/04/26 17:00:51]

Since the part of the spec I referred to only applies to the options page, I'd
be fine with the colors you chose but suggest to quickly check back with Jeen.

[a.giammarchi, 2018/04/27 10:59:04]

AFAIK Jeen already approved this component and wants it all over our UI :-)

[Thomas Greiner, 2018/05/02 11:53:27]

Acknowledged.

 }
 
-io-toggle[disabled] {
+io-toggle[disabled]
+{
   opacity: 0.5;
   cursor: default;
 }
 
-io-toggle button {
+io-toggle button
+{
   width: calc(var(--height) * 2);
   height: calc(var(--height) * 2);
   padding: 0;
   border: 2px solid #e1e0e1;
   border-radius: var(--height);
   transition: border .2s ease-out, box-shadow .2s ease-out,
               transform .2s ease-out, width .2s ease-out;
   will-change: border, box-shadow, transform, width;
   transform: translateY(var(--translateY));
   outline: none;
@@ skipping 23 matching lines @@
 {
   box-shadow: 0 2px 4px 0 #a6cede;
 }
 
 io-toggle button:focus,
 io-toggle button[aria-checked="true"]:focus
 {
   border: 2px solid #87bffe;
 }
 
-/* in case we will need to switch active/disabled direction

[Thomas Greiner, 2018/04/25 17:29:13]

I was also wondering about that and it appears that the direction should indeed
be switched for RTL locales. Although I didn't find any authoritative sources
for that, only those references:

- https://bugzilla.mozilla.org/show_bug.cgi?id=1414985
-
https://docs.oracle.com/cd/E87544_01/pt856pbr1/eng/pt/tgbl/task_WorkingWithBi...
- https://rtl-css.net/tutorial/quick-tip-rtling-css3-box-shadow-property

[a.giammarchi, 2018/04/26 10:44:50]

it took me a little while to figure this out, so I've preferred leaving it as
comment to eventually make active whenever we'll decide to do this.

[Thomas Greiner, 2018/04/26 17:00:52]

For some reason I thought you were talking about the direction of `box-shadow`
here because I initially intended to comment on that above.

But looking at it again you're referring to the toggle direction in general.
https://ux.stackexchange.com/questions/113633/should-a-toggle-switchs-label-b...
is the best discussion I found about it which suggests to indeed switch the
toggle direction.
In any case it shouldn't harm to double-check with Jeen and update the spec to
reflect such decisions.

Besides that, if this weren't the case, our coding style states: "Don't comment
code out, delete it."

[a.giammarchi, 2018/04/27 10:59:04]

I'll double check with Jeen before eventually removing this bit, thanks.

[Thomas Greiner, 2018/05/02 11:53:27]

Acknowledged.

 body[dir="rtl"] io-toggle button[aria-checked="true"]
 {
   transform: translateY(var(--translateY)) translateX(calc(var(--translateX) * -1));
 }
-//*/