-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
feat(design): add basic switch component #3143
Conversation
To do:
|
9483197
to
7b0d182
Compare
libs/design/switch/examples/src/basic-switch/basic-switch.component.html
Outdated
Show resolved
Hide resolved
To-do ARIA considerations (https://www.w3.org/WAI/ARIA/apg/patterns/switch/):
|
6c0e528
to
866759c
Compare
@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 |
There was a problem hiding this 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.
libs/design/switch/examples/src/basic-switch/basic-switch.component.html
Outdated
Show resolved
Hide resolved
bd41362
to
41ec631
Compare
libs/design/switch/examples/src/disabled-switch/disabled-switch.component.html
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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.
35347df
to
d78cc5c
Compare
@gracetxgao please remove the |
@gracetxgao please rebase on develop since we pushed updates to linting, so there may be a few linting errors in this PR. |
017a7a9
to
2b37cf8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
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?
Other information