Repository navigation
refactor: rename variant and enum values to shared names - #978
rohanchkrabrty wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 14 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 45 files. (13 skipped… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly summarizes the main change: renaming variant and enum values to shared names across components. Description check The description directly matches the changeset and documents the renamed variants, props, scale values, affected internal users, tests, documentation, and validation results. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 45 files. (13 skipped: 13 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
commit: |
| justifyItems, | ||
| alignItems, | ||
| justifyContent, | ||
| align, |
There was a problem hiding this comment.
suggestion: alignItems and justifyContent aren't pulled out of props anymore, so they fall into ...props and reach the DOM.
I tried <Grid alignItems="center" justifyContent="space-between"> (how JS or an MDX demo would still pass it): the div gets alignitems and justifycontent attributes, React warns about unknown props, and the alignment is silently lost.
Could we keep them as @deprecated aliases for a release (like #977 does for Sidebar position), or at least drop them before the spread? toCssAlignment already passes space-* through, so justify ?? justifyContent would keep old values working.
| from you. The full record of every release, features and fixes included, is | ||
| on [GitHub releases](https://github.com/raystack/apsara/releases). | ||
|
|
||
| ## Unreleased: variant and enum values are renamed |
There was a problem hiding this comment.
suggestion: packages/react/CHANGELOG.md keeps an "Unreleased" list with a (BREAKING) entry for each breaking change (#919 added one next to its Upgrading section). Could we add one for these renames too, so the changelog stays complete for the release?
|
|
||
| ## Unreleased: variant and enum values are renamed | ||
|
|
||
| Prop values are kebab-case, and a variant uses the same name as on other |
There was a problem hiding this comment.
nit: "Prop values are kebab-case" reads as true for the whole library, but Flex direction (rowReverse) and wrap (noWrap) are still camelCase, as the PR description notes. Could we scope the sentence to the values in this table?
| Prop values are kebab-case, and a variant uses the same name as on other | |
| The values in this table are kebab-case, and a variant uses the same name as on other |
|
|
||
| const variant = figma.selectedInstance.getEnum('Variant', { | ||
| Filled: 'filled' | ||
| Filled: 'solid' |
There was a problem hiding this comment.
question: Figma's Chip "Variant" option is still called "Filled", so this mapping is now the only place the two names meet. Is a design-side rename to "Solid" planned? It would keep designers and code on the same word. EmptyState's "Empty"/"Zero" vs compact/page is the same story.
Summary
variantvalues to the names other components use:Input,TextAreaandSearchtakeoutlineandtext(weredefaultandborderless),FilterChiptakesoutline(wasdefault), andChiptakessolid(wasfilled).EmptyStatevariants after their layouts:empty1iscompactandempty2ispage.Gridtakesalignandjustifyin place ofalignItemsandjustifyContent, asFlexdoes.justifyandalignContenttakebetween,aroundandevenlyin place of thespace-*values.alignContentandjustifyItemskeep their names.CalendarPreviewnames the half-year scalehalf-year(washalfYear) inscales,scale,defaultScale,onScaleChange, value objects anddata-scale.Decisions
EmptyStatenames:compactis the centered layout with stacked actions for panels and lists.pageis the full-page layout with left-aligned content, a larger icon and actions in a row. The docs already describe them this way, and the CSS calls the second oneemptyStatePage.<prop>-<value>: FilterChipchip-defaultisvariant-outline, and Chip's private--chip-filled-*variables are--chip-solid-*. Computed styles do not change.variantrow it was missing.docs/V1-migration.mdanddocs/rfcs/005-calendar-preview.mdkeep the old names, because they describe earlier releases.direction(rowReverse,columnReverse) andwrap(noWrap,wrapReverse) values are camelCase, but the proposal does not list them, so they do not change here.Checks
pnpm lint,pnpm exec tsc --noEmitinpackages/react,pnpm test:apsara(3724 tests) andpnpm build:apsarapass.Compared the docs pages against apsara.raystack.org: Input, TextArea, Search, FilterChip, Chip, EmptyState, Grid, CalendarPreview, Command, Editor and PromptInput in light, and TextArea, Search, FilterChip, Chip, EmptyState and Grid in dark. The computed styles of every demo match. The Chip, TextArea, FilterChip, EmptyState and Grid playgrounds render the new values with the same styles as the old ones. The Upgrading page renders.
I have followed the pull request section of the contributing guide.