chore: align base components size prop values#2069
chore: align base components size prop values#2069antoineneff wants to merge 4 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR standardises size tokens across UI components: it renames size prop values from verbose tokens ('small'|'medium'|'large') to compact tokens ('sm'|'md'|'lg'), updates default values (where applicable) and corresponding class mappings in Button, Input and Link base components, and replaces usages site-wide (components, pages, stories and tests). No control-flow, state, or behavioural logic changes were introduced. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/components/Link/Link.stories.ts (1)
106-129:⚠️ Potential issue | 🟡 MinorSnapshot still uses legacy size token at Line 125.
The snapshot example still passes
size="small", which is out of sync with the newsm/mdconvention.🔧 Proposed fix
- <LinkBase to="/" variant="button-primary" size="small">Small Button</LinkBase> + <LinkBase to="/" variant="button-primary" size="sm">Small Button</LinkBase>
🧹 Nitpick comments (1)
app/components/Compare/FacetSelector.vue (1)
64-73: Use global focus-visible styling for this button instead of inline utility classes.Line 72 adds
focus-visible:outline-accent/70on aButtonBase; this should rely on the global button focus-visible rule used across the project.Based on learnings: In the npmx.dev project, focus-visible styling for buttons and selects is applied globally via `app/assets/main.css`, so per-element inline focus-visible utilities on buttons should be avoided.🎯 Suggested adjustment
- class="gap-1 px-1.5 rounded transition-colors focus-visible:outline-accent/70" + class="gap-1 px-1.5 rounded transition-colors"
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e65d1ea7-2ac2-48b9-87a1-a2dbd66c19d1
📒 Files selected for processing (31)
app/components/Button/Base.stories.tsapp/components/Button/Base.vueapp/components/Compare/FacetSelector.vueapp/components/Compare/PackageSelector.vueapp/components/Compare/ReplacementSuggestion.vueapp/components/Filter/Panel.vueapp/components/Header/AuthModal.client.vueapp/components/Header/ConnectorModal.vueapp/components/Header/SearchBox.vueapp/components/Input/Base.stories.tsapp/components/Input/Base.vueapp/components/Link/Base.vueapp/components/Link/Link.stories.tsapp/components/Org/MembersPanel.vueapp/components/Org/OperationsQueue.vueapp/components/Org/TeamsPanel.vueapp/components/Package/Card.vueapp/components/Package/Header.vueapp/components/Package/Keywords.vueapp/components/Package/ListControls.vueapp/components/Package/Maintainers.vueapp/components/Package/MetricsBadges.vueapp/components/Package/TableRow.vueapp/components/Package/TrendsChart.vueapp/components/Package/Versions.vueapp/components/SearchProviderToggle.client.vueapp/components/Terminal/Install.vueapp/pages/compare.vueapp/pages/index.vueapp/pages/package/[[org]]/[name].vuetest/nuxt/a11y.spec.ts
💤 Files with no reviewable changes (6)
- app/components/Package/ListControls.vue
- app/components/Org/TeamsPanel.vue
- app/components/Package/Header.vue
- app/components/Header/ConnectorModal.vue
- app/components/Package/TrendsChart.vue
- app/components/Header/AuthModal.client.vue
🔗 Linked issue
Resolves #1254
Note
There is already one PR opened linked to the same issue but it seems to do more than expected and was not updated since one month (#1319) so I took the liberty to create this one, I hope that's fine
🧭 Context
The point of this PR is to align
sizeprop values to uno and nuxt/ui components.Updating
BaseButton,BaseInputandBaseLinkcomponents.📚 Description
BaseButtonsizeprop values to'sm' | 'md'BaseInputsizeprop values to'sm' | 'md' | 'lg'BaseLinksizeprop values to'sm' | 'md'mediumvalue is used