-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(aria/combobox): dialog popup support #32279
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
Changes from all commits
d5b330c
f005662
aa8a021
9e7732a
3195dbf
5b500c7
0d57b47
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 |
|---|---|---|
|
|
@@ -25,6 +25,7 @@ import { | |
| ComboboxPattern, | ||
| ComboboxListboxControls, | ||
| ComboboxTreeControls, | ||
| ComboboxDialogPattern, | ||
| } from '@angular/aria/private'; | ||
| import {Directionality} from '@angular/cdk/bidi'; | ||
| import {toSignal} from '@angular/core/rxjs-interop'; | ||
|
|
@@ -84,7 +85,12 @@ export class Combobox<V> { | |
| readonly firstMatch = input<V | undefined>(undefined); | ||
|
|
||
| /** Whether the combobox is expanded. */ | ||
| readonly expanded = computed(() => this._pattern.expanded()); | ||
| readonly expanded = computed(() => this.alwaysExpanded() || this._pattern.expanded()); | ||
|
|
||
| // TODO: Maybe make expanded a signal that can be passed in? | ||
| // Or an "always expanded" option? | ||
|
|
||
| readonly alwaysExpanded = input(false); | ||
|
|
||
| /** Input element connected to the combobox, if any. */ | ||
| readonly inputElement = computed(() => this._pattern.inputs.inputEl()); | ||
|
|
@@ -103,7 +109,16 @@ export class Combobox<V> { | |
|
|
||
| constructor() { | ||
| afterRenderEffect(() => { | ||
| if (!this._deferredContentAware?.contentVisible() && this._pattern.isFocused()) { | ||
| if (this.alwaysExpanded()) { | ||
| this._pattern.expanded.set(true); | ||
| } | ||
| }); | ||
|
|
||
| afterRenderEffect(() => { | ||
| if ( | ||
| !this._deferredContentAware?.contentVisible() && | ||
| (this._pattern.isFocused() || this.alwaysExpanded()) | ||
| ) { | ||
| this._deferredContentAware?.contentVisible.set(true); | ||
| } | ||
| }); | ||
|
|
@@ -146,10 +161,15 @@ export class ComboboxInput { | |
| ); | ||
| this.combobox._pattern.inputs.inputValue = this.value; | ||
|
|
||
| const controls = this.combobox.popup()?.controls(); | ||
| if (controls instanceof ComboboxDialogPattern) { | ||
| return; | ||
| } | ||
|
|
||
| /** Focuses & selects the first item in the combobox if the user changes the input value. */ | ||
| afterRenderEffect(() => { | ||
| this.value(); | ||
| this.combobox.popup()?.controls()?.items(); | ||
| controls?.items(); | ||
| untracked(() => this.combobox._pattern.onFilter()); | ||
| }); | ||
| } | ||
|
|
@@ -172,6 +192,58 @@ export class ComboboxPopup<V> { | |
|
|
||
| /** The controls the popup exposes to the combobox. */ | ||
| readonly controls = signal< | ||
| ComboboxListboxControls<any, V> | ComboboxTreeControls<any, V> | undefined | ||
| | ComboboxListboxControls<any, V> | ||
| | ComboboxTreeControls<any, V> | ||
| | ComboboxDialogPattern | ||
| | undefined | ||
| >(undefined); | ||
| } | ||
|
|
||
| @Directive({ | ||
| selector: 'dialog[ngComboboxDialog]', | ||
| exportAs: 'ngComboboxDialog', | ||
| host: { | ||
| '[attr.data-open]': 'combobox._pattern.expanded()', | ||
| '(keydown)': '_pattern.onKeydown($event)', | ||
| '(click)': '_pattern.onClick($event)', | ||
| }, | ||
| hostDirectives: [ComboboxPopup], | ||
| }) | ||
| export class ComboboxDialog { | ||
| /** The dialog element. */ | ||
| readonly element = inject(ElementRef<HTMLDialogElement>); | ||
|
|
||
| /** The combobox that the dialog belongs to. */ | ||
| readonly combobox = inject(Combobox); | ||
|
|
||
| /** A reference to the parent combobox popup, if one exists. */ | ||
| private readonly _popup = inject<ComboboxPopup<unknown>>(ComboboxPopup, { | ||
| optional: true, | ||
| }); | ||
|
|
||
| _pattern: ComboboxDialogPattern; | ||
|
|
||
| constructor() { | ||
| this._pattern = new ComboboxDialogPattern({ | ||
| id: () => '', | ||
| element: () => this.element.nativeElement, | ||
| combobox: this.combobox._pattern, | ||
| }); | ||
|
|
||
| if (this._popup) { | ||
| this._popup.controls.set(this._pattern); | ||
| } | ||
|
|
||
| afterRenderEffect(() => { | ||
| if (this.element) { | ||
| this.combobox._pattern.expanded() | ||
|
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 think that by default native dialogs can be closed by clicking outside. I think we might end up in a situation where the
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. This is handled by the ComboboxDialogPattern, but in an unintuitive way. I went back and added a comment to clarify. The ComboboxDialogPattern has an onClick listener that handles this case. If the user clicks outside of the dialog, the click event fires on the dialog element. |
||
| ? this.element.nativeElement.showModal() | ||
| : this.element.nativeElement.close(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| close() { | ||
| this._popup?.combobox?._pattern.close(); | ||
| } | ||
| } | ||
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.
Is the dialog meant to be positioned relative to a trigger element or will it always be centered on the page?
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.
The dialog can either be centered or have special positioning depending on the use case. The positioning of the dialog is left up to developers