From 19643f4e13056e414aedd6b8ef89beb7ad1e95ab Mon Sep 17 00:00:00 2001 From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com> Date: Tue, 5 Mar 2024 16:09:16 -0600 Subject: [PATCH] Select placeholder (#1842) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request ## 🤨 Rationale - #350 --- ...-c05b4e64-fa32-40e9-b6a1-d7680dd0dbeb.json | 7 + .../src/list-option/index.ts | 23 +- .../src/list-option/styles.ts | 26 +- .../nimble-components/src/select/index.ts | 255 +++++++++++------- .../nimble-components/src/select/styles.ts | 7 +- .../nimble-components/src/select/template.ts | 8 +- .../src/select/testing/select.pageobject.ts | 50 +++- .../tests/select-opened-matrix.stories.ts | 23 +- .../select/tests/select.foundation.spec.ts | 18 -- .../src/select/tests/select.mdx | 2 +- .../src/select/tests/select.spec.ts | 206 ++++++++++++-- .../src/select/tests/select.stories.ts | 27 +- 12 files changed, 479 insertions(+), 173 deletions(-) create mode 100644 change/@ni-nimble-components-c05b4e64-fa32-40e9-b6a1-d7680dd0dbeb.json diff --git a/change/@ni-nimble-components-c05b4e64-fa32-40e9-b6a1-d7680dd0dbeb.json b/change/@ni-nimble-components-c05b4e64-fa32-40e9-b6a1-d7680dd0dbeb.json new file mode 100644 index 0000000000..1a431a2772 --- /dev/null +++ b/change/@ni-nimble-components-c05b4e64-fa32-40e9-b6a1-d7680dd0dbeb.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Adding placeholder behavior for Select", + "packageName": "@ni/nimble-components", + "email": "26874831+atmgrifter00@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/nimble-components/src/list-option/index.ts b/packages/nimble-components/src/list-option/index.ts index 2baf184613..724e564514 100644 --- a/packages/nimble-components/src/list-option/index.ts +++ b/packages/nimble-components/src/list-option/index.ts @@ -2,7 +2,7 @@ import { DesignSystem, ListboxOption as FoundationListboxOption } from '@microsoft/fast-foundation'; -import { observable } from '@microsoft/fast-element'; +import { observable, attr } from '@microsoft/fast-element'; import { styles } from './styles'; import { template } from './template'; @@ -19,6 +19,27 @@ export class ListOption extends FoundationListboxOption { /** @internal */ public contentSlot!: HTMLSlotElement; + /** + * The hidden state of the element. + * + * @public + * @defaultValue - false + * @remarks + * HTML Attribute: hidden + */ + @attr({ mode: 'boolean' }) + public override hidden = false; + + /** + * @internal + * This attribute is required to allow use-cases that offer dynamic filtering + * (like the Select) to visually hide options that are filtered out, but still + * allow users to use the native 'hidden' attribute without it being affected + * by the filtering process. + */ + @attr({ attribute: 'visually-hidden', mode: 'boolean' }) + public visuallyHidden = false; + /** @internal */ @observable public hasOverflow = false; diff --git a/packages/nimble-components/src/list-option/styles.ts b/packages/nimble-components/src/list-option/styles.ts index 6bcb9f23db..54d499448c 100644 --- a/packages/nimble-components/src/list-option/styles.ts +++ b/packages/nimble-components/src/list-option/styles.ts @@ -22,17 +22,6 @@ export const styles = css` height: ${controlHeight}; } - [part='start'] { - display: none; - } - - .content { - padding: 8px 4px; - white-space: nowrap; - overflow: hidden; - text-overflow: ellipsis; - } - :host([aria-selected='true']) { box-shadow: none; outline: none; @@ -69,6 +58,21 @@ export const styles = css` cursor: default; } + :host([visually-hidden]) { + display: none; + } + + [part='start'] { + display: none; + } + + .content { + padding: 8px 4px; + white-space: nowrap; + overflow: hidden; + text-overflow: ellipsis; + } + .content[disabled] { box-shadow: none; outline: none; diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts index c3e5d02aab..5370b1f06f 100644 --- a/packages/nimble-components/src/select/index.ts +++ b/packages/nimble-components/src/select/index.ts @@ -15,8 +15,7 @@ import { SelectPosition, applyMixins, StartEnd, - DelegatesARIASelect, - Listbox + DelegatesARIASelect } from '@microsoft/fast-foundation'; import { keyArrowDown, @@ -36,7 +35,7 @@ import { errorTextTemplate } from '../patterns/error/template'; import type { ErrorPattern } from '../patterns/error/types'; import { iconExclamationMarkTag } from '../icons/exclamation-mark'; import { template } from './template'; -import type { ListOption } from '../list-option'; +import { ListOption } from '../list-option'; import { FilterMode } from './types'; import { diacriticInsensitiveStringNormalizer } from '../utilities/models/string-normalizers'; import { FormAssociatedSelect } from './models/select-form-associated'; @@ -51,6 +50,10 @@ declare global { // eslint-disable-next-line @typescript-eslint/no-invalid-void-type type BooleanOrVoid = boolean | void; +const isNimbleListOption = (el: Element): el is ListOption => { + return el instanceof ListOption; +}; + /** * A nimble-styled HTML select. */ @@ -82,6 +85,12 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { @attr({ attribute: 'filter-mode' }) public filterMode: FilterMode = FilterMode.none; + /** + * @internal + */ + @observable + public displayPlaceholder = false; + /** * @internal */ @@ -156,7 +165,7 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { * @internal */ @observable - public committedSelectedOption: ListboxOption | undefined = undefined; + public committedSelectedOption?: ListboxOption; /** * The max height for the listbox when opened. @@ -189,26 +198,6 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { this.initializeOpenState(); } - /** - * The list of options. This mirrors FAST's override implementation for this - * member for the Combobox to support a filtered list in the dropdown. - * - * @public - * @remarks - * Overrides `Listbox.options`. - */ - public override get options(): ListboxOption[] { - Observable.track(this, 'options'); - return this.filteredOptions?.length - ? this.filteredOptions - : (this._options as ListOption[]); - } - - public override set options(value: ListboxOption[]) { - this._options = value; - Observable.notify(this, 'options'); - } - public override get value(): string { Observable.track(this, 'value'); return this._value; @@ -218,8 +207,6 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { const prev = this._value; let newValue = next; - // use 'options' here instead of '_options' as 'selectedIndex' may be relative - // to filtered set if (this.options?.length) { const newValueIndex = this.options.findIndex( el => el.value === newValue @@ -242,7 +229,7 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { this._value = newValue; super.valueChanged(prev, newValue); if (!this.open) { - this.committedSelectedOption = this._options.find( + this.committedSelectedOption = this.options.find( o => o.value === newValue ); } @@ -256,6 +243,7 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { /** * @internal */ + @volatile public get displayValue(): string { Observable.track(this, 'displayValue'); return this.committedSelectedOption?.text ?? ''; @@ -293,16 +281,20 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { next: Element[] ): void { const value = this.value; - this._options.forEach(o => { + this.options.forEach(o => { const notifier = Observable.getNotifier(o); notifier.unsubscribe(this, 'value'); + notifier.unsubscribe(this, 'hidden'); + notifier.unsubscribe(this, 'disabled'); }); super.slottedOptionsChanged(prev, next); - this._options.forEach(o => { + this.options.forEach(o => { const notifier = Observable.getNotifier(o); notifier.subscribe(this, 'value'); + notifier.subscribe(this, 'hidden'); + notifier.subscribe(this, 'disabled'); }); this.setProxyOptions(); this.updateValue(); @@ -329,10 +321,6 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { 'option,[role=option]' ); - if (!captured?.disabled) { - this.updateSelectedIndexFromFilteredSet(); - } - if (captured?.disabled) { return; } @@ -357,9 +345,34 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { * @override */ public override handleChange(source: unknown, propertyName: string): void { - super.handleChange(source, propertyName); - if (propertyName === 'value') { - this.updateValue(); + // don't call super.handleChange so hidden options can be selected programmatically + const sourceElement = source as Element; + switch (propertyName) { + case 'value': { + this.updateValue(); + break; + } + case 'selected': { + if (isNimbleListOption(sourceElement)) { + this.selectedIndex = this.options.indexOf(sourceElement); + } + this.setSelectedOptions(); + this.updateDisplayValue(); + break; + } + case 'hidden': { + if (isNimbleListOption(sourceElement)) { + sourceElement.visuallyHidden = sourceElement.hidden; + } + this.updateDisplayValue(); + break; + } + case 'disabled': { + this.updateDisplayValue(); + break; + } + default: + break; } } @@ -423,6 +436,16 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { * @internal */ public updateDisplayValue(): void { + if ( + this.committedSelectedOption?.disabled + && this.committedSelectedOption?.hidden + && this.committedSelectedOption?.selected + ) { + this.displayPlaceholder = true; + } else { + this.displayPlaceholder = false; + } + if (this.collapsible) { Observable.notify(this, 'displayValue'); } @@ -436,19 +459,10 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { */ public inputHandler(e: InputEvent): boolean { this.filter = this.filterInput?.value ?? ''; - if (!this.committedSelectedOption) { - this.committedSelectedOption = this._options.find( - option => option.selected - ); - } this.clearSelection(); this.filterOptions(); - if ( - this.filteredOptions.length > 0 - && this.committedSelectedOption - && !this.filteredOptions.includes(this.committedSelectedOption) - ) { + if (this.filteredOptions.length > 0) { const enabledOptions = this.filteredOptions.filter( o => !o.disabled ); @@ -475,7 +489,6 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { * @internal */ public override focusoutHandler(e: FocusEvent): BooleanOrVoid { - this.updateSelectedIndexFromFilteredSet(); super.focusoutHandler(e); if (!this.open) { return true; @@ -489,6 +502,10 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { if (!this.options?.includes(focusTarget as ListboxOption)) { this.open = false; + if (this.selectedIndex === -1) { + this.selectedIndex = this.indexWhenOpened!; + } + if (this.indexWhenOpened !== this.selectedIndex) { this.updateValue(true); } @@ -535,7 +552,6 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { ) { return false; } - this.updateSelectedIndexFromFilteredSet(); this.open = !this.open; if (!this.open) { this.focus(); @@ -543,23 +559,17 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { break; } case keyEscape: { - // clear filter as update to "selectedIndex" will result in processing - // "options" and not "_options" - this.filter = ''; - if (this.committedSelectedOption) { - this.clearSelection(); - this.selectedIndex = this._options.indexOf( - this.committedSelectedOption - ); + if (!this.open) { + break; } if (this.collapsible && this.open) { e.preventDefault(); this.open = false; } - // reset 'selected' state otherwise the selected state doesn't stick. - const selectedOption = this._options[this.selectedIndex]; - if (selectedOption) { - selectedOption.selected = true; + + if (this.selectedIndex !== this.indexWhenOpened!) { + this.options[this.selectedIndex]!.selected = false; + this.selectedIndex = this.indexWhenOpened!; } this.focus(); break; @@ -595,10 +605,14 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { * @internal */ public override selectedIndexChanged( - prev: number | undefined, - next: number + _: number | undefined, + __: number ): void { - super.selectedIndexChanged(prev, next); + // Don't call super.selectedIndexChanged as this will disallow disabled options + // from being valid initial selected values. Our setDefaultSelectedOption + // implementation handles skipping non-selected disabled options for the initial + // selected value. + this.setSelectedOptions(); this.updateValue(); } @@ -632,6 +646,28 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { } } + public override selectNextOption(): void { + // don't call super.selectNextOption as that relies on side-effecty + // behavior to not select disabled option (which no longer works) + for (let i = this.selectedIndex + 1; i < this.options.length; i++) { + if (!this.options[i]?.disabled) { + this.selectedIndex = i; + break; + } + } + } + + public override selectPreviousOption(): void { + // don't call super.selectPreviousOption as that relies on side-effecty + // behavior to not select disabled option (which no longer works) + for (let i = this.selectedIndex - 1; i >= 0; i--) { + if (!this.options[i]?.disabled) { + this.selectedIndex = i; + break; + } + } + } + // Prevents parent classes from resetting selectedIndex to a positive // value while filtering, which can result in a disabled option being // selected. @@ -711,11 +747,16 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { * @internal */ protected override selectedOptionsChanged( - prev: ListboxOption[] | undefined, + _prev: ListboxOption[] | undefined, next: ListboxOption[] ): void { - super.selectedOptionsChanged(prev, next); + // don't call super.selectedOptionsChanged so we don't filter out hidden elements + // when updating 'selected' state (copied relevant super implementation) this.options?.forEach((o, i) => { + const notifier = Observable.getNotifier(o); + notifier.unsubscribe(this, 'selected'); + o.selected = next.includes(o); + notifier.subscribe(this, 'selected'); const proxyOption = this.proxy?.options.item(i); if (proxyOption) { proxyOption.selected = o.selected; @@ -732,20 +773,38 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { */ protected override setDefaultSelectedOption(): void { const options: ListboxOption[] = this.options - ?? Array.from(this.children).filter(o => Listbox.slottedOptionFilter(o as HTMLElement)); - - const selectedIndex = options?.findIndex( - el => el.hasAttribute('selected') - || el.selected - || el.value === this.value - ); + ?? Array.from(this.children).filter(o => isNimbleListOption(o)); + + const optionIsSelected = (option: ListboxOption): boolean => { + return option.hasAttribute('selected') || option.selected; + }; + const optionIsDisabled = (option: ListboxOption): boolean => { + return option.hasAttribute('disabled') || option.disabled; + }; + let selectedIndex = -1; + let firstValidOptionIndex = -1; + for (let i = 0; i < options?.length; i++) { + const option = options[i]; + if (optionIsSelected(option!) || option?.value === this.value) { + selectedIndex = i; + } + if (firstValidOptionIndex === -1 && !optionIsDisabled(option!)) { + firstValidOptionIndex = i; + } + } if (selectedIndex !== -1) { this.selectedIndex = selectedIndex; - return; + } else if (firstValidOptionIndex !== -1) { + this.selectedIndex = firstValidOptionIndex; + } else { + this.selectedIndex = 0; } + this.committedSelectedOption = options[this.selectedIndex]; + } - this.selectedIndex = 0; + private committedSelectedOptionChanged(): void { + this.updateDisplayValue(); } private setPositioning(): void { @@ -785,17 +844,29 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { const filter = this.filter.toLowerCase(); if (filter) { - this.filteredOptions = this._options.filter(option => { - return diacriticInsensitiveStringNormalizer( - option.text - ).includes(diacriticInsensitiveStringNormalizer(filter)); + this.filteredOptions = this.options.filter(option => { + const normalizedFilter = diacriticInsensitiveStringNormalizer(filter); + return ( + !option.hidden + && diacriticInsensitiveStringNormalizer(option.text).includes( + normalizedFilter + ) + ); }); } else { - this.filteredOptions = this._options; + this.filteredOptions = this.options.filter( + option => !option.hidden + ); } - this._options.forEach(o => { - o.hidden = !this.filteredOptions.includes(o); + this.options.forEach(o => { + if (isNimbleListOption(o)) { + if (!this.filteredOptions.includes(o)) { + o.visuallyHidden = true; + } else { + o.visuallyHidden = false; + } + } }); } @@ -862,7 +933,7 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { return; } - this.committedSelectedOption = this._options[this.selectedIndex]; + this.committedSelectedOption = this.options[this.selectedIndex]; this.ariaControls = this.listboxId; this.ariaExpanded = 'true'; @@ -878,26 +949,6 @@ export class Select extends FormAssociatedSelect implements ErrorPattern { ); } } - - private updateSelectedIndexFromFilteredSet(): void { - const selectedItem = this.filteredOptions.length > 0 - ? this.options[this.selectedIndex] - ?? this.committedSelectedOption - : this.committedSelectedOption; - - if (!selectedItem) { - return; - } - // Clear filter so any logic resolving against 'this.options' resolves against all options, - // since selectedIndex should be relative to entire set. - this.filter = ''; - // translate selectedIndex for filtered list to selectedIndex for all items - this.selectedIndex = this._options.indexOf(selectedItem); - // force selected to true again if the selection hasn't actually changed - if (selectedItem === this.committedSelectedOption) { - selectedItem.selected = true; - } - } } const nimbleSelect = Select.compose({ diff --git a/packages/nimble-components/src/select/styles.ts b/packages/nimble-components/src/select/styles.ts index c6dabeac0e..d543d15819 100644 --- a/packages/nimble-components/src/select/styles.ts +++ b/packages/nimble-components/src/select/styles.ts @@ -25,7 +25,12 @@ export const styles = css` /* We are using flex `order` to define the visual ordering of the selected value, error icon, and dropdown arrow because they are not "interactive" i.e. part of the tab order */ '' } - [part='selected-value'] { + + .selected-value.placeholder { + color: ${placeholderFontColor}; + } + + .selected-value { order: 1; } diff --git a/packages/nimble-components/src/select/template.ts b/packages/nimble-components/src/select/template.ts index e5c2cdeb4e..3266934432 100644 --- a/packages/nimble-components/src/select/template.ts +++ b/packages/nimble-components/src/select/template.ts @@ -8,9 +8,9 @@ import { import { endSlotTemplate, FoundationElementTemplate, - Listbox, SelectOptions, - startSlotTemplate + startSlotTemplate, + isListboxOption } from '@microsoft/fast-foundation'; import type { Select } from '.'; import { anchoredRegionTag } from '../anchored-region'; @@ -62,7 +62,7 @@ SelectOptions > ${startSlotTemplate(context, definition)} -
(x.hasOverflow && x.displayValue ? x.displayValue : null)}> +
(x.hasOverflow && x.displayValue ? x.displayValue : null)}> ${x => x.displayValue}