- 
                Notifications
    You must be signed in to change notification settings 
- Fork 34
feat(#3134): update Form item component for V2 #3139
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v2-2998-coded-component-updates
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I'm not sure about the addition of the inputType property. 🤔
| type RequirementType = (typeof REQUIREMENT_TYPES)[number]; | ||
| type LabelSizeType = (typeof LABEL_SIZE_TYPES)[number]; | ||
| type VersionType = (typeof Version)[number]; | ||
| type InputTypeType = (typeof INPUT_TYPES)[number]; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| type InputTypeType = (typeof INPUT_TYPES)[number]; | |
| type InputType = (typeof INPUT_TYPES)[number]; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can call this InputType like we do elsewhere:
ui-components/libs/common/src/lib/common.ts
Lines 327 to 328 in f245d71
| export type GoabInputType = | |
| | "text" | 
| export let requirement: RequirementType = ""; | ||
| export let maxwidth: string = "none"; | ||
| export let version: VersionType = "1"; | ||
| export let inputType: InputTypeType = ""; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| export let inputType: InputTypeType = ""; | |
| export let type: InputType = ""; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious of how many teams will properly apply this input type property. I suspect it will be low for the following reasons:
- It requires a dev to know that the property exists and should be applied correctly. "I added a radio group to this form item so I need to set this to radio group."
- The difference is subltle and easy for a dev to overlook
I think a better approach would be to detect the type of the slotted input then add a class accordingly.
<!-- HTML -->
<div
  class:radio-group={hasRadioGroup}
  class:checkbox-list={hasCheckboxList}
  class:v2={version === "2"}
But that feels like an enhancement beyond the scope of our immediate work.
| <!-- HTML --> | ||
| <div | ||
| class:v2={version === "2"} | ||
| class={`${labelsize}${inputType ? ' ' + inputType : ''}`} | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| class={`${labelsize}${inputType ? ' ' + inputType : ''}`} | |
| class={`${labelsize}${type ? ' ' + type : ''}`} | 
| validateRequirementType(requirement); | ||
| validateLabelSize(labelsize); | ||
| validateVersion(version); | ||
| validateInputType(inputType); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| validateInputType(inputType); | |
| validateType(type); | 
| false, | ||
| ); | ||
| const [Version, validateVersion] = typeValidator("Version", ["1", "2"]); | ||
| const [INPUT_TYPES, validateInputType] = typeValidator( | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| const [INPUT_TYPES, validateInputType] = typeValidator( | |
| const [INPUT_TYPES, validateType] = typeValidator( | 
Summary
Updates the Form Item component to support V2 design system styling while
maintaining full backward compatibility with V1. Introduces compact size
variant, input-type-aware message spacing, and refined error message
structure.
Changes
New Props
version("1" | "2", default"1"): Controls V1 vs V2 stylinginputType("" | "text-input" | "textarea" | "checkbox-list" | "radio-group", default""): Enables input-type-specific message spacing in V2New Size Variant
labelsizenow accepts"compact"in addition to"regular"and"large"V2 Styling Updates
Label Typography & Spacing
padding
Message Container
Message Spacing from Input:
Error Message Structure
Message Stack
Technical Implementation
Input-Type Awareness
inputTypeprop enables CSS to apply different spacing based on controltype
default
.large.checkbox-list .messages-containeruses larger spacingthan
.large.text-inputV2 CSS Scoping
.v2class selector.v2.compact)Messages Container Pattern
Related