avatar: Add mode prop#3943
Conversation
🦋 Changeset detectedLatest commit: f75cedb The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
thompsongl
left a comment
There was a problem hiding this comment.
LGTM. The mode enum makes sense to me and the implementation appears to provide the necessary & requested styling hooks
| forwardedRef, | ||
| React.useCallback( | ||
| (node: HTMLImageElement | null) => { | ||
| if (!src) { |
There was a problem hiding this comment.
default mode sets status error + fires onLoadingStatusChange when src is missing; native stays idle and never fires. Should align them or document the difference.
| context.setImageLoadingStatus('error'), | ||
| )} | ||
| onLoad={composeEventHandlers(imageProps.onLoad, (event) => { | ||
| if (!src) { |
There was a problem hiding this comment.
This if (!src) return; guard and comment are copied from the ref callback. onLoad can only fire once the image has a src and loads, so the branch looks unreachable here, and the comment (about not updating from error) describes the ref-callback case rather than this one.
| }, [imageLoadingStatus, handleLoadingStatusChange]); | ||
|
|
||
| return ( | ||
| <Primitive.img |
There was a problem hiding this comment.
Always renders, which can lead to duplicate information between this and fallback on non-loaded states - should this be aria-hidden on non-loaded states?
| .image-root { | ||
| position: relative; | ||
| } | ||
| .image[data-radix-avatar-loading-status]:not( |
There was a problem hiding this comment.
data-radix-avatar-loading-status diverges from the data-state convention for consumer-facing state elsewhere (Progress exposes its loading lifecycle that way too) — I'm guessing this is an intentional direction change?
Also these are necessary to avoid duplicate UI, so it should maybe be called out more loudly.
This PR introduces a new
modeprop forAvatar.Imagefor more control over loading state and rendering.mode="default"(default): Matches the existing behavior by rendering animgelement conditionally based on the loading state of anImageobject constructed after hydration. This ensures backwards compatibility.mode="native": Renders animgelement unconditionally using its event handlers to update loading state. Innativemode, you can use CSS to target the image element and style it based on its loading state. Closes [Avatar] Avatar.Image doesn't implement lazy loading #2512 since thelazyattribute would dictate when the loading handler is called.mode="custom": Allows for more control over the image rendering. Therenderprop must be provided to render the image. This mode is useful when you want to use a custom image component, such as framework-specific image components or a design-system implementation that uses non-standard props. Closes [Avatar] AllowAvatar.Imageto be supplied acomponentto render the image #2230