Align meter to architecture goals#186
Merged
Merged
Conversation
|
📚 Storybook preview: https://pr-186-propel-storybook.vamsi-906.workers.dev |
- Add tone variant (low/warning · medium/accent · high/success) to meterIndicatorVariants; remove the hardcoded bg-accent-primary class. No defaultVariants — callers supply tone explicitly. - Make MeterIndicator accept a required `tone` prop and pass it to the cva variant. Export MeterIndicatorTone from both ui and components indexes. - Components-tier Meter gains low/high/optimum threshold props and a deriveIndicatorTone helper that mirrors native <meter> semantics: which segment is "best" is driven by optimum; the indicator fill follows automatically (warning / accent / success). - Add showValue prop (default true) so callers can hide the formatted value text without losing aria-valuenow on the root. - Update ui-tier and components-tier stories: pass tone="medium" in ui stories; add NoValue and Thresholds stories in the components tier.
…tier The components-tier composition stacked MeterLabel above the track and MeterValue below it, relying on the root's flex-col. Per the Meter design spec the label and value sit together on a row above the bar in a fixed position relationship, so that row is a missing anatomy part. Add MeterHeader (an anatomy extension beyond Base UI's parts) that owns the label/value row layout (flex, justify-between, items-center). The components tier now composes Header › Label + Value, then Track › Indicator, with no raw class names. Each ui part still renders exactly one element.
7e6dea0 to
1aa9446
Compare
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
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.
Closes #135
What changed
The meter component wasn't reflecting the designer's color-breakpoint spec and had a few architecture gaps. This PR tightens both.
Designer spec recap
Always the same (baked in):
Adjustable (now exposed as props):
value,min,max(already on root via Base UI)low,high,optimumthresholds (new on components-tier Meter)labeltext (already optional on components-tier)showValue— whether the formatted value text is shown (new)Architecture changes
ui/meter/variants.tsbg-accent-primaryinmeterIndicatorVariantswith atonecva variant:low→bg-warning-primary,medium→bg-accent-primary,high→bg-success-primary. NodefaultVariants— the caller must supplytone.ui/meter/meter-indicator.tsxtone: MeterIndicatorToneprop. Passes it straight to the cva call; nocxin the component file.MeterIndicatorTonefrom the ui and components indexes.components/meter/meter.tsxlow,high,optimumprops (mirrors native<meter>semantics).showValueprop (defaulttrue) — whenfalse,MeterValueis not rendered; aria-valuenow on the root still announces the value to assistive tech.deriveIndicatorTonehelper that computes which of the three cva tones to use from the current value and thresholds: theoptimumposition determines which segment is "best", and the indicator fill follows automatically.Stories
tone="medium"onMeterIndicator(required prop).NoValue(showValue=false) andThresholds(low/high/optimum with three fill levels) stories.Notes
Needs design review and approval from @bhaveshraja before merge.