fix(settings): make elements use proper focus colors#18852
fix(settings): make elements use proper focus colors#18852MagentaManifold merged 1 commit intomainfrom
Conversation
vpomerleau
left a comment
vpomerleau
left a comment
There was a problem hiding this comment.
Nice work on these! I had a look through with storybook and added some comments for your consideration :)
| <LinkExternal | ||
| href="https://www.mozilla.org/about/?utm_source=firefox-accounts&utm_medium=Referral" | ||
| data-testid="link-mozilla" | ||
| // Using grey outline here to match the rest of the footer |
There was a problem hiding this comment.
Could we switch the focus outline to our standard blue for consistency across the app? If there’s a specific reason to match the component’s current style instead, let me know and we can discuss.
| aria-expanded={!!isRevealed} | ||
| aria-haspopup="menu" | ||
| className="rounded-full border border-transparent hover:border-purple-500 focus:border-purple-500 focus:outline-none active:border-purple-700 transition-standard" | ||
| className="rounded-full border border-transparent hover:border-purple-500 focus-visible:outline focus-visible:outline-blue-500 focus-visible:outline-2 active:border-purple-700 transition-standard" |
There was a problem hiding this comment.
The focus outline looks great 👌 Do you have ideas about how to update the hover styling which is still purple and conflicts a tad with the focus outline?
|
Extra note that I also had a peek at the Storybook in Windows with High-Contrast Mode active, and the focus outlines look good there too! |
vpomerleau
left a comment
vpomerleau
left a comment
There was a problem hiding this comment.
Great attention to detail going through all the components + finding small tweaks that make the styling sharper.
I've added some thoughts for consideration - no blockers though because these updated styles all look good on Storybook. Please reach out if you'd like to discuss anything here!
| > | ||
| <button | ||
| className="w-4 h-4" | ||
| className="w-4 h-4 rounded-sm focus-visible-neutral outline-offset-2" |
There was a problem hiding this comment.
Same as before + this component is not currently in use so it's not important, but just noting that this close icon doesn't have hover or active state indicator (so no visible interaction feedback on this button unless navigating with keyboard)
| title={localizedButtonTitle} | ||
| aria-label={localizedButtonAriaLabel} | ||
| className="me-4 tablet:me-0 tablet:p-4 tablet:absolute tablet:-start-24" | ||
| className="me-4 tablet:me-0 tablet:p-4 tablet:absolute tablet:-start-24 focus-visible-neutral" |
There was a problem hiding this comment.
This work on focus states is making it obvious that many buttons/links are missing interaction feedback on hover/active states. 😰
| aria-expanded={!!isRevealed} | ||
| aria-haspopup="menu" | ||
| className="rounded-full border border-transparent hover:border-purple-500 focus:border-purple-500 focus:outline-none active:border-purple-700 transition-standard" | ||
| className="rounded-full border border-transparent hover:border-purple-500 active:border-purple-700 transition-standard focus-visible-neutral focus-visible:border-transparent" |
There was a problem hiding this comment.
You're already aware of this, but just noting here that the hover/active states here need a review from UX because they don't look clean when combined with the focus outline. This could be pulled in as a quick follow-up with some of the other interaction state updates you have planned (e.g., for the shadowy outlines on links) instead of holding up this PR though.
Because: * Front-page Settings elements focus use system default outlines. This commit: * makes elements use proper focus outline colors and adds rounded corners to focus outlines. Closes #FXA-5787
2 participants
Because
This pull request
Issue that this pull request solves
Closes: FXA-5787
Checklist
Put an
xin the boxes that applyScreenshots (Optional)
etc. Please checkout Storybook for full detail.
Other information (Optional)
Made some changes to the spacing (e.g., padding -> margin, margin -> gap) to make the outline box sizing look right. No actual layout is changed.