Skip to content

Conversation

@JordanConnor
Copy link
Contributor

Accessible selection and hover backgrounds (roomlist, settings etc)

@JordanConnor JordanConnor requested a review from a team as a code owner October 3, 2025 10:52
Copy link
Member

@pixlwave pixlwave left a comment

Choose a reason for hiding this comment

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

Looks good to me, although without context, the naming it isn't clear exactly where these new tokens fit in to the system. For example we have

bg/primary/rest, bg/primary/hovered & bg/primary/pressed
bg/secondary/rest, bg/secondary/hovered & bg/secondary/pressed
bg/accent/rest, bg/accent/hovered, bg/accent/pressed & bg/accent/selected

where the specific states are related to the intended use. Are these new ones intended to be used against e.g. bg/canvas/default, bg/subtle/primary and bg/subtle/secondary, or just one of them?

Which is a roundabout way of asking if these would make more sense to be bg/canvas/hovered and bg/canvas/selected?

"color.gradient.super": "S:012790c8c979c2e2d44a175382ee3b5865467656,",
"color.gradient.action.linear": "S:f429bb5c10c2aa662f844bdaaaa0378fdac2bdfa,"
"color.gradient.action.linear": "S:f429bb5c10c2aa662f844bdaaaa0378fdac2bdfa,",
"color.bg.badge.default": "S:2e389dd1535965b7704292fcaeac993cfe68887b,",
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure where this one has come from 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Looks like the last time the file was regenerated was part of the way through #147. I believe this file has to be exported manually from Tokens Studio?

Comment on lines +493 to +494
"color.bg.selection": "S:aafc47c563900ea23df3a4d400255fbba2bf8daa,",
"color.bg.hover": "S:3b12413853bf0d7841fd64b2dc8da9dd61e7639e,"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe these should be .selected and .hovered for consistency with other tokens in the system?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants