Align form to architecture goals#188
Open
lifeiscontent wants to merge 2 commits into
Open
Conversation
|
📚 Storybook preview: https://pr-188-propel-storybook.vamsi-906.workers.dev |
Add a required `layout` variant ("single" | "multi") to the Form
component, reflecting the designer's spec (issue #130): vertical
field stacking is always the same, but layout axis is adjustable.
Removes the redundant flex/col/gap wrapper div from UI-tier stories
so children are direct Form descendants and the component's own
vertical rhythm is what governs spacing.
The form had its field-stacking and bottom-actions layout conflated on the single Form root, with the layout axis baked there. Split them into two single-element anatomy parts so each region owns its own chrome: - FormBody — the field stack. Carries the adjustable single/multi-column `layout` variant (issue #130's "depends" axis); uniform field gap is baked. - FormActions — the bottom actions bar. Position is always the same; the `variant` axis selects right-aligned (`inline`) vs full-width (`stretch`) per the spec. Form root now only governs the vertical rhythm between regions. Both stories compose the new parts and register them as subcomponents.
lifeiscontent
force-pushed
the
align/form
branch
from
00ca0af to
def9e0b
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 #130
What the designer spec says
From @bhaveshraja's comment on #130:
Always the same — vertical field stacking, consistent gap between fields, uniform vertical rhythm, error state behavior, required-field indicator style.
Depends (adjustable) — fields and their arrangement, layout (single column vs multi-column), submit button label/variant, secondary actions, validation behavior, inline vs sectioned layout.
Changes
layoutvariant added toformVariants(variants.ts)The "layout" axis (single vs multi-column) is now a required CVA variant with two values:
"single"—flex-col, fields stack vertically (the spec's "always the same" vertical rhythm)"multi"—flex-row flex-wrap, suitable for side-by-side field pairsNo
defaultVariants— callers must be explicit, per the architecture rule.FormPropsextended with requiredlayout(form.tsx)layoutis extracted and forwarded toformVariants({ layout }). TheclassName/styleomit is unchanged.Story fixes (ui/form.stories.tsx)
The UI-tier stories wrapped children in
<div className="flex w-80 flex-col gap-4">— this made the Form's own flex layout a no-op since fields weren't direct children. The wrapper now only carriesw-80(as a meta decorator for demo width), and fields are direct<Form>children.layout="single"added to story args.Components-tier story updated (components/form.stories.tsx)
ExampleFormpasseslayout="single"to<Form>and story objects supply it inargsto satisfy the required type.Notes
layout="single"(sameflex flex-col gap-4).