-
-
Notifications
You must be signed in to change notification settings - Fork 189
fix(ui): improve MetricsBadges skeleton alignment #929
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
| </ClientOnly> | ||
| <template #fallback> | ||
| <div | ||
| class="flex items-center gap-1.5 list-none m-0 p-0 relative top-[5px] self-baseline ms-1 sm:ms-2" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The relative top-[5px] sucks a bit, but gets the job done. Honestly, I have no better idea how to fix it while keeping baseline alignment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about having the text visible so it's just the icon that loads? we could even replace it with a spinner to give better feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's probably a good idea, but if I may have a say, I'd do it as a next iterative improvement. It's likely not going to be such an easy fix as this was.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pushed a first stab - what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes the alignment of skeleton loading placeholders for the MetricsBadges component by adjusting the positioning, sizing, and flex layout structure.
Changes:
- Restructured the ClientOnly wrapper to be inside the flex container instead of wrapping it
- Added
contentsclass to list items in MetricsBadges to eliminate list item boxes and improve flex layout behavior - Changed badge elements from
inline-flextoflexfor better integration with the parent flex layout
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/pages/package/[...package].vue | Moved ClientOnly wrapper inside flex container and adjusted skeleton fallback styling with positioning and sizing changes |
| app/components/Package/MetricsBadges.vue | Added contents class to list items and changed badge display from inline-flex to flex |
📝 WalkthroughWalkthroughDestructures Possibly related PRs
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/Package/MetricsBadges.vue (1)
57-71:⚠️ Potential issue | 🟡 MinorPotential list semantics loss with
display: contentson<li>
display: contentscan cause some assistive tech to ignore list items. If you want to keep list semantics, consider addingrole="listitem"to the badge wrapper (or avoidcontents).🔧 Possible low-impact fix
- <li v-if="!props.isBinary" class="contents"> - <TooltipApp :text="typesTooltip"> + <li v-if="!props.isBinary" class="contents"> + <TooltipApp :text="typesTooltip" role="listitem"> <component :is="typesHref ? NuxtLink : 'span'" :to="typesHref" :tabindex="!typesHref ? 0 : undefined" class="flex items-center gap-1 px-1.5 py-0.5 font-mono text-xs rounded transition-colors duration-200 focus-visible:(outline-2 outline-accent)" - <li class="contents"> - <TooltipApp :text="hasEsm ? $t('package.metrics.esm') : $t('package.metrics.no_esm')"> + <li class="contents"> + <TooltipApp :text="hasEsm ? $t('package.metrics.esm') : $t('package.metrics.no_esm')" role="listitem"> <span tabindex="0" class="flex items-center gap-1 px-1.5 py-0.5 font-mono text-xs rounded transition-colors duration-200 focus-visible:(outline-2 outline-accent)" - <li v-if="hasCjs" class="contents"> - <TooltipApp :text="$t('package.metrics.cjs')"> + <li v-if="hasCjs" class="contents"> + <TooltipApp :text="$t('package.metrics.cjs')" role="listitem"> <span tabindex="0" class="flex items-center gap-1 px-1.5 py-0.5 font-mono text-xs text-fg-muted bg-bg-muted border border-border rounded transition-colors duration-200 focus-visible:(outline-2 outline-accent)"Also applies to: 84-93, 106-111
4c86d80 to
3932474
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
| <SkeletonBlock class="w-13 h-5.5 rounded" /> | ||
| <SkeletonBlock class="w-13 h-5.5 rounded" /> | ||
| <div class="w-13 h5.5" /> | ||
| <SkeletonBlock class="w-13 h-5.5 rounded bg-bg-subtle" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intentionally used a white div as the like button is itself white, but I'm okay with this.
Skeleton was misaligned due to:
Before:
Nagranie.z.ekranu.2026-02-4.o.12.31.24.mov
It's actually even worse than that because also the right toolbar with docs, code and compare may jump to the next line because of it.
After:
Nagranie.z.ekranu.2026-02-4.o.12.30.59.mov