Align field to architecture goals#195
Merged
Merged
Conversation
|
📚 Storybook preview: https://pr-195-propel-storybook.vamsi-906.workers.dev |
- Convert fieldDescriptionVariants and fieldErrorVariants from plain cx-wrapper functions to proper cva instances, so className composition lives entirely in cva - Convert inputFieldBoxVariants and textAreaFieldBoxVariants from cx-wrapper functions to standalone cva instances (tone + magnitude variants inlined); remove the now-dead fieldBoxVariants base - Extract fieldRootVariants cva for Field.Root base class; call it from field.tsx instead of the bare inline string - Rename iconSlotClass (plain cx constant) to iconSlotVariants (cva) and update the import in InputFieldIconSlot - Replace the anonymous inline-className div in TextAreaField with InputFieldContent orientation="vertical", eliminating the last raw className in the components tier Closes #128
FieldLabel was rendering its required asterisk as a bare second element with an inline className, so the part owned two elements and baked styling in. Pull the asterisk into a FieldLabelRequiredMarker part (single span, its own cva), keeping FieldLabel a single Base UI Label that composes the marker when required. Move the remaining inline classNames in the field ui tier into cva: the choice-option label/description column (fieldItemContentVariants) and the inline icon slot, which now reuses the shared node-slot class instead of baking a child size.
fieldRootVariants → fieldVariants (part: Field) iconSlotVariants → inputFieldIconSlotVariants (part: InputFieldIconSlot) labelGroupVariants → fieldLabelGroupVariants (part: FieldLabelGroup)
FieldLabelRequiredMarker no longer bakes the asterisk via children ?? "*"; it renders only its children. FieldLabel now passes the * glyph in when required, and the UI story demonstrates the bare slot with an explicit glyph.
lifeiscontent
force-pushed
the
align/field
branch
from
fab16e3 to
a634444
Compare
FieldHelperText composes two ui parts (FieldError + FieldDescription) with the error-XOR-hint rule — it is a composition, not a single-element ui part, so it belongs in components/field/ (where its 9 consumers already live), not ui/field/.
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 #128
What the designer's spec says
From the issue comment, the things that are always the same (baked in):
And the things that depend on usage (adjustable props):
The component already honours this split well. These changes tighten the implementation layer without touching the public API.
Architecture changes
variants.tsfieldDescriptionVariantsandfieldErrorVariantswere plain functions that calledcx("text-tertiary", helperVariants({ magnitude })). Converted both to propercvainstances so all className composition lives in cva, not in ad-hoc wrapper functions.inputFieldBoxVariantsandtextAreaFieldBoxVariantswere plain functions that calledcx(fieldBoxVariants(...), frameVariants(...)). Converted both to standalonecvainstances withtoneandmagnitudevariants inlined. The now-deadfieldBoxVariantsbase (which was only used as a private building block for those two functions) is removed.iconSlotClasswas a plaincx(...)constant. Renamed toiconSlotVariantsand converted tocva.fieldRootVariantscva for theField.Rootbase class (was a bare string literal in the component file).field.tsxfieldRootVariants()instead of inlining"flex flex-col gap-1.5"directly on the element.input-field-icon-slot.tsxiconSlotClasstoiconSlotVariantsand callsiconSlotVariants().text-area-field.tsx(components tier)<div className="flex w-full flex-col gap-1.5">. Replaced with<InputFieldContent orientation="vertical">, which renders the same classes via its cva variant and reuses the existing ui part rather than a bare div with inline style.Notes
vp check --no-fmtpasses on all 39 affected files.