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

fix: add focused stylign to tabs #714

Merged
merged 1 commit into from
Nov 29, 2023
Merged

fix: add focused stylign to tabs #714

merged 1 commit into from
Nov 29, 2023

Conversation

ahuth
Copy link
Contributor

@ahuth ahuth commented Nov 29, 2023

This PR adds a focus outline to the tabs component, for the SDS accessibility hackday project.

Screenshot 2023-11-29 at 12 56 57 PM

See it live on Chromatic

Keyboard usage of the tabs is actually working as expected:

  • Tab onto and off of the active tab
  • When focused on the active tab, left and right arrows changes the active tab
  • Space changes to the selected tab

@@ -94,6 +94,11 @@ export const StyledTab = styled(RawTab, {
color: black;
}
&.Mui-focusVisible {
outline: 5px auto Highlight;
outline: 5px auto -webkit-focus-ring-color;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is currently the same focus outline that buttons have:
Screenshot 2023-11-29 at 12 56 57 PM

I am not a designer, and I'm happy to change this to something else. Figured it made sense for now to keep it simple and match buttons and text inputs.

Note that mui has default focus styles which we're suppressing as part of the theme:

MuiButtonBase: {
defaultProps: {
disableRipple: true,
},
},
MuiLink: {
defaultProps: {
underline: "hover",
},
},
MuiTab: {
defaultProps: {
disableRipple: true,
},
},
},

We could turn those back on, although it looks weird and would take tweaking colors for both buttons and tabs.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 hmm... Are we re-doing work that we can just get for free from Mui?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We definitely are, although I think it would take design and eng work to get back to something that looks reasonable with the defaults for all components.

@ahuth
Copy link
Contributor Author

ahuth commented Nov 29, 2023

Thanks 🙏

@ahuth ahuth merged commit eb6870f into main Nov 29, 2023
12 checks passed
@ahuth ahuth deleted the ah-tabs-focus branch November 29, 2023 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants