Toggle switch styles

[camertron]

What are you trying to accomplish?

The DI team is currently working on accessible forms in dotcom. Part of this work is creating a new toggle switch component that functions as a replacement for checkboxes.

toggle_switch.mov

What approach did you choose and why?

I tried to do as much of the styling in CSS as possible to keep the markup and Javascript as minimal as possible.

What should reviewers focus on?

IANACE (I am not a CSS expert), so I would really appreciate some feedback on the structure and organization of the CSS styles. For example, this is the first time I've used BEM and I'm not sure I got some of the naming correct.

Can these changes ship as is?

Yes, these changes can be shipped as-is. The corresponding component in dotcom uses these styles, but we'll need to ship this change before I can submit the dotcom PR. Eventually we'll be upstreaming the component into primer/view_component.s

• Yes, this PR does not depend on additional changes. 🚢

[changeset-bot]

🦋 Changeset detected

Latest commit: d537c3d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/css	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[camertron]

@vdepizzol I'm not sure what to do about the failing lint check and nesting level depth. There's probably a better way to organize things?

[simurai]

Re. BEM... it mostly looks good. One thing we might wanna change are the "child elements" like ToggleSwitch--switch, ToggleSwitch--knob, ToggleSwitch--lineIcon, .ToggleSwitch--circleIcon, ToggleSwitch--onLabel, etc. they should only have a single dash -:

• ToggleSwitch--switch -> ToggleSwitch-switch

The double dash -- is used for modifiers, variants or states. E.g. .ToggleSwitch--checked ✅ .

---

Also, what does the typical markup look like? And should we add an example to the docs or Storybook to be able to test it?

Based on the anatomy of the interface guidelines, changing .ToggleSwitch--switch to .ToggleSwitch-track might fit better? But I would have to the see the markup to know if it makes more sense. 😅

[camertron]

Thanks @simurai, I've made the BEM changes you suggested and added a playground story :)

[simurai]

added a playground story

Nice.. thanks. I took a look at Storybook. Here some feedback about the disabled state and a few other things based on the markup:

• Should the ToggleSwitch-labels text also look disabled? We could use this color: --color-primer-fg-disabled.
• The checked knob looks still a bit strong for the disabled state. But it uses --color-switch-knob-checked-disabled-bg, so maybe that's intended.
  

Also cc @mperrotti for another 👀

[mperrotti]

This looks great! Excellent job reproducing all the little details from the PRC implementation.

Will approve once the minor feedback I had is addressed.

[langermank]

Looking good @camertron! I left some suggestions 😄