Variant-prop vocabulary pass: fix mis-named and dead axes#207
Open
lifeiscontent wants to merge 11 commits into
Open
Variant-prop vocabulary pass: fix mis-named and dead axes#207lifeiscontent wants to merge 11 commits into
lifeiscontent wants to merge 11 commits into
Conversation
…, no defaults) The public mode prop (TableCell/TableHead) was remapped onto a misnamed cva axis 'surface', so the parts couldn't use & VariantProps. Renamed the axis to mode (matching the public prop, the TableMode type, and the vocabulary: 'surface' is a host background it adapts to, which table/spreadsheet are not). Dropped the function-level pinned/padding defaults so every axis is required (first pass, no defaults); Props intersect the derived TableCell/TableHeadVariantProps (private). components cells read mode from context and forward the required pinned/padding; call sites pass them explicitly.
… 10, no defaults)
FieldLabel hand-wrote magnitude + an optional inset; Props now intersect FieldLabelVariantProps
(StrictVariantProps), making inset required like every first-pass axis. Local VariantProps config
renamed to FieldLabelVariantConfig; the bundle stays private. Call sites pass inset explicitly
(inset={false} for stacked labels; field-label-group keeps its computed inline-row inset).
|
📚 Storybook preview: https://pr-207-propel-storybook.vamsi-906.workers.dev |
…d/flat) 'surface' means a host background a control adapts to; the menu popup's two looks are its own elevation (a floating raised popup vs a flat one embedded in an OverlayPanel) — same misuse we fixed on Table. Matches toolbar's existing elevation (raised/flat).
linearProgressVariants only ever had layout: { linear }, a one-value variant — a required prop with no
choice. Folded the class into the base cva and removed the axis + the now-empty VariantProps.
… not colors) 'tone' is for color (neutral/danger/hues); the meter indicator's low/medium/high name the value's threshold range, which the cva maps to warning/accent/success fills. Renamed the axis, the MeterIndicatorLevel type, and the deriveIndicatorLevel helper.
density is comfortable/compact in every component (the doc's short/default/auto was aspirational). Added side (Drawer start/end) and visibility (ScrollArea auto/always) as first-class axes rather than force-fitting placement.
lifeiscontent
changed the title
…d, toast, tabs)
A ui part renders a single element + {children}; composition is the components tier's job (rule 1/2).
- field: FieldLabel and FieldItemContent each became a single-element ui slot; the required-marker and
label+description composition moved to new components/field ready-mades. Repointed the 4 field
consumers + required-passing call sites.
- toast: the solid-* glyph assets + the tone->glyph map moved to components/toast; ui/toast keeps only
the single-element styled status-icon slot (cva sizes/colors the svg child).
- tabs: the underline track+bar split into TabUnderlineBarTrack + TabUnderlineBar; the track-wraps-bar
nesting moved into components/tabs (the underline Tab).
… author render={<X/>})
ToolbarToggle (ui) rendered <Toolbar.Button render={<Toggle/>}/> — authoring a render-prop to compose
a second Base UI behavior is composition, which belongs in components. The ready-made now composes the
styled ui ToolbarButton + Toggle (and reads density from the Toolbar context); the ui-tier story
inlines that same ToolbarButton+Toggle directly (a ui story never imports from components).
…mport components
Clarify rule 1 — forwarding the consumer's render (useRender) is render-capability and fine, but
baking a render={<Specific/>} target is composition (components' job). Add rule 2b — ui/base stories
import only from ui/base + Base UI/external, never components; build a composition inline from atoms.
The error look is a STATE, not a prop. Base UI's Field.Root propagates data-invalid to the Checkbox.Root, so the box cva rests at the neutral border and recolors to danger off data-invalid:border-danger-strong — removing the tone axis from checkbox-box and the tone prop from Checkbox, CheckboxField, CheckboxGroupFieldOption (an invalid field now tones every option danger for free, no cascade). Verified end-to-end: play tests assert data-invalid reaches the checkbox box and the danger border renders. The menu checkbox indicator drops its tone=neutral arg (rests neutral).
lifeiscontent
force-pushed
the
fix/table-fieldlabel-variantprops
branch
from
3ec31bd to
2b9c7ad
Compare
Removed the tone axis from field-control-surface and every bordered control (input, text-area, select, combobox, autocomplete, number-field, otp). Danger now derives from Base UI's validity: the surface recolors off data-invalid (when it IS the focusable control: select trigger, otp digit) or :has([data-invalid]) (when it WRAPS one: input/textarea/etc.). Danger wins on focus (danger ring instead of accent). The *Field components derive invalid from error only; otp dropped its hand-rolled danger ring (the surface now supplies it). Renamed the danger stories Error->Invalid (a story named Error shadowed the global Error, forcing globalThis.Error). Verified via play tests that an invalid field renders the danger border.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A scrutiny of every cva axis across
uibefore propel gets adopted — renaming variant props is painful once people depend on them. All of these are API-shape fixes, no visual change.Table (TableCell/TableHead). Public
modewas remapped onto a cva axis calledsurface, which blocked a clean& VariantProps.surfacemeans "the host background a control adapts to" —table/spreadsheetis the table's own mode, so renamed the axis tomode. Also dropped the function-levelpinned/paddingdefaults so every axis is required (first pass, no defaults); the ready-made cells readmodefrom context, call sites passpinned/padding.FieldLabel.
insetwas optional with no default; now required viaFieldLabelVariantProps, passed explicitly at call sites.MenuPopup. Same
surfacemisuse as Table — the popup's two looks are its own elevation (a floating raised popup vs a flat one inside anOverlayPanel), not a host background. Renamedsurface→elevation(raised/inset→raised/flat), matching toolbar's existingelevation.LinearProgress.
linearProgressVariantshad a single-valuelayout: { linear }— a required prop with no choice. Folded into the base class, axis removed.Meter. The indicator's
tonewaslow·medium·high— those are value ranges, not colors. Renamed the axis tolevel(andMeterIndicatorTone→MeterIndicatorLevel); the cva still maps them to warning/accent/success fills.Docs.
densityiscomfortable·compacteverywhere (the table'sshort·default·autowas aspirational). Addedside(Drawerstart·end) andvisibility(ScrollAreaauto·always) as real axes rather than force-fittingplacement.Deliberately left for a follow-up (need per-component care, not clear renames): the boolean state axes (
checkbox.disabled,nav-item.open,pagination.current,breadcrumb.group) that restyle via a prop where adata-*selector might do; and whether progress'sbrandtone should beaccent.vp check+ the affected tests are green. Wants a design pass before merge.