spartan
spartan copied to clipboard
fix: Add support for wrapped form controls
PR Checklist
Please check if your PR fulfills the following requirements:
- [ ] The commit message follows our guidelines: https://github.com/spartan-ng/spartan/blob/main/CONTRIBUTING.md#-commit-message-guidelines
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been added / updated (for bug fixes / features)
PR Type
What kind of change does this PR introduce?
- [ ] Bugfix
- [x] Feature
- [ ] Code style update (formatting, local variables)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] CI related changes
- [ ] Documentation content changes
- [ ] Other... Please describe:
Which package are you modifying?
Primitives
- [ ] accordion
- [ ] alert
- [ ] alert-dialog
- [ ] aspect-ratio
- [ ] autocomplete
- [ ] avatar
- [ ] badge
- [ ] breadcrumb
- [ ] button
- [ ] button-group
- [ ] calendar
- [ ] card
- [ ] carousel
- [ ] checkbox
- [ ] collapsible
- [ ] combobox
- [ ] command
- [ ] context-menu
- [ ] data-table
- [ ] date-picker
- [ ] dialog
- [ ] empty
- [ ] dropdown-menu
- [ ] field
- [x] form-field
- [ ] hover-card
- [ ] icon
- [ ] input
- [ ] input-group
- [ ] input-otp
- [ ] item
- [ ] kbd
- [ ] label
- [ ] menubar
- [ ] navigation-menu
- [ ] pagination
- [ ] popover
- [ ] progress
- [ ] radio-group
- [ ] resizable
- [ ] scroll-area
- [ ] select
- [ ] separator
- [ ] sheet
- [ ] sidebar
- [ ] skeleton
- [ ] slider
- [ ] sonner
- [ ] spinner
- [ ] switch
- [ ] table
- [ ] tabs
- [ ] textarea
- [ ] toggle
- [ ] toggle-group
- [ ] tooltip
- [ ] typography
Others
- [ ] trpc
- [ ] nx
- [ ] repo
- [ ] cli
What is the current behavior?
The hlm-form-field component only supports BrnFormFieldControl (descendant controls). It requires an explicit control directive to be present and doesn't work with wrapped BrnFormFieldControls or standard Angular reactive forms using formControl or formControlName directives.
I usually wrap things into components for re-usability:
@Component({
selector: 'app-select',
imports: [ReactiveFormsModule, HlmSelectImports, BrnSelectImports],
template: `
<hlm-select class="inline-block w-full" placeholder="Select values" [formControl]="formControl">
<hlm-select-trigger class="w-full">
<hlm-select-value />
</hlm-select-trigger>
<hlm-select-content class="max-h-96">
<hlm-select-scroll-up />
@for (obj of getSortedOptions(items()); track obj.item; let i = $index) {
<hlm-option [value]="obj.item">{{ obj.label }}</hlm-option>
}
<hlm-select-scroll-down />
</hlm-select-content>
</hlm-select>
`,
providers: [
{
provide: NG_VALUE_ACCESSOR,
useExisting: forwardRef(() => Select),
multi: true,
},
],
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class Select<T> implements ControlValueAccessor {
...
}
<hlm-form-field>
<label hlmLabel for="input-{{ pair.key }}"
><span hlmBadge variant="default">{{ pair.key }}</span></label
>
<app-select
[items]="toOptions(secrets()[pair.key].options)"
[formControl]="pair.value"
id="input-{{ pair.key }}"
/>
</hlm-form-field>
Currently, HlmFormField doesn't work with this.
Closes #
What is the new behavior?
The hlm-form-field component now supports both:
- BrnFormFieldControl (existing behavior)
- Wrapped
BrnFormFieldControls - Angular NgControl (formControl/formControlName) directives
Does this PR introduce a breaking change?
- [ ] Yes
- [x] No
Other information
This enhancement improves flexibility by allowing developers to use standard Angular reactive forms patterns with the form-field component, without requiring the BrnFormFieldControl directive. This means BrnFormFieldControls wrapped in a component.
Hey @dolanmiu!
Thanks for your PR!
The Form primitives are currently one of the part of the project that we definitely want to improve before releasing v1.
Here is another PR that tries tackling this: https://github.com/spartan-ng/spartan/pull/911 There's also been some discussion in our discord here: https://discord.com/channels/1145362148621557921/1145362149133271082/1437880161474187314
One thing that's also relevant to this PR and might need some better instructions in the contribution guide: I only see your changes in the .template files - these are actually auto generated through the following generator: nx g @spartan-ng/tools:hlm-to-cli-generator
The changes are actually coming from the libs/helm project.
Please let me know if this is helpful!
Thanks for the quick reply! Good to know it's being worked on. Can close this then?
@dolanmiu let's keep this for now as a reference, because I absolutely believe this should be supported!