Add ToggleSwitch component #1933

mperrotti · 2022-03-08T01:37:47Z

Adds a toggle switch for turning settings on and off and immediately save the change.

Closes https://github.com/github/primer/issues/690

Screenshots

Merge checklist

Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.

changeset-bot · 2022-03-08T01:37:49Z

🦋 Changeset detected

Latest commit: bb78093

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

This PR includes changesets to release 1 package

Name	Type
@primer/react	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

github-actions · 2022-03-08T01:40:29Z

size-limit report 📦

Path	Size
dist/browser.esm.js	65.06 KB (+2.3% 🔺)
dist/browser.umd.js	65.41 KB (+2.29% 🔺)

…/switch-component

mperrotti · 2022-03-09T02:02:20Z

What do we think of calling it ToggleSwitch instead of Switch? 🤔

docs/content/Switch.mdx

colebemis · 2022-03-11T18:19:40Z

docs/content/Switch.mdx

+  />
+  <PropsTableRow name="defaultOn" type="boolean" description="Uncontrolled - whether the switch is turned on" />
+  <PropsTableRow name="disabled" type="boolean" description="Whether the switch is ready for user input" />
+  <PropsTableRow name="isLoading" type="boolean" description="Whether the switch is ready for user input" />


Can we change this to loading?

Suggested change

<PropsTableRow name="isLoading" type="boolean" description="Whether the switch is ready for user input" />

<PropsTableRow name="loading" type="boolean" description="Whether the switch is ready for user input" />

Also, is it intentional that the description is the same as the disabled prop?

Not intentional. Thanks for flagging that.

docs/content/Switch.mdx

src/Switch.tsx

colebemis · 2022-03-11T18:36:20Z

src/Switch.tsx

+      display="inline-flex"
+      alignItems="center"
+      flexDirection={statusLabelPosition === 'start' ? 'row' : 'row-reverse'}
+      mb={4}


Why does this component have a margin bottom?

I have no idea where this came from. Removing.

colebemis · 2022-03-11T18:38:45Z

docs/content/Switch.mdx

+  <PropsTableRow name="defaultOn" type="boolean" description="Uncontrolled - whether the switch is turned on" />
+  <PropsTableRow name="disabled" type="boolean" description="Whether the switch is ready for user input" />
+  <PropsTableRow name="isLoading" type="boolean" description="Whether the switch is ready for user input" />
+  <PropsTableRow name="on" type="boolean" description="Whether the switch is turned on" />


Should we call this prop checked instead of on to align with the <input type="checkbox"> API?

I'm good with that.

colebemis · 2022-03-11T18:39:41Z

src/Switch.tsx

