fix: display workspace avatars correctly when URLs fail to load #14814
Conversation
|
@Parkreiner, a few changes in the UI tests are showing misalignment. |
|
@matifali Yeah, sorry, I should've left a message. I was hoping this would be a hotfix, but after seeing how many stories would be changed, I decided that I'd put this through a full review process, and then went to bed Definitely not planning to push this through as-is, but appreciate you catching some wonkiness that crept through |
|
Edit: never mind. Good to go
|
There was a problem hiding this comment.
Nothing about the logic here changed – just got formatted differently
| avatar = <Avatar src={src}>{title}</Avatar>; | ||
| avatar = ( | ||
| <Avatar background src={src}> | ||
| {typeof title === "string" ? title || "-" : imgFallbackText ?? "-"} |
There was a problem hiding this comment.
| {typeof title === "string" ? title || "-" : imgFallbackText ?? "-"} | |
| {(typeof title === "string" ? title : imgFallbackText) || "-"} |
| /** | ||
| * Useful for when you need to pass in a ReactNode for the title of the | ||
| * component. | ||
| * | ||
| * MUI will try to take any string titles and turn them into the first | ||
| * character, but if you pass in a ReactNode, MUI can't do that, because it | ||
| * has no way to reliably grab the text content during renders. | ||
| * | ||
| * Tried writing some layout effect/JSX parsing logic to do the extraction, | ||
| * but it added complexity and render overhead, and wasn't reliable enough | ||
| */ |
There was a problem hiding this comment.
| /** | |
| * Useful for when you need to pass in a ReactNode for the title of the | |
| * component. | |
| * | |
| * MUI will try to take any string titles and turn them into the first | |
| * character, but if you pass in a ReactNode, MUI can't do that, because it | |
| * has no way to reliably grab the text content during renders. | |
| * | |
| * Tried writing some layout effect/JSX parsing logic to do the extraction, | |
| * but it added complexity and render overhead, and wasn't reliable enough | |
| */ | |
| /** | |
| * Useful for when you need to pass in a `ReactNode` for the title of the | |
| * component, but you still want to use the first character of a bit of text | |
| * as the fallback. | |
| */ |
| } | ||
|
|
||
| return url.startsWith("/emojis/"); | ||
| } |
There was a problem hiding this comment.
hmm. I'm skeptic about adding special handling like this, but I get the motivation.
No issue to link – this was an issue that surfaced for certain URL types when kicking the tires on the orgs work
Changes made
AvatarDatacomponent to let you manually specify the source of the text used when images fail to load, and updated the orgs breadcrumb segment to use it