feat: show organization name in workspaces table #14547
Conversation
| play: async ({ canvasElement }) => { | ||
| const canvas = within(canvasElement); | ||
| const accessibleTableCell = await canvas.findByRole("cell", { | ||
| // Need whitespace classes because different parts of the element | ||
| // are going in different spans, and Storybook doesn't consolidate | ||
| // these | ||
| name: /org\s?(?:anization)?\s?:\s?Limbus Co\./i, | ||
| }); | ||
|
|
||
| expect(accessibleTableCell).toBeDefined(); | ||
| }, |
There was a problem hiding this comment.
Just wanted to make sure that I understand the limitations around decorators: they're basically assumed to always be static values, right? There's no easy way to make a decorator re-render with a new value?
There was a problem hiding this comment.
each "decorator" undergoes a small transformation to get turned into a component by storybook. there's no way to force a rerender from the outside, just like any other component. being a component tho, they can call hooks and have state, which is kind of non-obvious.
| org | ||
| <span css={{ ...visuallyHidden }}>anization</span>: |
There was a problem hiding this comment.
are screen readers going to handle this split well though? my gut feeling is they say "org anization" with some awful disconnected pronounciation
There was a problem hiding this comment.
I tried it with the built-in Mac screen reader, but (1) I think I was just assuming that any elements that are semantically inline elements wouldn't affect pronunciation, and (2) the Mac screen reader isn't even the most popular reader by a long shot
Let me change this just to be on the safe side
| @@ -54,7 +54,7 @@ export const AvatarData: FC<AvatarDataProps> = ({ | |||
| css={{ | |||
| fontSize: 13, | |||
| color: theme.palette.text.secondary, | |||
| lineHeight: "150%", | |||
| lineHeight: 1.5, | |||
There was a problem hiding this comment.
ah the wonderful subtleties of css. so annoying that this actually means something different. 🙃
There was a problem hiding this comment.
CSS is so silly sometimes
|
@aslilac Sorry I didn't get back to this yesterday, but now that I think about it, I think you're right, and that the labels are unnecessary I guess my worry was that, as we keep cramming multiple values into single table cells, we still need a way to structure the information. Because the HTML spec basically assumes that table cells will basically be like database columns – no more than one value per cell. So anytime we diverge from that, we can't assume that we'll have accessible info just because we're using the correct baseline semantics. I think I'm also just a bit skittish about multiple unlabeled values in general I still think the labels are good for screen readers, but I do think it's overkill for sighted users. Let me switch things around, which should also make it easier to maintain on our end |
Closes #14278
Changes made
showOrganizationproperty fromuseDashboardistrueAvatarDatacomponent to make sure that its line height can't inherently incorrectly