+  }, [onChange, onProp, isControlled])
+
+  return (
+    <Box


Should this component render a visually hidden checkbox in case people plan to use this inside a form?

We intentionally used a button with role="switch" because this component should not be used in a form. The accessibility team doesn't want checkbox changes to be saved unless the user explicitly clicks a "save" or "submit" button.

cc @alliethu or @ichelsea to keep me correct

Ah okay. Should we mention that in the docs?

<button role=switch> sounds like the right thing to do.

Sure, I'll include something in the docs.

@colebemis - I'm not sure where in the React docs this should go. Could I add a new heading for accessibility?

src/Switch.tsx

docs/content/Switch.mdx

src/Switch.tsx

Co-authored-by: Cole Bemis <colebemis@github.com>

colebemis

Looks good! 👍

Co-authored-by: Cole Bemis <colebemis@github.com>

mperrotti · 2022-03-21T20:28:03Z

~~@colebemis - could you also have a look at primer/primitives#309 ?~~

~~It blocks this PR~~

Update: it's merged

mperrotti · 2022-03-21T20:40:53Z

Help wanted: the themePreval snapshot test is failing, but I don't understand why.

simurai · 2022-03-22T06:20:48Z

the themePreval snapshot test is failing, but I don't understand why.

Ohh.. I think the same themePreval error happened to me too: #1961 (comment). You could try:

Update the date here
Then run test:update

react/package.json

Line 46 in 1bfb128

"test:update": "npm run test -- --updateSnapshot",
Commit changes

Not quite sure why this is needed, but I assume something with re-generating some "snapshot" so it can be compared? This probably should be documented if it's needed every time primer/primitives is updated.

mperrotti · 2022-03-23T00:08:04Z

A PR has also been opened for documenting interface guidelines: primer/design#249

mperrotti · 2022-03-23T00:14:43Z

@simurai - that doesn't seem to have worked 🤔

Could you check my last commit and let me know if I did something wrong?

I updated the snapshots, but themePreval won't update. I even tried running rm -rf node_modules && npm i just to be sure.

simurai · 2022-03-24T04:33:34Z

@mperrotti that doesn't seem to have worked

Ahh, yess.. sorry. I missed the build step. So it would be:

Update the date in src/theme-preval.js
Run build

react/package.json

Line 38 in 1bfb128

"build": "./script/build",
Run test:update

react/package.json

Line 46 in 1bfb128

"test:update": "npm run test -- --updateSnapshot",
Commit changes

Here the commit fa9ff75 that seems to now pass CI (Visual testing still needs an approval).

Something weird in fa9ff75 are the changes to package-lock.json. Those changes were made by simply opening the mp/switch-component branch in a Codespace. So I thought maybe ok to commit that too. 🤷

mperrotti · 2022-03-24T17:17:31Z

Thanks for fixing that @simurai !

mperrotti added 4 commits March 7, 2022 18:01

adds Switch component and stories

81d5b63

adds tests and more ARIA markup

f02595e

adds component docs and fixes transition

872bdb4

adds changeset

0316ed3

mperrotti requested review from a team and colebemis March 8, 2022 01:37

Merge branch 'main' into mp/switch-component

27dccc7

github-actions bot deployed to Preview March 8, 2022 14:50 View deployment

mperrotti added 2 commits March 8, 2022 09:59

update snapshots

ca8950c

Merge branch 'mp/switch-component' of github.com:primer/react into mp…

108b40f

…/switch-component

github-actions bot deployed to Preview March 8, 2022 15:01 View deployment

mperrotti added 2 commits March 8, 2022 16:33

Merge branch 'main' of github.com:primer/react into mp/switch-component

40ff806

addresses component design feedback

cebfa1b

github-actions bot deployed to Preview March 9, 2022 02:01 View deployment

mperrotti added 2 commits March 10, 2022 14:29

improves docs and storybook examples

176b5b8

Merge branch 'main' of github.com:primer/react into mp/switch-component

d653469

github-actions bot deployed to Preview March 10, 2022 19:29 View deployment

updates snaps

30612f9

github-actions bot deployed to Preview March 10, 2022 19:41 View deployment

Merge branch 'main' into mp/switch-component

b475d8a

github-actions bot deployed to Preview March 11, 2022 16:00 View deployment

colebemis reviewed Mar 11, 2022

View reviewed changes

docs/content/Switch.mdx Outdated Show resolved Hide resolved

colebemis reviewed Mar 11, 2022

View reviewed changes

src/Switch.tsx Outdated Show resolved Hide resolved

Update src/Switch.tsx

1d52067

Co-authored-by: Cole Bemis <colebemis@github.com>

github-actions bot deployed to Preview March 11, 2022 19:52 View deployment

Update docs/content/Switch.mdx

37ed9ae

Co-authored-by: Cole Bemis <colebemis@github.com>

colebemis approved these changes Mar 21, 2022

View reviewed changes

mperrotti and others added 2 commits March 21, 2022 16:14

Update docs/content/ToggleSwitch.mdx

d3022e4

Co-authored-by: Cole Bemis <colebemis@github.com>

Update docs/content/ToggleSwitch.mdx

97cea00

Co-authored-by: Cole Bemis <colebemis@github.com>

github-actions bot deployed to Preview March 21, 2022 20:14 View deployment

Merge branch 'main' of github.com:primer/react into mp/switch-component

09b735e

updates themePreval snapshot

7afe8e3

github-actions bot deployed to Preview March 21, 2022 20:30 View deployment

mperrotti added 2 commits March 22, 2022 19:54

Merge branch 'main' of github.com:primer/react into mp/switch-component

2871849

upgrades primitives, updates snapshots

5aee49d

github-actions bot deployed to Preview March 23, 2022 00:03 View deployment

mperrotti requested a review from ichelsea March 23, 2022 00:04

Update snapshot

fa9ff75

simurai temporarily deployed to visual-testing March 24, 2022 03:46 Inactive

github-actions bot deployed to Preview March 24, 2022 03:46 View deployment

Merge branch 'main' into mp/switch-component

bb78093

github-actions bot deployed to Preview March 24, 2022 17:28 View deployment

mperrotti temporarily deployed to visual-testing March 24, 2022 17:35 Inactive

mperrotti requested a review from colebemis March 24, 2022 18:23

mperrotti merged commit ae7650f into main Mar 24, 2022

mperrotti deleted the mp/switch-component branch March 24, 2022 19:14

primer-css mentioned this pull request Mar 24, 2022

Release tracking #1993

Merged

bolinfest mentioned this pull request May 16, 2022

Remove label for ToggleSwitch, breaks i18n #2078

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add ToggleSwitch component #1933

Add ToggleSwitch component #1933

mperrotti commented Mar 8, 2022

changeset-bot bot commented Mar 8, 2022 •

edited

Loading

github-actions bot commented Mar 8, 2022 •

edited

Loading

mperrotti commented Mar 9, 2022

colebemis Mar 11, 2022

mperrotti Mar 11, 2022

colebemis Mar 11, 2022

mperrotti Mar 11, 2022

colebemis Mar 11, 2022

mperrotti Mar 11, 2022

colebemis Mar 11, 2022

mperrotti Mar 11, 2022

colebemis Mar 11, 2022

ichelsea Mar 14, 2022

mperrotti Mar 15, 2022

mperrotti Mar 15, 2022

colebemis left a comment

mperrotti commented Mar 21, 2022 •

edited

Loading

mperrotti commented Mar 21, 2022

simurai commented Mar 22, 2022 •

edited

Loading

mperrotti commented Mar 23, 2022

mperrotti commented Mar 23, 2022

simurai commented Mar 24, 2022 •

edited

Loading

mperrotti commented Mar 24, 2022

	<PropsTableRow name="isLoading" type="boolean" description="Whether the switch is ready for user input" />
	<PropsTableRow name="loading" type="boolean" description="Whether the switch is ready for user input" />

Add ToggleSwitch component #1933

Add ToggleSwitch component #1933

Conversation

mperrotti commented Mar 8, 2022

Screenshots

Merge checklist

changeset-bot bot commented Mar 8, 2022 • edited Loading

🦋 Changeset detected

github-actions bot commented Mar 8, 2022 • edited Loading

size-limit report 📦

mperrotti commented Mar 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colebemis left a comment

Choose a reason for hiding this comment

mperrotti commented Mar 21, 2022 • edited Loading

mperrotti commented Mar 21, 2022

simurai commented Mar 22, 2022 • edited Loading

mperrotti commented Mar 23, 2022

mperrotti commented Mar 23, 2022

simurai commented Mar 24, 2022 • edited Loading

mperrotti commented Mar 24, 2022

changeset-bot bot commented Mar 8, 2022 •

edited

Loading

github-actions bot commented Mar 8, 2022 •

edited

Loading

mperrotti commented Mar 21, 2022 •

edited

Loading

simurai commented Mar 22, 2022 •

edited

Loading

simurai commented Mar 24, 2022 •

edited

Loading