fix(react): fix icon rendering in v0.9 basic catalog#1031
fix(react): fix icon rendering in v0.9 basic catalog#1031jacobsimionato merged 2 commits intogoogle:mainfrom
Conversation
Add camelCase to snake_case conversion for Material Symbols icon names. The A2UI spec uses camelCase names (e.g., skipPrevious, shoppingCart) but Material Symbols Outlined requires snake_case (skip_previous, shopping_cart). This matches the approach used in the Angular renderer and React v0_8 renderer.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a toSnakeCase helper function to the Icon component to ensure icon names are correctly formatted for Material Symbols. Feedback was provided to improve the regex implementation to avoid generating leading underscores for capitalized input strings.
| * e.g., "shoppingCart" -> "shopping_cart", "skipPrevious" -> "skip_previous" | ||
| */ | ||
| function toSnakeCase(str: string): string { | ||
| return str.replace(/([A-Z])/g, '_$1').toLowerCase(); |
There was a problem hiding this comment.
The current implementation of toSnakeCase will produce a leading underscore for strings that start with an uppercase letter (e.g., 'ShoppingCart' becomes '_shopping_cart'). This is likely not the intended behavior for Material Symbols font ligatures.
A more robust implementation would handle this case correctly. Using a negative lookbehind (?<!^) can prevent matching an uppercase letter at the beginning of the string.
| return str.replace(/([A-Z])/g, '_$1').toLowerCase(); | |
| return str.replace(/(?<!^)([A-Z])/g, '_$1').toLowerCase(); |
There was a problem hiding this comment.
The A2UI schema enforces camelCase icon names via a Zod enum (ICON_NAMES in basic_components.ts), so PascalCase input is never valid. This regex is also identical to Angular v0.9 (icon.component.ts:98) and React v0.8 (Icon.tsx:29). Changing it here would break consistency across renderers.
|
This LGTM, but I saw some icons are still broken - e.g. play? I think the icons in our specification don't exactly match material, so it's necessary to have a lookup table to translate the names. Would you be willing to figure that out? Thank you so much again!! |
|
Thanks for the review and the screenshot! You're right, the camelCase-to-snake_case conversion only fixes multi-word icons. Single-word mismatches like I checked all 59 spec icon names against Material Symbols Outlined ligatures. Only 4 need explicit mapping:
The other 55 all resolve correctly with the camelCase-to-snake_case conversion alone. I'll add the lookup table to this PR. |
|
I checked all v0.9 renderers and the issue goes beyond React:
Lit v0.9 doesn't even do camelCase-to-snake_case, so multi-word icons like Happy to fix all three v0.9 renderers in this PR if you'd prefer, or I can keep this one scoped to React and open separate issues for the others. |
|
Thank you so much for the analysis!! I'll merge this change now seeing as it's a strict improvement and brings renderers to consistency. If you could write another change to fix all three renderers for v0.9 so that all the icons work, that would be amazing! |
|
Of course I can mate!! omw! |
|
Opened #1146 with the override map for all three v0.9 renderers and tests. Hope its good!! |
2 participants
Description
Adds camelCase-to-snake_case conversion for icon names in the React v0.9 Icon component.
The A2UI spec defines icon names in camelCase (e.g.,
shoppingCart) but Material Symbols Outlined font requires snake_case ligatures (shopping_cart). Without this conversion, multi-word icon names rendered as empty/broken boxes. The fix matches the existing pattern used in Angular v0.9 and React v0.8 renderers.Fixes #946
Pre-launch Checklist