Skip to content

Conversation

@ansor4
Copy link
Member

@ansor4 ansor4 commented Jan 7, 2026

https://artsyproduct.atlassian.net/browse/EMI-2614

Description

  • Shows outline around the entire tab instead of the current behavior, which only highlights bottom and side of tab
  • I had a hard time getting the focus outline to expand to the top, but that's the current area of the Clickable element. I imagine the core fix there is to be the Clickable element take up that space.

Before:
Screenshot 2026-01-06 at 8 47 04 PM

Screenshot 2026-01-06 at 8 47 00 PM

After:

Screenshot 2026-01-06 at 8 49 58 PM Screenshot 2026-01-06 at 8 50 46 PM

@ansor4 ansor4 requested review from dzucconi and rquartararo January 7, 2026 04:56
@ansor4 ansor4 self-assigned this Jan 7, 2026
Copy link
Member

@dzucconi dzucconi left a comment

Choose a reason for hiding this comment

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

It'd make sense for this to be a more intentionally designed state, which this doesn't appear to be? (I could imagine the text just being underlined, or the bg getting darker, etc.)

&:focus {
outline: none;
z-index: 2;
border: 1px solid ${themeGet("colors.blue100")};
Copy link
Member

Choose a reason for hiding this comment

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

I find it surprising this doesn't alter the spacing of the elements, which applying a border typically would.

@rquartararo
Copy link
Member

This is just for accessibility so we don't want to use the focus state here as this will render the blue border on mouse click as well.

@rquartararo
Copy link
Member

rquartararo commented Jan 7, 2026

@ansor4 I'm good with this change if we use focus-visible as @dzucconi suggested, though ideally I'd like to figure out why the outline is being cut off 🤔

@dzucconi
Copy link
Member

dzucconi commented Jan 7, 2026

It gets cut off because of the overflow-x on the HorizontalOverflow. outline positions a border outside of the bounding box of the div. box-shadow is also an acceptable attribute to use to create the outline if the border poses a positioning problem. I still think something else, like a text-decoration, would look better though.

@rquartararo
Copy link
Member

@dzucconi I don't think text-decoration would be sufficient for accessibility. Adding 1px of padding to the HorizontalOverflow on BaseTabs seems to do the trick.

@dzucconi
Copy link
Member

dzucconi commented Jan 7, 2026

Eh, I wouldn't do that as it'll change the spacing of every instance by 1px. Better to use border or box-shadow.

@ansor4 ansor4 merged commit 6958919 into main Jan 13, 2026
11 checks passed
@ansor4 ansor4 deleted the ansor4/accessibility-tabs branch January 13, 2026 16:52
@artsyit
Copy link
Contributor

artsyit commented Jan 13, 2026

🚀 PR was released in @artsy/[email protected], @artsy/[email protected] 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants