Fix v0.9 basic catalog examples and add v0.8 versions#762
Conversation
![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 introduces a comprehensive set of v0.8 and v0.9 basic catalog examples, alongside fixes to existing v0.9 examples. My review focused on ensuring these examples are correct and semantically aligned with their respective specifications. I've identified a few critical issues where examples contained invalid data structures or referenced icons not present in the standard catalogs, which would lead to client-side errors. Additionally, I've provided several medium-severity suggestions to improve the semantic appropriateness of icons, which will enhance clarity and consistency in the rendered UI.
Note: Security Review has been skipped due to the limited scope of the PR.
| "key": "forecast", | ||
| "valueString": "[{'icon': '\u2600\ufe0f', 'temp': '74\u00b0'}, {'icon': '\u2600\ufe0f', 'temp': '76\u00b0'}, {'icon': '\u26c5', 'temp': '71\u00b0'}, {'icon': '\u2600\ufe0f', 'temp': '73\u00b0'}, {'icon': '\u2600\ufe0f', 'temp': '75\u00b0'}]" | ||
| } |
There was a problem hiding this comment.
The valueString for the forecast key contains a Python-style string representation of a list ([{...}]), which is not valid JSON. JSON requires double quotes for all keys and string values. Using single quotes will cause parsing errors on standard JSON clients.
While fixing the quotes makes it a valid JSON string, encoding structured data within a string is an anti-pattern that should generally be avoided, as it forces clients to implement custom parsing logic. Given the limitations of v0.8, if an array of objects cannot be represented directly, consider flattening the data structure for clarity and correctness.
| "key": "forecast", | |
| "valueString": "[{'icon': '\u2600\ufe0f', 'temp': '74\u00b0'}, {'icon': '\u2600\ufe0f', 'temp': '76\u00b0'}, {'icon': '\u26c5', 'temp': '71\u00b0'}, {'icon': '\u2600\ufe0f', 'temp': '73\u00b0'}, {'icon': '\u2600\ufe0f', 'temp': '75\u00b0'}]" | |
| } | |
| "valueString": "[{\"icon\": \"\u2600\ufe0f\", \"temp\": \"74\u00b0\"}, {\"icon\": \"\u2600\ufe0f\", \"temp\": \"76\u00b0\"}, {\"icon\": \"\u26c5\", \"temp\": \"71\u00b0\"}, {\"icon\": \"\u2600\ufe0f\", \"temp\": \"73\u00b0\"}, {\"icon\": \"\u2600\ufe0f\", \"temp\": \"75\u00b0\"}]" | |
| "key": "currentStepIcon", | ||
| "valueString": "local_shipping" | ||
| }, |
There was a problem hiding this comment.
The icon local_shipping is not defined in the v0.8 standard catalog (standard_catalog_definition.json). Using an undefined icon will cause rendering errors on the client. Please use an icon from the allowed list. For example, send is available and semantically related to shipping.
"key": "currentStepIcon",
"valueString": "send"
| "surfaceId": "gallery-shipping-status", | ||
| "value": { | ||
| "trackingNumber": "Tracking: 1Z999AA10123456784", | ||
| "currentStepIcon": "local_shipping", |
There was a problem hiding this comment.
The icon local_shipping is not defined in the v0.9 basic catalog (basic_catalog.json). Using an undefined icon will cause rendering errors on the client. Please use an icon from the allowed list. For example, send is available and semantically related to shipping.
| "currentStepIcon": "local_shipping", | |
| "currentStepIcon": "send", |
| { | ||
| "id": "controls", | ||
| "component": "Row", | ||
| "children": [ | ||
| "prev-btn", | ||
| "play-btn", | ||
| "next-btn" | ||
| ], | ||
| "justify": "center" | ||
| }, |
There was a problem hiding this comment.
The prev-btn and next-btn components use Text components with emojis (⏮ and ⏭) for their visuals. For better semantics, accessibility, and to allow clients to use themed icons, it's recommended to use Icon components instead. The v0.9 basic catalog provides skipPrevious and skipNext icons that would be appropriate here.
jacobsimionato
requested review from
ditman,
dmandar,
nan-yu,
yjbanov and
zeroasterisk
as code owners
jacobsimionato
removed request for
ditman,
dmandar,
nan-yu,
yjbanov and
zeroasterisk
2 participants
This PR updates the v0.9 basic catalog examples based on feedback (correcting icon names, removing invalid actions from TextFields, fixing password field types, and renaming misleading IDs) and adds corresponding v0.8 versions adapted to the v0.8 specification.