Improve examples for Avatar, Checkbox, ColorField, ColorPicker, and Icon#3926
Improve examples for Avatar, Checkbox, ColorField, ColorPicker, and Icon#3926
Conversation
|
We detected some changes in |
sordaz00
changed the title
Updates example documentation for Avatar, Checkbox, ColorField, ColorPicker, and Icon components: - Update all example descriptions to 3-sentence pattern - Remove 6 redundant examples to reduce noise - Fix ColorPicker default to show basic picker (no alpha) - Merge format hint into ColorField default placeholder Components: Avatar (8), Checkbox (5), ColorField (8), ColorPicker (2), Icon (8)
- Remove JSX tabs, keep HTML with language: 'preview' for interactive previews - Remove 'Click to interact with the preview.' from all descriptions - Add missing backticks for property names and values - Delete unused .jsx example files (keep JSX-only validation examples)
sordaz00
force-pushed
the
uiext-polaris-examples-batch2-clean
branch
from
2a92e82 to
894fc18
Compare
mcvinci
left a comment
mcvinci
left a comment
There was a problem hiding this comment.
Thanks for these improvements, @sordaz00! I think I have similar thoughts as here.
It might be worthwhile revisiting some descriptions to make them a little more closely tied to describing what's happening in the code (referencing specific props, etc). Non-blocking comment though, and definitely something we could consider in a follow-up PR. ❤️
| { | ||
| description: | ||
| 'Shows how the component handles store and business names of varying lengths in commerce contexts.', | ||
| 'Display initials of varying lengths consistently. This example shows avatars with 2, 3, and 4 character initials.', |
There was a problem hiding this comment.
| 'Display initials of varying lengths consistently. This example shows avatars with 2, 3, and 4 character initials.', | |
| 'Display initials of varying lengths consistently. This example shows avatars with two, three, and four character initials.', |
| { | ||
| description: | ||
| 'Demonstrates avatar integration with other admin-ui components in a merchant section layout.', | ||
| 'Build rich merchant profile cards. This example shows an avatar combined with Section, Heading, and Text components for a complete layout.', |
There was a problem hiding this comment.
| 'Build rich merchant profile cards. This example shows an avatar combined with Section, Heading, and Text components for a complete layout.', | |
| 'Build rich merchant profile cards. This example shows an avatar combined with [section](/docs/api/{API_NAME}/{API_VERSION}/polaris-web-components/layout-and-structure/section), [heading](/docs/api/{API_NAME}/{API_VERSION}/polaris-web-components/typography-and-content/heading), and [text](/docs/api/{API_NAME}/{API_VERSION}/polaris-web-components/typography-and-content/text) components for a complete layout.', |
There was a problem hiding this comment.
"Build rich merchant profile cards" sounds a little on the market-y side. 😆 Maybe there's a way that we can disambiguate this?
| 'Build rich merchant profile cards. This example shows an avatar combined with Section, Heading, and Text components for a complete layout.', | ||
| codeblock: { | ||
| title: 'With Section component', | ||
| title: 'Combine with Section', |
There was a problem hiding this comment.
| title: 'Combine with Section', | |
| title: 'Build a merchant profile card', |
There was a problem hiding this comment.
Not quite sure this is the route to go, but I was just trying to highlight the use case versus the underlying tech here.
| defaultExample: { | ||
| image: 'color-picker-default.png', | ||
| description: | ||
| 'Let users visually select colors with an interactive picker. This example shows a basic color picker with hue and saturation controls.', |
There was a problem hiding this comment.
I noticed that color picker has far fewer examples than other components. Is it worthwhile adding another example or two? Are there other use cases we haven't illustrated?
There was a problem hiding this comment.
Hey! Nice work on this. Left some comments.
There are some definite code issues to look at though.
Code review (.html + .jsx example files)
Avatar
default.html vs basic-usage.html: Near-identical examples. default.html shows initials="JD" alt="John Doe", basic-usage.html shows initials="SC" alt="Sarah Chen". Same pattern, same props, different names. One should be removed.
in-customer-list-context.html: Uses relative paths src="/customers/merchant-alice.jpg" and src="/customers/charlie-cafe.jpg". These will never render in a sandboxed extension environment. Should use CDN URLs (e.g., https://cdn.shopify.com/...) or remove the src to show initials-only fallback.
with-image-source-and-fallback.html: Uses relative path src="/customers/profile-123.jpg". The "fallback" behavior can't be demonstrated since the image will always fail.
with-section-component.html: Uses relative path src="/merchants/premium-store.jpg". Same issue.
color-consistency-demo.html: Contains HTML comment <!-- Note: Both AB avatars will have the same color -->. The behavior should be described in the doc.ts description, not inline.
Example count (9): High for a component with 3 props (initials, alt, src, size). Consider merging default.html + basic-usage.html, and possibly size-variations.html + long-initials-handling.html.
ColorField
default.html: Has both placeholder="Select a color (e.g., #FF0000)" and value="#FF0000". The placeholder is invisible when a value is present. these props are contradictory.
**default.html:**Mmissin a label prop. All other ColorField examples have labels. The default example should demonstrate best practices.
basic-usage-web-component.html vs basic-usage-required.html: Nearly identical. Same label="Brand color", same value="#FF0000". One adds name="color", the other adds required. These could be a single example with both name and required.
form-integration.html: Uses both value and defaultValue on the same field (value="#1a73e8" defaultValue="#1a73e8"). In React-style components, value is controlled and defaultValue is uncontrolled . I think using both is a bit backwardss. Should use one or the other.
validation-example.jsx: Uses onInput event. Verify this is a valid event for s-color-field (batch1 had the same question for s-money-field). Also uses e.currentTarget.value to read the field value.
ColorPicker
with-alpha-transparency.html: Wrapped in s-box with border/padding styling. Unnecessary wrapper for this example. It adds visual noise, without doin' much.
Icon
default.html vs basic-usage.html: Near-identical. Both show inline icon rows. default.html has home, order, product, settings. basic-usage.html has home, edit, duplicate, save, export. Same pattern, different icon sets. One should be removed.
with-id-property.html: <s-icon type="settings" id="settings-icon"></s-icon>. The id attribute is a generic HTML capability, really applies to a lot of stuff. You can add id to any element. This doesn't demonstrate any Icon-specific functionality.
in-button-components.html: Shows <s-button icon="plus">. This uses the Button's icon prop, not the s-icon component. While it shows icons in context, it doesn't demonstrate s-icon usage.
in-badge-components.html: Same issue -- shows <s-badge icon="check-circle">, not s-icon.
| defaultExample: { | ||
| image: 'avatar-default.png', | ||
| description: | ||
| 'Identify users visually when no profile image is available. This example shows an avatar displaying initials derived from a name.', |
There was a problem hiding this comment.
- "Identify users visually" is a little vague. Aall avatars identify users visually
- "initials derived from a name" overstates it I think. Thehe code just has
initials="JD", there's no derivation happening
| { | ||
| description: | ||
| 'Loads a customer profile image with automatic fallback to initials if the image fails to load.', | ||
| 'Display profile photos with graceful error handling. This example shows an avatar with a source image that falls back to initials if the image fails to load.', |
There was a problem hiding this comment.
| 'Display profile photos with graceful error handling. This example shows an avatar with a source image that falls back to initials if the image fails to load.', | |
| 'Display profile photos with error handling. This example shows an avatar with a source image that falls back to initials if the image fails to load.', |
There was a problem hiding this comment.
Wasn't sure what graceful error handlin' was. Either you handle it or don't. 😄 Could say something like: Show an image that falls back to initials when the image fails to load.
- The example uses a relative path (
/customers/profile-123.jpg) so the "fallback" is all you'll ever see. Is this URL correct?
| { | ||
| description: | ||
| 'Displays customer and merchant avatars in different sizes for various interface contexts.', | ||
| 'Adapt avatar prominence to different UI contexts. This example shows all five available sizes from `small-200` to `large-200`.', |
There was a problem hiding this comment.
- "prominence" and "UI contexts" feels like vague marketing-y language. Can we be more specific to what the example is doing?
| { | ||
| description: | ||
| 'Demonstrates that identical initials always receive the same color assignment across different store types.', | ||
| 'Ensure visual consistency across the interface. This example shows that avatars with identical initials always display the same background color.', |
There was a problem hiding this comment.
'visual consistency' could mean a lot of different things here. The second sentence is 🔥
| defaultExample: { | ||
| image: 'color-field-default.png', | ||
| description: | ||
| 'Let users select colors using a visual picker or text input. This example shows a color field with a placeholder and pre-selected hex value.', |
There was a problem hiding this comment.
"This example shows a color field with a placeholder and pre-selected hex value."
It's advertising both props as the point of the example, but only one of them is ever visible at a time. A dev would look at the rendered preview, see #FF0000 in the field, and wonder where the placeholder is that the description mentions.
Example behind this: The HTML is:
<s-color-field placeholder="Select a color (e.g., #FF0000)" value="#FF0000"></s-color-field>
When a value is set, the placeholder is never reall visible. The browser/component renders the value instead. So placeholder="Select a color (e.g., #FF0000)" is dead code. The developer copying this example would never see the placeholder unless they delete the value first.
It's also missing a label prop. Every other ColorField example has a label.
Would need to do something like:
<s-color-field
label="Brand color"
value="#FF0000"
></s-color-field>
With no placeholder. Or with it:
<s-color-field
label="Brand color"
placeholder="e.g., #FF0000"
></s-color-field>
| { | ||
| description: | ||
| 'Interactive example showing real-time hex color validation with error messages.', | ||
| 'Provide immediate feedback on color format validity. This example shows real-time validation that checks hex format as the user types.', |
There was a problem hiding this comment.
I see quite a few descriptions that say "Provide immediate feedback". I think we could be more specific on what feedback, or how it renders.
| 'Provide immediate feedback on color format validity. This example shows real-time validation that checks hex format as the user types.', | ||
| codeblock: { | ||
| title: 'Color validation', | ||
| title: 'Validate in real time', |
There was a problem hiding this comment.
Validate what? But we don't want to make the title too long.
| defaultExample: { | ||
| image: 'color-picker-default.png', | ||
| description: | ||
| 'Let users visually select colors with an interactive picker. This example shows a basic color picker with hue and saturation controls.', |
There was a problem hiding this comment.
I don't think we're actually letting someone do something here. This also feels like it's just restating what ColorPicker is.
| { | ||
| description: | ||
| 'Compact icon sizing for space-constrained interfaces and inline usage. Shows how to render a small-sized icon that takes up minimal space while maintaining clarity.', | ||
| 'Fit icons into tight layouts without losing clarity. This example shows a small-sized icon that takes up minimal space.', |
There was a problem hiding this comment.
"without losing clarity" feels a bit marketing-y. Clarity of what? nothing in the code demonstrates "clarity", really.
| { | ||
| description: | ||
| 'Icon with unique identifier for JavaScript targeting and styling. Demonstrates adding a specific ID to an icon, which can be used for JavaScript interactions, CSS styling, or accessibility purposes.', | ||
| 'Enable programmatic access to specific icons. This example shows an icon with an ID attribute for JavaScript targeting or CSS styling.', |
There was a problem hiding this comment.
Not a big thing, but maybe the example needs to be improved. This describes adding an HTML id attribute, which is a generic HTML capability, not Icon-specific.
3 participants
Updating examples only for App Home polaris components: Avatar, Checkbox, ColorField, ColorPicker, and Icon.
Updates:
Relates to:
https://github.com/Shopify/shopify-dev/issues/67413
https://github.com/Shopify/shopify-dev/issues/67415
https://github.com/Shopify/shopify-dev/issues/67416
https://github.com/Shopify/shopify-dev/issues/67430
https://github.com/Shopify/shopify-dev/issues/67431
Test plan