-
Notifications
You must be signed in to change notification settings - Fork 29.3k
feat(forms): experimental prototype signal forms support for select[multiple] #68350
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: main
Are you sure you want to change the base?
Changes from all commits
fd2636e
fddfa29
950ab18
9d76e1b
05ec87f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| /** | ||
| * @license | ||
| * Copyright Google LLC All Rights Reserved. | ||
| * | ||
| * Use of this source code is governed by an MIT-style license that can be | ||
| * found in the LICENSE file at https://angular.dev/license | ||
| */ | ||
| import { | ||
| ɵRuntimeError as RuntimeError, | ||
| type ɵControlDirectiveHost as ControlDirectiveHost, | ||
| } from '@angular/core'; | ||
| import {RuntimeErrorCode} from '../errors'; | ||
| import { | ||
| applyControlStateBindings, | ||
| bindingUpdated, | ||
| createBindings, | ||
| type ControlBindingKey, | ||
| } from './bindings'; | ||
| import type {FormField} from './form_field'; | ||
| import {observeSelectMutations} from './select'; | ||
|
|
||
| export function selectMultipleControlCreate( | ||
| host: ControlDirectiveHost, | ||
| parent: FormField<unknown>, | ||
| ): () => void { | ||
| let updateMode = false; | ||
| const select = parent.nativeFormElement as HTMLSelectElement; | ||
|
|
||
| host.listenToDom('input', () => | ||
| parent.state().controlValue.set(getSelectMultipleControlValue(select)), | ||
| ); | ||
| host.listenToDom('blur', () => parent.state().markAsTouched()); | ||
|
|
||
| parent.registerAsBinding(); | ||
|
|
||
| observeSelectMutations( | ||
| select, | ||
| () => { | ||
| if (!updateMode) { | ||
| return; | ||
| } | ||
| setSelectMultipleControlValue(select, parent.state().controlValue()); | ||
| }, | ||
| parent.destroyRef, | ||
| ); | ||
|
|
||
| const bindings = createBindings<ControlBindingKey | 'controlValue'>(); | ||
|
|
||
| return () => { | ||
| if (ngDevMode && !updateMode && !select.multiple) { | ||
| throw new RuntimeError( | ||
| RuntimeErrorCode.DYNAMIC_SELECT_MULTIPLE_BINDING, | ||
| ngDevMode && | ||
| `Signal Forms does not support dynamic [multiple] bindings on <select>. ` + | ||
| `Use the static 'multiple' attribute instead.`, | ||
| ); | ||
| } | ||
|
|
||
| const state = parent.state(); | ||
| const controlValue = state.controlValue(); | ||
| if (bindingUpdated(bindings, 'controlValue', controlValue)) { | ||
| setSelectMultipleControlValue(select, controlValue); | ||
| } | ||
|
|
||
| applyControlStateBindings(bindings, state, host, parent); | ||
|
|
||
| updateMode = true; | ||
| }; | ||
| } | ||
|
|
||
| function getSelectMultipleControlValue(select: HTMLSelectElement): string[] { | ||
| const selected: string[] = []; | ||
| for (let i = 0; i < select.options.length; i++) { | ||
| const option = select.options[i]; | ||
| if (option.selected) { | ||
| selected.push(option.value); | ||
| } | ||
| } | ||
| return selected; | ||
| } | ||
|
Comment on lines
+71
to
+80
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not obvious why we need to cover both cases. Can you add a comment about that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a comment to the fallback branch explaining why it exists.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Angular doesn't use JSDOM but Domino. But it looks like this argument still hold. I couldn't find any trace of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good correction, thanks. It's Domino, not jsdom. I updated the comment to reflect that.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I second guessing this. If we have that 2nd part that checks for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
|
||
| function setSelectMultipleControlValue(select: HTMLSelectElement, value: unknown): void { | ||
| if (!Array.isArray(value)) { | ||
| throw new RuntimeError( | ||
| RuntimeErrorCode.SELECT_MULTIPLE_NON_ARRAY_VALUE, | ||
| ngDevMode && | ||
| `Expected an array value for select[multiple], but got ${typeof value}. ` + | ||
| `Bind a signal holding a string array: e.g. form(signal<string[]>([]))`, | ||
| ); | ||
| } | ||
| const selectedValues = new Set<string>(value); | ||
| for (let i = 0; i < select.options.length; i++) { | ||
| const option = select.options[i]; | ||
| option.selected = selectedValues.has(option.value); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,7 @@ import {bindingUpdated, type ControlBindingKey, createBindings} from './bindings | |
| import {customControlCreate} from './control_custom'; | ||
| import {cvaControlCreate} from './control_cva'; | ||
| import {nativeControlCreate} from './control_native'; | ||
| import {selectMultipleControlCreate} from './control_select_multiple'; | ||
| import { | ||
| isNativeFormElement, | ||
| isNumericFormElement, | ||
|
|
@@ -341,6 +342,12 @@ export class FormField<T> { | |
| this.ɵngControlUpdate = cvaControlCreate(host, this as FormField<unknown>); | ||
| } else if (host.customControl) { | ||
| this.ɵngControlUpdate = customControlCreate(host, this as FormField<unknown>); | ||
| } else if ( | ||
| this.elementIsNativeFormElement && | ||
| this.nativeFormElement.tagName === 'SELECT' && | ||
| (this.nativeFormElement as HTMLSelectElement).multiple | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same concern raised with the type check code above. If the element has a [multiple] property binding, it may not be set yet at this point. Do we want to delegate all <select> elements through a selectControlCreate(), which dynamically handles multiple? Alternatively we can prohibit a dynamic multiple binding if we prefer.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Went with the prohibit option. The check happens at the start of the first update cycle (after Angular has applied property bindings), so it catches the case where |
||
| ) { | ||
| this.ɵngControlUpdate = selectMultipleControlCreate(host, this as FormField<unknown>); | ||
| } else if (this.elementIsNativeFormElement) { | ||
| this.ɵngControlUpdate = nativeControlCreate( | ||
| host, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.