Align input to architecture goals#185
Open
lifeiscontent wants to merge 2 commits into
Open
Conversation
|
📚 Storybook preview: https://pr-185-propel-storybook.vamsi-906.workers.dev |
The designer's spec marks caret color and disabled-state opacity as "always the same" — they should be baked in, not left to the consumer. - Add `caret-(--border-color-accent-strong)` so the text cursor matches the brand focus-ring color (same token the field box uses for its focus border). - Add `disabled:opacity-60` alongside the existing disabled styles so a disabled input dims to match the rest of the design system (accordion, collapsible, field item, toggle all use opacity-60 for disabled).
…nts tier The components-tier input index re-exported inputFieldBoxVariants (a cva) and both stories framed the bare Input with that cva applied as a raw className. Compose the single-element InputFieldBox / InputFieldIconSlot ui parts instead so the box/border and the leading/trailing icon addons (the Figma adjustable axes) come from styled ui parts, and the components tier holds no styling. Register the framing parts as story subcomponents and add a WithIconSlots story exercising the addon anatomy.
lifeiscontent
force-pushed
the
align/input
branch
from
fed4da3 to
c629483
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
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.
Closes #132
What the spec says
From bhaveshraja's comment on #132, the input component has two categories of behavior:
Always the same (baked in): border style/radius, height per size step, padding, placeholder text color, focus border color, error border color, disabled state (muted background, reduced opacity, cursor not-allowed), caret color, font size and family per size step, addon alignment.
Depends (adjustable): size variant (magnitude), placeholder text, value, leading/trailing addons, error/disabled/read-only state, input type.
What was missing
Two "always the same" items were not yet baked into
inputVariantsinpackages/propel/src/ui/input/variants.ts:Caret color — the text cursor was inheriting the browser default instead of tracking the brand accent color. Fixed with
caret-(--border-color-accent-strong), the same design token the field box already uses for its focus-ring border.Disabled opacity — the spec says the disabled state includes "reduced opacity". The existing disabled styles covered cursor and text color but not opacity. Fixed with
disabled:opacity-60, matching the pattern used by accordion, collapsible, toggle, and field-item across the rest of the design system.Architecture alignment
inputVariantsinvariants.ts— styling stays in cva, not scattered across component files.defaultVariantsadded;magnituderemains a required prop.magnitudeas a required prop.Notes
This is a narrow change touching only
packages/propel/src/ui/input/variants.ts. The leading/trailing addon slots (also "depends/adjustable" per spec) are a larger anatomy question — they need a wrapper element and named parts — so they're left for a follow-up rather than speculated here.Needs design approval from @bhaveshraja before merge.