Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(design): add basic switch component #3143

Merged
merged 23 commits into from
Dec 16, 2024

Conversation

gracetxgao
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Starter for #1022

What is the new behavior?

A basic switch component is set up.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@gracetxgao gracetxgao requested a review from a team as a code owner October 4, 2024 06:56
@xelaint xelaint added package: design @daffodil/design status: wip This PR is WIP. It should be marked as a draft. labels Oct 4, 2024
@xelaint xelaint marked this pull request as draft October 4, 2024 18:27
@xelaint
Copy link
Member

xelaint commented Oct 4, 2024

To do:

  • Loading state
  • Disabled state (ability to disable a switch, or set disabled when toggle is also loading)
  • Show error message if switch fails to toggle
  • A label position property that changes the orientation of the label from being displayed left or right of the toggle UI
  • ARIA considerations (https://www.w3.org/WAI/ARIA/apg/patterns/switch/)
  • Keyboard interactions

@gracetxgao gracetxgao force-pushed the daff-switch-component branch from 9483197 to 7b0d182 Compare October 12, 2024 02:44
@xelaint
Copy link
Member

xelaint commented Oct 14, 2024

To-do ARIA considerations (https://www.w3.org/WAI/ARIA/apg/patterns/switch/):

  • A visible label referenced by the value of aria-labelledby set on the element with role switch.
  • aria-label set on the element with role switch.
  • When on, the switch element has state aria-checked set to true.
  • When off, the switch element has state aria-checked set to false.

@gracetxgao gracetxgao force-pushed the daff-switch-component branch from 6c0e528 to 866759c Compare October 25, 2024 02:08
@xelaint
Copy link
Member

xelaint commented Oct 28, 2024

@gracetxgao I went ahead and made updates to the aria labels. Lmk if you have any questions.

In the basic switch example, clicking on the disabled or loading switches toggles the default switch. Clicking on the switch in switch-label-positions-example also toggles the default switch. I think you may need to add form controls to examples.

Copy link
Member

@xelaint xelaint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keyboard functionalities:

  • tab key should tab to a switch. Switches are currently not tabbable.
  • space key to toggle the switch.

@gracetxgao gracetxgao force-pushed the daff-switch-component branch from bd41362 to 41ec631 Compare November 5, 2024 02:02
Copy link
Member

@xelaint xelaint left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seems to be something funny with tabindex and focus. When I tab to the first switch and then tab again, the focus goes somewhere not visible. Focus goes to the HTML button upon the 3rd tab.

Tabbing after focus is on the switch should move focus to the next available element (in this case is the HTML button). Something else that's not visible within the component seems to be tabbable.

@xelaint xelaint marked this pull request as ready for review November 5, 2024 16:10
@xelaint xelaint requested a review from a team as a code owner November 5, 2024 16:10
@xelaint xelaint added status: awaiting review This PR is awaiting review and removed status: wip This PR is WIP. It should be marked as a draft. labels Nov 5, 2024
@xelaint xelaint requested a review from damienwebdev November 5, 2024 16:10
@xelaint xelaint force-pushed the daff-switch-component branch from 35347df to d78cc5c Compare November 27, 2024 05:02
@damienwebdev
Copy link
Member

damienwebdev commented Dec 2, 2024

@gracetxgao please remove the package-lock.json from this diff.

xelaint
xelaint previously approved these changes Dec 11, 2024
@xelaint
Copy link
Member

xelaint commented Dec 12, 2024

@gracetxgao please rebase on develop since we pushed updates to linting, so there may be a few linting errors in this PR.

@gracetxgao gracetxgao force-pushed the daff-switch-component branch from 017a7a9 to 2b37cf8 Compare December 12, 2024 18:30
Copy link
Member

@damienwebdev damienwebdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@damienwebdev damienwebdev merged commit 8879733 into graycoreio:develop Dec 16, 2024
13 checks passed
@gray-bot gray-bot mentioned this pull request Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: design @daffodil/design status: awaiting review This PR is awaiting review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants