fix(Avatar): resolve height/width calculation for AvatarGroup and custom size classes#6008
fix(Avatar): resolve height/width calculation for AvatarGroup and custom size classes#6008maximepvrt wants to merge 7 commits intonuxt:v4from
Conversation
…pport custom size classes
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAvatar.vue: Replaces a static Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/runtime/components/Avatar.vue`:
- Around line 73-78: The computed sizePx logic is setting numericValue to null
which passes the NaN guard and causes 0 instead of the 32 fallback and also
causes a TypeScript type error for parseFloat's parameter; update the size
extraction in the sizePx computed (rootClass.value, sizeClass) so you pass a
string to Number.parseFloat (e.g. use sizeClass?.split('-')[1] ?? '') and assign
numericValue as a number (use Number.parseFloat result, not null), then use
Number.isFinite(numericValue) to decide return numericValue * 4 or the fallback
32; this fixes the TS error and ensures null/undefined/non-numeric values fall
through to the 32 default.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/runtime/components/Avatar.vue`:
- Line 75: The code assigns a bare NaN to numericValue which violates
unicorn/prefer-number-properties; update the expression that computes
numericValue (the const numericValue = ... using sizeClass and
Number.parseFloat) to return Number.NaN instead of the bare NaN when sizeClass
is falsy or split yields no numeric part, i.e., replace occurrences of NaN with
Number.NaN so numericValue uses the Number namespace per the rule while
preserving existing fallback behavior in the sizeClass -> Number.parseFloat
logic.
benjamincanac
changed the title
commit: |
There was a problem hiding this comment.
🤖 Fix all issues with AI agents
Verify each finding against the current code and only fix it if needed.
In `@renovate.json`:
- Around line 5-6: The renovate.json change introduces unrelated configuration
keys ("timezone" and "schedule") into an Avatar bug-fix PR; remove these two
keys (the "timezone" and "schedule" entries) from this branch and either revert
them from the PR or move them into a separate branch and open a dedicated PR
containing only the renovate.json changes so the Avatar fix PR contains only
avatar-related diffs.
🧹 Nitpick comments (1)
🤖 Fix all nitpicks with AI agents
Verify each finding against the current code and only fix it if needed. In `@renovate.json`: - Around line 5-6: The renovate.json change introduces unrelated configuration keys ("timezone" and "schedule") into an Avatar bug-fix PR; remove these two keys (the "timezone" and "schedule" entries) from this branch and either revert them from the PR or move them into a separate branch and open a dedicated PR containing only the renovate.json changes so the Avatar fix PR contains only avatar-related diffs.renovate.json (1)
5-6: Unrelated change bundled into an Avatar bug-fix PR.These Renovate config additions (timezone and schedule) are unrelated to the Avatar height/width fix described in the PR objectives. Consider splitting them into a separate PR for a cleaner commit history and easier bisecting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@renovate.json` around lines 5 - 6, The renovate.json change introduces unrelated configuration keys ("timezone" and "schedule") into an Avatar bug-fix PR; remove these two keys (the "timezone" and "schedule" entries) from this branch and either revert them from the PR or move them into a separate branch and open a dedicated PR containing only the renovate.json changes so the Avatar fix PR contains only avatar-related diffs.
benjamincanac
left a comment
benjamincanac
left a comment
There was a problem hiding this comment.
Good catch!
The class parsing approach introduces a regression though. Avatars with non-numeric size classes like size-full (used in FileUpload) all fall back to width="32" instead of their correct values. You can see it in the FileUpload snapshots dimensions are now all 32.
A simpler fix would be to use the size computed from useAvatarGroup instead of props.size in the lookup and only fall back to class parsing for custom sizes:
const sizePx = computed(() => {
const sizeMap: Record<string, number> = {
'3xs': 16, '2xs': 20, 'xs': 24, 'sm': 28,
'md': 32, 'lg': 36, 'xl': 40, '2xl': 44, '3xl': 48
}
const mapped = sizeMap[size.value || 'md']
if (mapped) return mapped
const classes = ui.value.root({ class: [uiProp.value?.root, props.class] })
const sizeClass = classes.split(' ').find(c => /^size-\d/.test(c))
if (sizeClass) {
const num = Number.parseFloat(sizeClass.split('-')[1] ?? '')
if (!Number.isNaN(num)) return num * 4
}
return 32
})This fixes the AvatarGroup bug without regressing FileUpload.
|
@benjamincanac, your code currently returns early because of the default 'md' value, which makes the size class analysis unreachable when size is undefined. const mapped = sizeMap[size.value || 'md']
if (mapped) return mappedI think we should check for the CSS class first and use the sizeMap only as a fallback. Otherwise, if a user overrides the avatar size via theme, the computed value remains incorrect. |
|
@benjamincanac Just to clarify regarding the fileUpload component in my initial PR: whether we set the width and height to 32px or 36px, those values aren't actually used to fetch an optimized image because we aren't using Nuxt Image but a native img component. Furthermore, the image currently has a size-full sizing applied. As a result, when it's rendered in the grid layout, it stretches to over 100px anyway, completely ignoring the 32px/36px dimensions. 
Is it worth keeping a default size with size-full, at the risk of getting a blurry, pixelated image if Nuxt Image eventually generates a 32px version ? |
2 participants
🔗 Linked issue
❓ Type of change
📚 Description
This PR fixes an issue when
UAvataris used by size defined inUAvatarGroup, the height and width were not correctly resolved. It also enables the generation of correct height and width for custom size attributes, allowing the component to dynamically adapt to any Tailwindsize-{n}class.These attributes are critical for Nuxt Image to fetch and serve the asset at the correct dimensions, preventing layout shifts and optimizing bandwidth.
Changes
Dynamic Extraction: Added a computed property to extract the numerical value from Tailwind
size-{n}classesExample
Using
<UAvatar class="size-64" />will now correctly passheight="256"andwidth="256"to the image renderer, even if a:sizeprop is also present or inherited from a group.📝 Checklist