From 799d39216faa9ad98c94627c6505439c846cbac9 Mon Sep 17 00:00:00 2001 From: Fred Visser <1458528+fredvisser@users.noreply.github.com> Date: Tue, 7 May 2024 08:00:55 -0500 Subject: [PATCH 1/6] Control Minimum Width token applied to Select and Combobox (#2073) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request ## ๐Ÿคจ Rationale The `sl-details-panel` restyling recently went in, and some apps use `nimble-select` and `nimble-combobox` controls in their details-panels. These dropdown controls were set with `min-width:250px`, and couldn't resize correctly down to 176px. Before: ![image](https://github.com/ni/nimble/assets/1458528/bbfe44de-761e-4641-bf96-800fb15a1f1f) After: ![image](https://github.com/ni/nimble/assets/1458528/89b2af79-547e-4ad0-8b6f-cff3a89d0a85) ## ๐Ÿ‘ฉโ€๐Ÿ’ป Implementation - Set the dropdown min-width to the menuMinWidth token (176px) - Updated Select and Combobox stories, so those instances didn't change sizes ## ๐Ÿงช Testing - Manual testing of CSS changes with Work Orders app - Storybook review - Relying on Chromatic ## โœ… Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed. --- ...-components-a5b4c275-28d7-4509-af45-748fdd2f69fb.json | 7 +++++++ .../nimble-components/src/patterns/dropdown/styles.ts | 3 ++- packages/storybook/chromatic.config.json | 3 ++- .../src/nimble/combobox/combobox-matrix.stories.ts | 7 +++---- .../nimble/combobox/combobox-opened-in-div.stories.ts | 2 +- .../nimble/combobox/combobox-opened-matrix.stories.ts | 2 +- .../storybook/src/nimble/combobox/combobox.stories.ts | 3 +-- .../storybook/src/nimble/select/select-matrix.stories.ts | 9 ++++----- .../src/nimble/select/select-opened-in-div.stories.ts | 2 +- .../src/nimble/select/select-opened-matrix.stories.ts | 2 +- packages/storybook/src/nimble/select/select.stories.ts | 3 +-- 11 files changed, 24 insertions(+), 19 deletions(-) create mode 100644 change/@ni-nimble-components-a5b4c275-28d7-4509-af45-748fdd2f69fb.json diff --git a/change/@ni-nimble-components-a5b4c275-28d7-4509-af45-748fdd2f69fb.json b/change/@ni-nimble-components-a5b4c275-28d7-4509-af45-748fdd2f69fb.json new file mode 100644 index 0000000000..0fee042299 --- /dev/null +++ b/change/@ni-nimble-components-a5b4c275-28d7-4509-af45-748fdd2f69fb.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Set Select and Combobox min-width to menuMinWidth token (176px)", + "packageName": "@ni/nimble-components", + "email": "1458528+fredvisser@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/nimble-components/src/patterns/dropdown/styles.ts b/packages/nimble-components/src/patterns/dropdown/styles.ts index 194bb55ad3..51af755599 100644 --- a/packages/nimble-components/src/patterns/dropdown/styles.ts +++ b/packages/nimble-components/src/patterns/dropdown/styles.ts @@ -10,6 +10,7 @@ import { borderWidth, controlHeight, iconSize, + menuMinWidth, popupBorderColor, smallDelay, smallPadding, @@ -37,7 +38,7 @@ export const styles = css` position: relative; justify-content: center; ${userSelectNone} - min-width: 250px; + min-width: ${menuMinWidth}; outline: none; vertical-align: top; --ni-private-hover-indicator-width: calc(${borderWidth} + 1px); diff --git a/packages/storybook/chromatic.config.json b/packages/storybook/chromatic.config.json index d7aeb0afc1..970461b6b3 100644 --- a/packages/storybook/chromatic.config.json +++ b/packages/storybook/chromatic.config.json @@ -1,4 +1,5 @@ { "projectId": "Project:60e89457a987cf003efc0a5b", - "storybookBaseDir": "./packages/nimble-components" + "storybookBaseDir": "packages/storybook", + "buildScriptName": "build" } diff --git a/packages/storybook/src/nimble/combobox/combobox-matrix.stories.ts b/packages/storybook/src/nimble/combobox/combobox-matrix.stories.ts index 113f3feb36..175a29690c 100644 --- a/packages/storybook/src/nimble/combobox/combobox-matrix.stories.ts +++ b/packages/storybook/src/nimble/combobox/combobox-matrix.stories.ts @@ -3,7 +3,6 @@ import { html, ViewTemplate } from '@microsoft/fast-element'; import { controlLabelFont, controlLabelFontColor, - menuMinWidth, standardPadding } from '@ni/nimble-components/dist/esm/theme-provider/design-tokens'; import { listOptionTag } from '@ni/nimble-components/dist/esm/list-option'; @@ -74,7 +73,7 @@ const component = ( error-text="${() => errorText}" value="${() => value}" placeholder="${() => placeholder}" - style="width: var(${menuMinWidth.cssCustomProperty});" + style="width: 250px;" > <${listOptionTag} value="1">Option 1 <${listOptionTag} value="2" disabled>Option 2 @@ -95,14 +94,14 @@ export const comboboxThemeMatrix: StoryFn = createMatrixThemeStory( export const hiddenCombobox: StoryFn = createStory( hiddenWrapper( - html`<${comboboxTag} hidden> + html`<${comboboxTag} hidden style="width: 250px;"> <${listOptionTag} value="1">Option 1 ` ) ); export const blankListOption: StoryFn = createStory( - html`<${comboboxTag} open> + html`<${comboboxTag} open style="width: 250px;"> <${listOptionTag} value="1">Option 1 <${listOptionTag}> ` diff --git a/packages/storybook/src/nimble/combobox/combobox-opened-in-div.stories.ts b/packages/storybook/src/nimble/combobox/combobox-opened-in-div.stories.ts index e782e3c388..fcc99454bd 100644 --- a/packages/storybook/src/nimble/combobox/combobox-opened-in-div.stories.ts +++ b/packages/storybook/src/nimble/combobox/combobox-opened-in-div.stories.ts @@ -27,7 +27,7 @@ const component = ([ ]: PositionState): ViewTemplate => html`
(position === 'below' ? 'height: 150px' : null)}>
- <${comboboxTag} open position="${() => position}"> + <${comboboxTag} open position="${() => position}" style="width: 250px;"> <${listOptionTag} value="1">Option 1 <${listOptionTag} value="2">Option 2 <${listOptionTag} value="3">Option 3 diff --git a/packages/storybook/src/nimble/combobox/combobox-opened-matrix.stories.ts b/packages/storybook/src/nimble/combobox/combobox-opened-matrix.stories.ts index cd12e124ba..5f090a4cc0 100644 --- a/packages/storybook/src/nimble/combobox/combobox-opened-matrix.stories.ts +++ b/packages/storybook/src/nimble/combobox/combobox-opened-matrix.stories.ts @@ -26,7 +26,7 @@ const component = ([ position, positionStyle ]: PositionState): ViewTemplate => html` - <${comboboxTag} open position="${() => position}" style="${() => positionStyle}"> + <${comboboxTag} open position="${() => position}" style="width: 250px; ${() => positionStyle}"> <${listOptionTag} value="1">Option 1 <${listOptionTag} value="2" disabled>Option 2 <${listOptionTag} value="3">Option 3 diff --git a/packages/storybook/src/nimble/combobox/combobox.stories.ts b/packages/storybook/src/nimble/combobox/combobox.stories.ts index ad6a9b1dea..dd5d6bc705 100644 --- a/packages/storybook/src/nimble/combobox/combobox.stories.ts +++ b/packages/storybook/src/nimble/combobox/combobox.stories.ts @@ -3,7 +3,6 @@ import { html, repeat } from '@microsoft/fast-element'; import { withActions } from '@storybook/addon-actions/decorator'; import type { HtmlRenderer, Meta, StoryObj } from '@storybook/html'; import { listOptionTag } from '@ni/nimble-components/dist/esm/list-option'; -import { menuMinWidth } from '@ni/nimble-components/dist/esm/theme-provider/design-tokens'; import { comboboxTag } from '@ni/nimble-components/dist/esm/combobox'; import { ExampleOptionsType } from '@ni/nimble-components/dist/esm/combobox/tests/types'; import { @@ -104,7 +103,7 @@ const metadata: Meta = { appearance="${x => x.appearance}" value="${x => x.currentValue}" placeholder="${x => x.placeholder}" - style="width: var(${menuMinWidth.cssCustomProperty});" + style="width: 250px;" > ${repeat(x => optionSets[x.optionsType], html` <${listOptionTag} ?disabled="${x => x.disabled}">${x => x.label} diff --git a/packages/storybook/src/nimble/select/select-matrix.stories.ts b/packages/storybook/src/nimble/select/select-matrix.stories.ts index 746a5d9e5b..300bd0e829 100644 --- a/packages/storybook/src/nimble/select/select-matrix.stories.ts +++ b/packages/storybook/src/nimble/select/select-matrix.stories.ts @@ -3,7 +3,6 @@ import { html, ViewTemplate } from '@microsoft/fast-element'; import { controlLabelFont, controlLabelFontColor, - menuMinWidth, standardPadding } from '@ni/nimble-components/dist/esm/theme-provider/design-tokens'; import { listOptionTag } from '@ni/nimble-components/dist/esm/list-option'; @@ -67,7 +66,7 @@ const component = ( error-text="${() => errorText}" ?disabled="${() => disabled}" appearance="${() => appearance}" - style="width: var(${menuMinWidth.cssCustomProperty});" + style="width: 250px;" > <${listOptionTag} value="1">${valueValue} <${listOptionTag} value="2" disabled>Option 2 @@ -88,14 +87,14 @@ export const selectThemeMatrix: StoryFn = createMatrixThemeStory( export const hiddenSelect: StoryFn = createStory( hiddenWrapper( - html`<${selectTag} hidden> + html`<${selectTag} hidden style="width: 250px;"> <${listOptionTag} value="1">Option 1 ` ) ); export const blankListOption: StoryFn = createStory( - html`<${selectTag} open> + html`<${selectTag} open style="width: 250px;"> <${listOptionTag} value="1">Option 1 <${listOptionTag}> ` @@ -104,7 +103,7 @@ export const blankListOption: StoryFn = createStory( export const textCustomized: StoryFn = createMatrixThemeStory( textCustomizationWrapper( html` - <${selectTag}> + <${selectTag} style="width: 250px;"> Inner text <${listOptionTag}> Nimble select item diff --git a/packages/storybook/src/nimble/select/select-opened-in-div.stories.ts b/packages/storybook/src/nimble/select/select-opened-in-div.stories.ts index 2075e260bc..cdf0a0d468 100644 --- a/packages/storybook/src/nimble/select/select-opened-in-div.stories.ts +++ b/packages/storybook/src/nimble/select/select-opened-in-div.stories.ts @@ -27,7 +27,7 @@ const component = ([ ]: PositionState): ViewTemplate => html`
(position === 'below' ? 'height: 150px' : null)}>
- <${selectTag} open position="${() => position}"> + <${selectTag} open position="${() => position}" style="width: 250px;"> <${listOptionTag} value="1">Option 1 <${listOptionTag} value="2">Option 2 <${listOptionTag} value="3">Option 3 diff --git a/packages/storybook/src/nimble/select/select-opened-matrix.stories.ts b/packages/storybook/src/nimble/select/select-opened-matrix.stories.ts index 8fe68c946b..c756e2a9c8 100644 --- a/packages/storybook/src/nimble/select/select-opened-matrix.stories.ts +++ b/packages/storybook/src/nimble/select/select-opened-matrix.stories.ts @@ -34,7 +34,7 @@ const component = ( filterMode: FilterModeState, placeholder?: PlaceholderState ): ViewTemplate => html` - <${selectTag} open position="${() => position}" style="${() => positionStyle}" filter-mode="${() => filterMode}"> + <${selectTag} open position="${() => position}" style="width: 250px; ${() => positionStyle}" filter-mode="${() => filterMode}"> <${listOptionTag} value="1" ${placeholder ? 'selected disabled hidden' : ''} >Option 1 <${listOptionTag} value="2" disabled>Option 2 <${listOptionTag} value="3">Option 3 diff --git a/packages/storybook/src/nimble/select/select.stories.ts b/packages/storybook/src/nimble/select/select.stories.ts index 5878122d84..f11fb35b2a 100644 --- a/packages/storybook/src/nimble/select/select.stories.ts +++ b/packages/storybook/src/nimble/select/select.stories.ts @@ -2,7 +2,6 @@ import { html, repeat, when } from '@microsoft/fast-element'; import { withActions } from '@storybook/addon-actions/decorator'; import type { HtmlRenderer, Meta, StoryObj } from '@storybook/html'; import { listOptionTag } from '@ni/nimble-components/dist/esm/list-option'; -import { menuMinWidth } from '@ni/nimble-components/dist/esm/theme-provider/design-tokens'; import { selectTag } from '@ni/nimble-components/dist/esm/select'; import { FilterMode } from '@ni/nimble-components/dist/esm/select/types'; import { ExampleOptionsType } from '@ni/nimble-components/dist/esm/select/tests/types'; @@ -100,7 +99,7 @@ const metadata: Meta = { position="${x => x.dropDownPosition}" appearance="${x => x.appearance}" filter-mode="${x => (x.filterMode === 'none' ? undefined : x.filterMode)}" - style="width: var(${menuMinWidth.cssCustomProperty});" + style="width: 250px;" > ${when(x => x.placeholder, html` <${listOptionTag} From 0ac9d81713bf9e91e1d89578cabdbc4db0b67009 Mon Sep 17 00:00:00 2001 From: rajsite Date: Tue, 7 May 2024 08:17:00 -0500 Subject: [PATCH 2/6] applying package updates [skip ci] --- .../projects/ni/nimble-angular/CHANGELOG.json | 15 +++++++++++++++ .../projects/ni/nimble-angular/CHANGELOG.md | 10 +++++++++- .../projects/ni/nimble-angular/package.json | 4 ++-- .../projects/ni/spright-angular/CHANGELOG.json | 15 +++++++++++++++ .../projects/ni/spright-angular/CHANGELOG.md | 10 +++++++++- .../projects/ni/spright-angular/package.json | 4 ++-- ...ents-a5b4c275-28d7-4509-af45-748fdd2f69fb.json | 7 ------- package-lock.json | 14 +++++++------- packages/nimble-components/CHANGELOG.json | 15 +++++++++++++++ packages/nimble-components/CHANGELOG.md | 10 +++++++++- packages/nimble-components/package.json | 2 +- packages/spright-components/CHANGELOG.json | 15 +++++++++++++++ packages/spright-components/CHANGELOG.md | 10 +++++++++- packages/spright-components/package.json | 4 ++-- 14 files changed, 110 insertions(+), 25 deletions(-) delete mode 100644 change/@ni-nimble-components-a5b4c275-28d7-4509-af45-748fdd2f69fb.json diff --git a/angular-workspace/projects/ni/nimble-angular/CHANGELOG.json b/angular-workspace/projects/ni/nimble-angular/CHANGELOG.json index 2bffdfda16..808964aa89 100644 --- a/angular-workspace/projects/ni/nimble-angular/CHANGELOG.json +++ b/angular-workspace/projects/ni/nimble-angular/CHANGELOG.json @@ -1,6 +1,21 @@ { "name": "@ni/nimble-angular", "entries": [ + { + "date": "Tue, 07 May 2024 13:17:00 GMT", + "version": "24.0.1", + "tag": "@ni/nimble-angular_v24.0.1", + "comments": { + "patch": [ + { + "author": "beachball", + "package": "@ni/nimble-angular", + "comment": "Bump @ni/nimble-components to v28.0.1", + "commit": "not available" + } + ] + } + }, { "date": "Mon, 06 May 2024 19:00:05 GMT", "version": "24.0.0", diff --git a/angular-workspace/projects/ni/nimble-angular/CHANGELOG.md b/angular-workspace/projects/ni/nimble-angular/CHANGELOG.md index e78af03920..e8ff4d507e 100644 --- a/angular-workspace/projects/ni/nimble-angular/CHANGELOG.md +++ b/angular-workspace/projects/ni/nimble-angular/CHANGELOG.md @@ -1,9 +1,17 @@ # Change Log - @ni/nimble-angular -This log was last generated on Mon, 06 May 2024 19:00:05 GMT and should not be manually modified. +This log was last generated on Tue, 07 May 2024 13:17:00 GMT and should not be manually modified. +## 24.0.1 + +Tue, 07 May 2024 13:17:00 GMT + +### Patches + +- Bump @ni/nimble-components to v28.0.1 + ## 24.0.0 Mon, 06 May 2024 19:00:05 GMT diff --git a/angular-workspace/projects/ni/nimble-angular/package.json b/angular-workspace/projects/ni/nimble-angular/package.json index e61c69b671..b736fd6850 100644 --- a/angular-workspace/projects/ni/nimble-angular/package.json +++ b/angular-workspace/projects/ni/nimble-angular/package.json @@ -1,6 +1,6 @@ { "name": "@ni/nimble-angular", - "version": "24.0.0", + "version": "24.0.1", "description": "Angular components for the NI Nimble Design System", "scripts": { "invoke-publish": "npm run invoke-publish:setup && cd ../../../dist/ni/nimble-angular && npm publish", @@ -32,7 +32,7 @@ "@angular/forms": "^16.2.12", "@angular/localize": "^16.2.12", "@angular/router": "^16.2.12", - "@ni/nimble-components": "^28.0.0" + "@ni/nimble-components": "^28.0.1" }, "dependencies": { "tslib": "^2.2.0" diff --git a/angular-workspace/projects/ni/spright-angular/CHANGELOG.json b/angular-workspace/projects/ni/spright-angular/CHANGELOG.json index 9dde65757c..f352c76fb7 100644 --- a/angular-workspace/projects/ni/spright-angular/CHANGELOG.json +++ b/angular-workspace/projects/ni/spright-angular/CHANGELOG.json @@ -1,6 +1,21 @@ { "name": "@ni/spright-angular", "entries": [ + { + "date": "Tue, 07 May 2024 13:17:00 GMT", + "version": "0.1.12", + "tag": "@ni/spright-angular_v0.1.12", + "comments": { + "patch": [ + { + "author": "beachball", + "package": "@ni/spright-angular", + "comment": "Bump @ni/spright-components to v0.0.15", + "commit": "not available" + } + ] + } + }, { "date": "Mon, 06 May 2024 19:00:05 GMT", "version": "0.1.11", diff --git a/angular-workspace/projects/ni/spright-angular/CHANGELOG.md b/angular-workspace/projects/ni/spright-angular/CHANGELOG.md index d6170c28bd..c45df0d798 100644 --- a/angular-workspace/projects/ni/spright-angular/CHANGELOG.md +++ b/angular-workspace/projects/ni/spright-angular/CHANGELOG.md @@ -1,9 +1,17 @@ # Change Log - @ni/spright-angular -This log was last generated on Mon, 06 May 2024 19:00:05 GMT and should not be manually modified. +This log was last generated on Tue, 07 May 2024 13:17:00 GMT and should not be manually modified. +## 0.1.12 + +Tue, 07 May 2024 13:17:00 GMT + +### Patches + +- Bump @ni/spright-components to v0.0.15 + ## 0.1.11 Mon, 06 May 2024 19:00:05 GMT diff --git a/angular-workspace/projects/ni/spright-angular/package.json b/angular-workspace/projects/ni/spright-angular/package.json index 4047587185..6af177ac8e 100644 --- a/angular-workspace/projects/ni/spright-angular/package.json +++ b/angular-workspace/projects/ni/spright-angular/package.json @@ -1,6 +1,6 @@ { "name": "@ni/spright-angular", - "version": "0.1.11", + "version": "0.1.12", "description": "Angular components for NI Spright", "scripts": { "invoke-publish": "npm run invoke-publish:setup && cd ../../../dist/ni/spright-angular && npm publish", @@ -24,7 +24,7 @@ "peerDependencies": { "@angular/common": "^16.2.12", "@angular/core": "^16.2.12", - "@ni/spright-components": "^0.0.14" + "@ni/spright-components": "^0.0.15" }, "dependencies": { "tslib": "^2.2.0" diff --git a/change/@ni-nimble-components-a5b4c275-28d7-4509-af45-748fdd2f69fb.json b/change/@ni-nimble-components-a5b4c275-28d7-4509-af45-748fdd2f69fb.json deleted file mode 100644 index 0fee042299..0000000000 --- a/change/@ni-nimble-components-a5b4c275-28d7-4509-af45-748fdd2f69fb.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "type": "patch", - "comment": "Set Select and Combobox min-width to menuMinWidth token (176px)", - "packageName": "@ni/nimble-components", - "email": "1458528+fredvisser@users.noreply.github.com", - "dependentChangeType": "patch" -} diff --git a/package-lock.json b/package-lock.json index 411f4c3fbe..ddfb7f3a9b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -82,7 +82,7 @@ }, "angular-workspace/projects/ni/nimble-angular": { "name": "@ni/nimble-angular", - "version": "24.0.0", + "version": "24.0.1", "license": "MIT", "dependencies": { "tslib": "^2.2.0" @@ -93,12 +93,12 @@ "@angular/forms": "^16.2.12", "@angular/localize": "^16.2.12", "@angular/router": "^16.2.12", - "@ni/nimble-components": "^28.0.0" + "@ni/nimble-components": "^28.0.1" } }, "angular-workspace/projects/ni/spright-angular": { "name": "@ni/spright-angular", - "version": "0.1.11", + "version": "0.1.12", "license": "MIT", "dependencies": { "tslib": "^2.2.0" @@ -106,7 +106,7 @@ "peerDependencies": { "@angular/common": "^16.2.12", "@angular/core": "^16.2.12", - "@ni/spright-components": "^0.0.14" + "@ni/spright-components": "^0.0.15" } }, "node_modules/@11ty/dependency-tree": { @@ -32678,7 +32678,7 @@ }, "packages/nimble-components": { "name": "@ni/nimble-components", - "version": "28.0.0", + "version": "28.0.1", "license": "MIT", "dependencies": { "@microsoft/fast-colors": "^5.3.1", @@ -32826,14 +32826,14 @@ }, "packages/spright-components": { "name": "@ni/spright-components", - "version": "0.0.14", + "version": "0.0.15", "license": "MIT", "dependencies": { "@microsoft/fast-colors": "^5.3.1", "@microsoft/fast-element": "^1.12.0", "@microsoft/fast-foundation": "^2.49.6", "@microsoft/fast-web-utilities": "^6.0.0", - "@ni/nimble-components": "28.0.0", + "@ni/nimble-components": "28.0.1", "@ni/nimble-tokens": "^6.13.5", "tslib": "^2.2.0" }, diff --git a/packages/nimble-components/CHANGELOG.json b/packages/nimble-components/CHANGELOG.json index aa2e34d410..23f6707143 100644 --- a/packages/nimble-components/CHANGELOG.json +++ b/packages/nimble-components/CHANGELOG.json @@ -1,6 +1,21 @@ { "name": "@ni/nimble-components", "entries": [ + { + "date": "Tue, 07 May 2024 13:17:00 GMT", + "version": "28.0.1", + "tag": "@ni/nimble-components_v28.0.1", + "comments": { + "patch": [ + { + "author": "1458528+fredvisser@users.noreply.github.com", + "package": "@ni/nimble-components", + "commit": "799d39216faa9ad98c94627c6505439c846cbac9", + "comment": "Set Select and Combobox min-width to menuMinWidth token (176px)" + } + ] + } + }, { "date": "Mon, 06 May 2024 20:26:50 GMT", "version": "28.0.0", diff --git a/packages/nimble-components/CHANGELOG.md b/packages/nimble-components/CHANGELOG.md index 38eaee98bd..08f1ee27a3 100644 --- a/packages/nimble-components/CHANGELOG.md +++ b/packages/nimble-components/CHANGELOG.md @@ -1,9 +1,17 @@ # Change Log - @ni/nimble-components -This log was last generated on Mon, 06 May 2024 19:00:05 GMT and should not be manually modified. +This log was last generated on Tue, 07 May 2024 13:17:00 GMT and should not be manually modified. +## 28.0.1 + +Tue, 07 May 2024 13:17:00 GMT + +### Patches + +- Set Select and Combobox min-width to menuMinWidth token (176px) ([ni/nimble@799d392](https://github.com/ni/nimble/commit/799d39216faa9ad98c94627c6505439c846cbac9)) + ## 28.0.0 Mon, 06 May 2024 19:00:05 GMT diff --git a/packages/nimble-components/package.json b/packages/nimble-components/package.json index 2dcd5df23b..c3c53dcbc0 100644 --- a/packages/nimble-components/package.json +++ b/packages/nimble-components/package.json @@ -1,6 +1,6 @@ { "name": "@ni/nimble-components", - "version": "28.0.0", + "version": "28.0.1", "description": "Styled web components for the NI Nimble Design System", "scripts": { "build": "npm run generate-icons && npm run generate-workers && npm run build-components && npm run bundle-components && npm run generate-scss", diff --git a/packages/spright-components/CHANGELOG.json b/packages/spright-components/CHANGELOG.json index 43202e3cf3..3324fcefa4 100644 --- a/packages/spright-components/CHANGELOG.json +++ b/packages/spright-components/CHANGELOG.json @@ -1,6 +1,21 @@ { "name": "@ni/spright-components", "entries": [ + { + "date": "Tue, 07 May 2024 13:17:00 GMT", + "version": "0.0.15", + "tag": "@ni/spright-components_v0.0.15", + "comments": { + "patch": [ + { + "author": "beachball", + "package": "@ni/spright-components", + "comment": "Bump @ni/nimble-components to v28.0.1", + "commit": "not available" + } + ] + } + }, { "date": "Mon, 06 May 2024 19:00:05 GMT", "version": "0.0.14", diff --git a/packages/spright-components/CHANGELOG.md b/packages/spright-components/CHANGELOG.md index 1241a90b2d..5c23eddaef 100644 --- a/packages/spright-components/CHANGELOG.md +++ b/packages/spright-components/CHANGELOG.md @@ -1,9 +1,17 @@ # Change Log - @ni/spright-components -This log was last generated on Mon, 06 May 2024 19:00:05 GMT and should not be manually modified. +This log was last generated on Tue, 07 May 2024 13:17:00 GMT and should not be manually modified. +## 0.0.15 + +Tue, 07 May 2024 13:17:00 GMT + +### Patches + +- Bump @ni/nimble-components to v28.0.1 + ## 0.0.14 Mon, 06 May 2024 19:00:05 GMT diff --git a/packages/spright-components/package.json b/packages/spright-components/package.json index 0feb682c25..118c1daacc 100644 --- a/packages/spright-components/package.json +++ b/packages/spright-components/package.json @@ -1,6 +1,6 @@ { "name": "@ni/spright-components", - "version": "0.0.14", + "version": "0.0.15", "description": "NI Spright Components", "scripts": { "build": "npm run build-components && npm run bundle-components", @@ -55,7 +55,7 @@ "@microsoft/fast-element": "^1.12.0", "@microsoft/fast-foundation": "^2.49.6", "@microsoft/fast-web-utilities": "^6.0.0", - "@ni/nimble-components": "28.0.0", + "@ni/nimble-components": "28.0.1", "@ni/nimble-tokens": "^6.13.5", "tslib": "^2.2.0" }, From bbe265424e76e1824a5748b8bc6cb6d69a1b9a17 Mon Sep 17 00:00:00 2001 From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com> Date: Tue, 7 May 2024 11:09:33 -0500 Subject: [PATCH 3/6] Select - no longer update value while navigating dropdown (#2029) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request ## ๐Ÿคจ Rationale While in the midst of implementing the [clear select](https://github.com/ni/nimble/issues/1951) feature, I recognized that the fact that we were updating the value while navigating the dropdown was both confusing (since we don't update the display value) and resulting in an awkward implementation for the feature. --- ...-9da2496e-e544-4019-a666-d7fca3046771.json | 7 + .../src/list-option/index.ts | 11 + .../nimble-components/src/select/index.ts | 200 ++++++++++----- .../nimble-components/src/select/styles.ts | 14 ++ .../src/select/testing/select.pageobject.ts | 76 +++++- .../select/tests/select.foundation.spec.ts | 22 +- .../src/select/tests/select.spec.ts | 232 ++++++++++++++++-- .../nimble/select/select-matrix.stories.ts | 27 +- 8 files changed, 496 insertions(+), 93 deletions(-) create mode 100644 change/@ni-nimble-components-9da2496e-e544-4019-a666-d7fca3046771.json diff --git a/change/@ni-nimble-components-9da2496e-e544-4019-a666-d7fca3046771.json b/change/@ni-nimble-components-9da2496e-e544-4019-a666-d7fca3046771.json new file mode 100644 index 0000000000..3cdc357bd2 --- /dev/null +++ b/change/@ni-nimble-components-9da2496e-e544-4019-a666-d7fca3046771.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Change Select to not update value as you navigate options in dropdown.", + "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 6f60d343e2..a529ebd1d8 100644 --- a/packages/nimble-components/src/list-option/index.ts +++ b/packages/nimble-components/src/list-option/index.ts @@ -41,6 +41,17 @@ export class ListOption extends FoundationListboxOption { @attr({ attribute: 'visually-hidden', mode: 'boolean' }) public visuallyHidden = false; + /** + * @internal + * This attribute is used to control the visual selected state of an option. This + * is handled independently of the public 'selected' attribute, as 'selected' is + * representative of the current value of the container control. However, while + * a dropdown is open users can navigate through the options (requiring visual + * updates) without changing the value of the container control. + */ + @attr({ attribute: 'active-option', mode: 'boolean' }) + public activeOption = false; + /** @internal */ @observable public hasOverflow = false; diff --git a/packages/nimble-components/src/select/index.ts b/packages/nimble-components/src/select/index.ts index dce1e7557b..05fac5618b 100644 --- a/packages/nimble-components/src/select/index.ts +++ b/packages/nimble-components/src/select/index.ts @@ -18,6 +18,7 @@ import { DelegatesARIASelect } from '@microsoft/fast-foundation'; import { + findLastIndex, keyArrowDown, keyArrowUp, keyEnd, @@ -25,7 +26,6 @@ import { keyEscape, keyHome, keySpace, - keyTab, uniqueId } from '@microsoft/fast-web-utilities'; import { arrowExpanderDown16X16 } from '@ni/nimble-tokens/dist/icons/js'; @@ -53,7 +53,7 @@ declare global { // eslint-disable-next-line @typescript-eslint/no-invalid-void-type type BooleanOrVoid = boolean | void; -const isNimbleListOption = (el: Element): el is ListOption => { +const isNimbleListOption = (el: Element | undefined): el is ListOption => { return el instanceof ListOption; }; @@ -196,7 +196,7 @@ export class Select private _value = ''; private forcedPosition = false; - private indexWhenOpened?: number; + private openActiveIndex?: number; /** * @internal @@ -204,7 +204,9 @@ export class Select public override connectedCallback(): void { super.connectedCallback(); this.forcedPosition = !!this.positionAttribute; - this.initializeOpenState(); + if (this.open) { + this.initializeOpenState(); + } } public override get value(): string { @@ -338,8 +340,7 @@ export class Select super.clickHandler(e); this.open = this.collapsible && !this.open; - - if (!this.open && this.indexWhenOpened !== this.selectedIndex) { + if (!this.open && this.selectedIndex !== -1) { this.updateValue(true); } } @@ -468,23 +469,22 @@ export class Select */ public inputHandler(e: InputEvent): boolean { this.filter = this.filterInput?.value ?? ''; - this.clearSelection(); this.filterOptions(); - if (this.filteredOptions.length > 0) { - const enabledOptions = this.filteredOptions.filter( - o => !o.disabled - ); - if (enabledOptions.length > 0) { - enabledOptions[0]!.selected = true; - } else { - // only filtered option is disabled - this.selectedOptions = []; - this.selectedIndex = -1; - } - } else if (this.committedSelectedOption) { - this.committedSelectedOption.selected = true; + const enabledOptions = this.filteredOptions.filter(o => !o.disabled); + let activeOptionIndex = this.filter !== '' + ? this.openActiveIndex ?? this.selectedIndex + : this.selectedIndex; + + if ( + enabledOptions.length > 0 + && !enabledOptions.find(o => o === this.options[activeOptionIndex]) + ) { + activeOptionIndex = this.options.indexOf(enabledOptions[0]!); + } else if (enabledOptions.length === 0) { + activeOptionIndex = -1; } + this.setActiveOption(activeOptionIndex); if (e.inputType.includes('deleteContent') || !this.filter.length) { return true; @@ -510,12 +510,14 @@ export class Select } if (!this.options?.includes(focusTarget as ListboxOption)) { + let currentActiveIndex = this.openActiveIndex ?? this.selectedIndex; this.open = false; - if (this.selectedIndex === -1) { - this.selectedIndex = this.indexWhenOpened!; + if (currentActiveIndex === -1) { + currentActiveIndex = this.selectedIndex; } - if (this.indexWhenOpened !== this.selectedIndex) { + if (this.selectedIndex !== currentActiveIndex) { + this.selectedIndex = currentActiveIndex; this.updateValue(true); } } @@ -526,12 +528,14 @@ export class Select * @internal */ public override keydownHandler(e: KeyboardEvent): BooleanOrVoid { + const initialSelectedIndex = this.selectedIndex; super.keydownHandler(e); const key = e.key; if (e.ctrlKey || e.shiftKey) { return true; } + let currentActiveIndex = this.openActiveIndex ?? this.selectedIndex; switch (key) { case keySpace: { // when dropdown is open allow user to enter a space for filter text @@ -571,35 +575,28 @@ export class Select if (!this.open) { break; } + if (this.collapsible && this.open) { e.preventDefault(); this.open = false; } - if (this.selectedIndex !== this.indexWhenOpened!) { - this.options[this.selectedIndex]!.selected = false; - this.selectedIndex = this.indexWhenOpened!; - } + currentActiveIndex = this.selectedIndex; this.focus(); break; } - case keyTab: { - if (this.collapsible && this.open) { - e.preventDefault(); - this.open = false; - } - - return true; - } default: { break; } } - if (!this.open && this.indexWhenOpened !== this.selectedIndex) { + if (!this.open && this.selectedIndex !== currentActiveIndex) { + this.selectedIndex = currentActiveIndex; + } + + if (!this.open && initialSelectedIndex !== this.selectedIndex) { this.updateValue(true); - this.indexWhenOpened = this.selectedIndex; } return !(key === keyArrowDown || key === keyArrowUp); @@ -622,9 +619,34 @@ export class Select // implementation handles skipping non-selected disabled options for the initial // selected value. this.setSelectedOptions(); + if (this.open) { + this.setActiveOption(this.selectedIndex); + } this.updateValue(); } + /** + * @internal + * Fork of Listbox implementation, so that the selectedIndex is not changed while the dropdown + * is open. + */ + public override typeaheadBufferChanged(_: string, __: string): void { + if (this.$fastController.isConnected) { + const typeaheadMatches = this.getTypeaheadMatches(); + + if (typeaheadMatches.length) { + const activeOptionIndex = this.options.indexOf( + typeaheadMatches[0] as ListOption + ); + if (!(this.open && this.filterMode !== FilterMode.none)) { + this.setActiveOption(activeOptionIndex); + } + } + + this.typeaheadExpired = false; + } + } + /** * Synchronize the `aria-disabled` property when the `disabled` property changes. * @@ -655,36 +677,65 @@ export class Select } } + /** + * @internal + */ 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++) { + const startIndex = this.openActiveIndex ?? this.selectedIndex; + for (let i = startIndex + 1; i < this.options.length; i++) { const listOption = this.options[i]!; if ( isNimbleListOption(listOption) && isOptionSelectable(listOption) ) { - this.selectedIndex = i; + this.setActiveOption(i); break; } } } + /** + * @internal + */ 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--) { + const startIndex = this.openActiveIndex ?? this.selectedIndex; + for (let i = startIndex - 1; i >= 0; i--) { const listOption = this.options[i]!; if ( isNimbleListOption(listOption) && isOptionSelectable(listOption) ) { - this.selectedIndex = i; + this.setActiveOption(i); break; } } } + /** + * @internal + */ + public override selectFirstOption(): void { + const newActiveOptionIndex = this.options.findIndex( + o => isNimbleListOption(o) && isOptionSelectable(o) + ); + this.setActiveOption(newActiveOptionIndex); + } + + /** + * @internal + */ + public override selectLastOption(): void { + const newActiveOptionIndex = findLastIndex( + this.options, + o => isNimbleListOption(o) && isOptionSelectable(o) + ); + this.setActiveOption(newActiveOptionIndex); + } + /** * @internal */ @@ -709,7 +760,6 @@ export class Select if (this.open && this.selectedIndex === -1) { return; } - super.setSelectedOptions(); } @@ -722,6 +772,12 @@ export class Select } } + protected override getTypeaheadMatches(): ListboxOption[] { + const matches = super.getTypeaheadMatches(); + // Don't allow placeholder to be matched + return matches.filter(o => !o.hidden && !o.disabled); + } + protected positionChanged( _: SelectPosition | undefined, next: SelectPosition | undefined @@ -757,11 +813,14 @@ export class Select if (this.open) { this.initializeOpenState(); - this.indexWhenOpened = this.selectedIndex; - return; } + const activeOption = this.options[this.openActiveIndex ?? this.selectedIndex]; + if (isNimbleListOption(activeOption)) { + activeOption.activeOption = false; + } + this.openActiveIndex = undefined; this.filter = ''; if (this.filterInput) { this.filterInput.value = ''; @@ -837,6 +896,46 @@ export class Select this.committedSelectedOption = options[this.selectedIndex]; } + private setActiveOption(newActiveIndex: number): void { + const activeOption = this.options[newActiveIndex]; + if (this.open) { + if (isNimbleListOption(activeOption)) { + activeOption.activeOption = true; + } + + const previousActiveIndex = this.openActiveIndex ?? this.selectedIndex; + const previousActiveOption = this.options[previousActiveIndex]; + if ( + previousActiveIndex !== newActiveIndex + && isNimbleListOption(previousActiveOption) + ) { + previousActiveOption.activeOption = false; + } + + this.openActiveIndex = newActiveIndex; + this.focusAndScrollActiveOptionIntoView(); + } else { + this.selectedIndex = newActiveIndex; + } + + this.ariaActiveDescendant = activeOption?.id ?? ''; + } + + private focusAndScrollActiveOptionIntoView(): void { + const optionToFocus = this.options[this.openActiveIndex ?? this.selectedIndex]; + // Copied from FAST: To ensure that the browser handles both `focus()` and + // `scrollIntoView()`, the timing here needs to guarantee that they happen on + // different frames. Since this function is typically called from the `openChanged` + // observer, `DOM.queueUpdate` causes the calls to be grouped into the same frame. + // To prevent this, `requestAnimationFrame` is used instead of `DOM.queueUpdate`. + if (optionToFocus !== undefined && this.contains(optionToFocus)) { + optionToFocus.focus(); + requestAnimationFrame(() => { + optionToFocus.scrollIntoView({ block: 'nearest' }); + }); + } + } + private committedSelectedOptionChanged(): void { this.updateDisplayValue(); } @@ -946,12 +1045,6 @@ export class Select } } - private clearSelection(): void { - this.options.forEach(option => { - option.selected = false; - }); - } - private filterChanged(): void { this.filterOptions(); } @@ -961,13 +1054,8 @@ export class Select } private initializeOpenState(): void { - if (!this.open) { - this.ariaExpanded = 'false'; - this.ariaControls = ''; - return; - } - this.committedSelectedOption = this.options[this.selectedIndex]; + this.setActiveOption(this.selectedIndex); this.ariaControls = this.listboxId; this.ariaExpanded = 'true'; diff --git a/packages/nimble-components/src/select/styles.ts b/packages/nimble-components/src/select/styles.ts index d543d15819..28f87e02d0 100644 --- a/packages/nimble-components/src/select/styles.ts +++ b/packages/nimble-components/src/select/styles.ts @@ -6,6 +6,8 @@ import { borderRgbPartialColor, borderWidth, controlHeight, + fillHoverSelectedColor, + fillSelectedColor, mediumPadding, placeholderFontColor, smallPadding @@ -116,6 +118,18 @@ export const styles = css` overflow: auto; } + ::slotted([role='option']) { + background-color: transparent; + } + + ::slotted([role='option'][active-option]) { + background-color: ${fillSelectedColor}; + } + + ::slotted([role='option'][active-option]:hover) { + background-color: ${fillHoverSelectedColor}; + } + .no-results-label { color: ${placeholderFontColor}; height: ${controlHeight}; diff --git a/packages/nimble-components/src/select/testing/select.pageobject.ts b/packages/nimble-components/src/select/testing/select.pageobject.ts index 4ea4cda6db..e649f58ca9 100644 --- a/packages/nimble-components/src/select/testing/select.pageobject.ts +++ b/packages/nimble-components/src/select/testing/select.pageobject.ts @@ -3,7 +3,8 @@ import { keyEscape, keyArrowDown, keyArrowUp, - keySpace + keySpace, + keyTab } from '@microsoft/fast-web-utilities'; import type { Select } from '..'; import type { ListOption } from '../../list-option'; @@ -56,6 +57,20 @@ export class SelectPageObject { return (this.selectElement.selectedOptions[0] as ListOption) ?? null; } + public getActiveOption(): ListOption | null { + return ( + (this.selectElement.options.find( + o => (o as ListOption).activeOption + ) as ListOption) ?? null + ); + } + + public getDisplayText(): string { + const displayText = this.selectElement.shadowRoot?.querySelector('.selected-value') + ?.textContent ?? ''; + return displayText.trim(); + } + /** * Either opens or closes the dropdown depending on its current state */ @@ -63,12 +78,16 @@ export class SelectPageObject { this.selectElement.click(); } - public clickSelectedItem(): void { + public clickActiveItem(): void { if (!this.selectElement.open) { throw new Error('Select must be open to click selectedItem'); } - this.clickOption(this.selectElement.selectedIndex); + const selectedOption = this.getActiveOption(); + if (!selectedOption) { + throw new Error('No option is selected to click'); + } + this.clickOption(this.selectElement.options.indexOf(selectedOption)); } public async clickFilterInput(): Promise { @@ -126,6 +145,13 @@ export class SelectPageObject { ); } + public pressTabKey(): void { + this.selectElement.dispatchEvent( + new KeyboardEvent('keydown', { key: keyTab }) + ); + this.selectElement.dispatchEvent(new FocusEvent('focusout')); + } + public pressArrowDownKey(): void { this.selectElement.dispatchEvent( new KeyboardEvent('keydown', { key: keyArrowDown }) @@ -138,6 +164,30 @@ export class SelectPageObject { ); } + public pressCharacterKey(character: string): void { + if (character.length !== 1) { + throw new Error( + 'character parameter must contain only a single character' + ); + } + + if ( + this.selectElement.open + && this.selectElement.filterMode !== FilterMode.none + ) { + const filterInput = this.selectElement.filterInput!; + filterInput.value += character; + } + const inputElement = this.selectElement.open + && this.selectElement.filterMode !== FilterMode.none + ? this.selectElement.filterInput + : this.selectElement; + inputElement!.dispatchEvent(new InputEvent('input')); + this.selectElement.dispatchEvent( + new KeyboardEvent('keydown', { key: character }) + ); + } + public async pressSpaceKey(): Promise { const alreadyOpen = this.selectElement.open; this.selectElement.dispatchEvent( @@ -193,7 +243,21 @@ export class SelectPageObject { } public getFilterInputText(): string { - return this.getFilterInput()?.value ?? ''; + return this.selectElement.filterInput?.value ?? ''; + } + + public async waitForChange(): Promise { + return new Promise(resolve => { + this.selectElement.addEventListener( + 'change', + () => { + resolve(this.selectElement.value); + }, + { + once: true + } + ); + }); } private getFilterInput(): HTMLInputElement | null | undefined { @@ -202,8 +266,6 @@ export class SelectPageObject { 'Select has filterMode of "none" so there is no filter input' ); } - return this.selectElement.shadowRoot?.querySelector( - '.filter-input' - ); + return this.selectElement.filterInput; } } diff --git a/packages/nimble-components/src/select/tests/select.foundation.spec.ts b/packages/nimble-components/src/select/tests/select.foundation.spec.ts index df84a45042..8405d03cf5 100644 --- a/packages/nimble-components/src/select/tests/select.foundation.spec.ts +++ b/packages/nimble-components/src/select/tests/select.foundation.spec.ts @@ -1,4 +1,8 @@ -// Based on tests in FAST repo: https://github.com/microsoft/fast/blob/085cb27d348ed6f59d080c167fa62aeaa1e3940e/packages/web-components/fast-foundation/src/select/select.spec.ts +/** + * Based on tests in FAST repo: https://github.com/microsoft/fast/blob/085cb27d348ed6f59d080c167fa62aeaa1e3940e/packages/web-components/fast-foundation/src/select/select.spec.ts + * One notable change to the tests is that we no longer expect the selectedIndex to change for the Select as the user + * navigates the dropdown menu. As as result, some test names, and the relevant expect assertions have changed. + */ import { keyArrowDown, keyArrowUp, @@ -298,7 +302,7 @@ describe('Select', () => { await disconnect(); }); - describe("should NOT emit a 'change' event when the value changes by user input while open", () => { + describe("should NOT emit a 'change' event while open", () => { it('via arrow down key', async () => { const { element, connect, disconnect } = await setup(); @@ -322,7 +326,7 @@ describe('Select', () => { expect(wasChanged).toBeFalse(); - expect(element.value).toEqual('two'); + expect(element.value).toEqual('one'); await disconnect(); }); @@ -354,7 +358,7 @@ describe('Select', () => { expect(wasChanged).toBeFalse(); - expect(element.value).toEqual('one'); + expect(element.value).toEqual('two'); await disconnect(); }); @@ -412,13 +416,13 @@ describe('Select', () => { expect(wasChanged).toBeFalse(); - expect(element.value).toEqual('three'); + expect(element.value).toEqual('one'); await disconnect(); }); }); - describe("should NOT emit an 'input' event when the value changes by user input while open", () => { + describe("should NOT emit an 'input' event while open", () => { it('via arrow down key', async () => { const { element, connect, disconnect } = await setup(); @@ -442,7 +446,7 @@ describe('Select', () => { expect(wasInput).toBeFalse(); - expect(element.value).toEqual('two'); + expect(element.value).toEqual('one'); await disconnect(); }); @@ -474,7 +478,7 @@ describe('Select', () => { expect(wasInput).toBeFalse(); - expect(element.value).toEqual('one'); + expect(element.value).toEqual('two'); await disconnect(); }); @@ -532,7 +536,7 @@ describe('Select', () => { expect(wasInput).toBeFalse(); - expect(element.value).toEqual('three'); + expect(element.value).toEqual('one'); await disconnect(); }); diff --git a/packages/nimble-components/src/select/tests/select.spec.ts b/packages/nimble-components/src/select/tests/select.spec.ts index 7f23f76d8f..0be6a103c8 100644 --- a/packages/nimble-components/src/select/tests/select.spec.ts +++ b/packages/nimble-components/src/select/tests/select.spec.ts @@ -184,8 +184,6 @@ describe('Select', () => { pageObject.clickSelect(); pageObject.pressArrowDownKey(); await waitForUpdatesAsync(); - expect(element.selectedIndex).toBe(1); - pageObject.pressEscapeKey(); await waitForUpdatesAsync(); @@ -201,7 +199,7 @@ describe('Select', () => { pageObject.pressArrowDownKey(); await waitForUpdatesAsync(); - expect(element.displayValue).toBe('One'); + expect(pageObject.getDisplayText()).toBe('One'); await disconnect(); }); @@ -310,10 +308,10 @@ describe('Select', () => { await waitForUpdatesAsync(); await clickAndWaitForOpen(element); pageObject.pressArrowDownKey(); - expect(pageObject.getSelectedOption()?.value).toBe('three'); + expect(pageObject.getActiveOption()?.value).toBe('three'); pageObject.pressArrowUpKey(); - expect(pageObject.getSelectedOption()?.value).toBe('one'); + expect(pageObject.getActiveOption()?.value).toBe('one'); await disconnect(); }); @@ -326,7 +324,78 @@ describe('Select', () => { const selectedOption = pageObject.getSelectedOption(); selectedOption!.textContent = 'foo'; await waitForUpdatesAsync(); - expect(element.displayValue).toBe('foo'); + expect(pageObject.getDisplayText()).toBe('foo'); + + await disconnect(); + }); + + it('after forcing Select to be blank, user can arrow down to first available option in dropdown', async () => { + const { element, connect, disconnect } = await setup(); + const pageObject = new SelectPageObject(element); + await connect(); + await waitForUpdatesAsync(); + element.selectedIndex = -1; + await waitForUpdatesAsync(); + + pageObject.clickSelect(); + pageObject.pressArrowDownKey(); + expect(pageObject.getActiveOption()?.value).toBe('one'); + + await disconnect(); + }); + + it('after forcing Select to be blank, pressing arrow down sets value to first available option', async () => { + const { element, connect, disconnect } = await setup(); + const pageObject = new SelectPageObject(element); + await connect(); + await waitForUpdatesAsync(); + element.selectedIndex = -1; + await waitForUpdatesAsync(); + + pageObject.pressArrowDownKey(); + expect(pageObject.getSelectedOption()?.value).toBe('one'); + + await disconnect(); + }); + + it('selecting option via typing character while dropdown is closed changes value', async () => { + const { element, connect, disconnect } = await setup(); + const pageObject = new SelectPageObject(element); + await connect(); + await waitForUpdatesAsync(); + pageObject.pressCharacterKey('t'); + await waitForUpdatesAsync(); + + expect(element.value).toBe('two'); + + await disconnect(); + }); + + it('selecting option via typing character while dropdown is open with no filter does not change value', async () => { + const { element, connect, disconnect } = await setup(); + const pageObject = new SelectPageObject(element); + await connect(); + await waitForUpdatesAsync(); + pageObject.clickSelect(); + pageObject.pressCharacterKey('t'); + await waitForUpdatesAsync(); + + expect(element.value).toBe('one'); + expect(pageObject.getActiveOption()?.value).toBe('two'); + + await disconnect(); + }); + + it('while navigating dropdown, ariaActiveDescendant is set to active option', async () => { + const { element, connect, disconnect } = await setup(); + const pageObject = new SelectPageObject(element); + await connect(); + await waitForUpdatesAsync(); + pageObject.clickSelect(); + pageObject.pressArrowDownKey(); + expect(element.ariaActiveDescendant).toBe( + pageObject.getActiveOption()!.id + ); await disconnect(); }); @@ -358,6 +427,23 @@ describe('Select', () => { await disconnect(); }); + it('when handling change event, value of select element matches what was selected in dropdown on focusout', async () => { + const { element, connect, disconnect } = await setup(); + await connect(); + await waitForUpdatesAsync(); + const pageObject = new SelectPageObject(element); + await clickAndWaitForOpen(element); + + const changeValuePromise = pageObject.waitForChange(); + pageObject.pressArrowDownKey(); + await pageObject.clickAway(); + const selectValue = await changeValuePromise; + + expect(selectValue).toBe('two'); + + await disconnect(); + }); + describe('with 500 options', () => { async function setup500Options(): Promise> { // prettier-ignore @@ -568,6 +654,13 @@ describe('Select', () => { pageObject.pressEnterKey(); expect(document.activeElement).toBe(element); }); + + it('after closing dropdown by committing a value with , activeElement is not Select element', () => { + pageObject.clickSelect(); + pageObject.pressArrowDownKey(); + pageObject.pressTabKey(); + expect(document.activeElement).not.toBe(element); + }); }); }); }); @@ -630,12 +723,12 @@ describe('Select', () => { expect(element.value).toBe('one'); await pageObject.openAndSetFilterText('T'); // Matches 'Two' and 'Three' - currentSelection = pageObject.getSelectedOption(); + currentSelection = pageObject.getActiveOption(); expect(currentSelection?.text).toBe('Two'); pageObject.pressEscapeKey(); pageObject.clickSelect(); - currentSelection = pageObject.getSelectedOption(); + currentSelection = pageObject.getActiveOption(); expect(currentSelection?.text).toBe('One'); }); @@ -648,7 +741,7 @@ describe('Select', () => { pageObject.pressEnterKey(); pageObject.clickSelect(); - currentSelection = pageObject.getSelectedOption(); + currentSelection = pageObject.getActiveOption(); expect(currentSelection?.selected).toBeTrue(); }); @@ -663,13 +756,24 @@ describe('Select', () => { expect(element.open).toBeFalse(); }); - it('filtering out current selected item and then clicking selected option changes value and closes popup', async () => { + it('filtering out current selected item and then pressing changes value and closes popup', async () => { const currentSelection = pageObject.getSelectedOption(); expect(currentSelection?.text).toBe('One'); expect(element.value).toBe('one'); await pageObject.openAndSetFilterText('T'); // Matches 'Two' and 'Three' - pageObject.clickSelectedItem(); + pageObject.pressTabKey(); + expect(element.value).toBe('two'); // 'Two' is first option in list so it should be selected now + expect(element.open).toBeFalse(); + }); + + it('filtering out current selected item and then clicking active option changes value and closes popup', async () => { + const currentSelection = pageObject.getSelectedOption(); + expect(currentSelection?.text).toBe('One'); + expect(element.value).toBe('one'); + + await pageObject.openAndSetFilterText('T'); // Matches 'Two' and 'Three' + pageObject.clickActiveItem(); expect(element.value).toBe('two'); // 'Two' is first option in list so it should be selected now expect(element.open).toBeFalse(); }); @@ -698,7 +802,7 @@ describe('Select', () => { it('allows to be used as part of filter text', async () => { await pageObject.openAndSetFilterText(' '); // Matches 'Has Space' - const currentSelection = pageObject.getSelectedOption(); + const currentSelection = pageObject.getActiveOption(); expect(currentSelection?.text).toBe('Has Space'); expect(element.open).toBeTrue(); }); @@ -743,7 +847,7 @@ describe('Select', () => { currentFilteredOptions ); expect(element.open).toBeTrue(); - expect(pageObject.getSelectedOption()?.text).toBe('Two'); + expect(pageObject.getActiveOption()?.text).toBe('Two'); }); it('filtering to only disabled item, then pressing does not close popup or change value', async () => { @@ -753,6 +857,34 @@ describe('Select', () => { expect(element.value).toBe('one'); }); + it('filtering to only disabled item, then pressing closes popup and does not change value or selectedIndex', async () => { + await pageObject.openAndSetFilterText('Disabled'); + pageObject.pressEscapeKey(); + expect(element.open).toBeFalse(); + expect(element.value).toBe('one'); + expect(element.selectedIndex).toBe(0); + }); + + it('filtering to only disabled item, then pressing closes popup and does not change value or selectedIndex', async () => { + await pageObject.openAndSetFilterText('Disabled'); + pageObject.pressTabKey(); + expect(element.open).toBeFalse(); + expect(element.value).toBe('one'); + expect(element.selectedIndex).toBe(0); + }); + + it('filtering to no available options, then pressing does not close popup or change value', async () => { + await pageObject.openAndSetFilterText('abc'); + pageObject.pressEnterKey(); + expect(element.open).toBeTrue(); + expect(element.value).toBe('one'); + }); + + it('filtering to no available options sets ariaActiveDescendent to empty string', async () => { + await pageObject.openAndSetFilterText('abc'); + expect(element.ariaActiveDescendant).toBe(''); + }); + it('filtering to no available options, then pressing does not close popup or change value', async () => { await pageObject.openAndSetFilterText('abc'); pageObject.pressEnterKey(); @@ -769,7 +901,7 @@ describe('Select', () => { it('filtering to only disabled item does not select item', async () => { await pageObject.openAndSetFilterText('Disabled'); - expect(pageObject.getSelectedOption()).toBeNull(); + expect(pageObject.getActiveOption()).toBeNull(); }); it('updating slottedOptions while open applies filter to new options', async () => { @@ -813,11 +945,11 @@ describe('Select', () => { await pageObject.setOptions(newOptions); await pageObject.openAndSetFilterText('tw'); pageObject.pressArrowDownKey(); - let currentSelection = pageObject.getSelectedOption(); + let currentSelection = pageObject.getActiveOption(); expect(currentSelection?.value).toBe('twenty'); pageObject.pressArrowUpKey(); - currentSelection = pageObject.getSelectedOption(); + currentSelection = pageObject.getActiveOption(); expect(currentSelection?.value).toBe('two'); }); @@ -837,7 +969,7 @@ describe('Select', () => { expect(currentSelection?.value).toBe('one'); }); - it('cant not select option that has been filtered out pressing arrowDown', async () => { + it('can not select option that has been filtered out pressing arrowDown', async () => { await pageObject.openAndSetFilterText('tw'); pageObject.pressArrowDownKey(); pageObject.pressEnterKey(); @@ -851,6 +983,55 @@ describe('Select', () => { currentSelection = pageObject.getSelectedOption(); expect(currentSelection?.value).toBe('three'); }); + + it('when dropdown is closed, entering text executes typeahead and sets value', () => { + pageObject.pressCharacterKey('t'); + expect(element.value).toBe('two'); + }); + + it('opening dropdown after pressing during filter text entry, maintains original display text', async () => { + await clickAndWaitForOpen(element); + pageObject.pressCharacterKey('t'); + pageObject.pressEscapeKey(); + pageObject.clickSelect(); + await waitForUpdatesAsync(); + + expect(pageObject.getDisplayText()).toBe('One'); + }); + + it('filtering options does not change selected option in dropdown', async () => { + element.value = 'three'; + await pageObject.openAndSetFilterText('t'); // filters to 'Two' and 'Three' + + expect(pageObject.getActiveOption()?.value).toBe('three'); + }); + + it('filtering options does not change selected option in dropdown after navigating with arrow keys', async () => { + await clickAndWaitForOpen(element); + pageObject.pressArrowDownKey(); + pageObject.pressArrowDownKey(); // option 'Three' should be active + pageObject.pressCharacterKey('t'); // filters to 'Two' and 'Three' + + expect(pageObject.getActiveOption()?.value).toBe('three'); + }); + + it('dismissing dropdown with after navigation and then filtering to no options, does not update value', async () => { + await clickAndWaitForOpen(element); + pageObject.pressArrowDownKey(); + pageObject.pressCharacterKey('?'); + pageObject.pressTabKey(); + + expect(pageObject.getSelectedOption()?.value).toBe('one'); + }); + + it('dismissing dropdown by clicking away after navigation and then filtering to no options, does not update value', async () => { + await clickAndWaitForOpen(element); + pageObject.pressArrowDownKey(); + pageObject.pressCharacterKey('?'); + await pageObject.clickAway(); + + expect(pageObject.getSelectedOption()?.value).toBe('one'); + }); }); describe('placeholder', () => { @@ -869,6 +1050,7 @@ describe('Select', () => { element.filterMode = FilterMode.standard; await connect(); pageObject = new SelectPageObject(element); + await waitForUpdatesAsync(); }); afterEach(async () => { @@ -881,12 +1063,12 @@ describe('Select', () => { }); it('selecting option will replace placeholder text with selected option text', async () => { - expect(element.displayValue).toBe('One'); + expect(pageObject.getDisplayText()).toBe('One'); await clickAndWaitForOpen(element); pageObject.clickOption(1); await waitForUpdatesAsync(); - expect(element.displayValue).toBe('Two'); + expect(pageObject.getDisplayText()).toBe('Two'); }); it('placeholder can be changed to another option programmatically', async () => { @@ -897,12 +1079,22 @@ describe('Select', () => { element.options[1]!.selected = true; await waitForUpdatesAsync(); - expect(element.displayValue).toBe('Two'); + expect(pageObject.getDisplayText()).toBe('Two'); expect(element.value).toBe('two'); await clickAndWaitForOpen(element); expect(pageObject.isOptionVisible(0)).toBeTrue(); expect(pageObject.isOptionVisible(1)).toBeFalse(); }); + + it('selecting option via typing will not select placeholder', async () => { + const newOptions = element.options.map(o => o as ListOption); + newOptions.push(new ListOption('One one', 'one one')); + await pageObject.setOptions(newOptions); + expect(pageObject.getDisplayText()).toBe('One'); + pageObject.pressCharacterKey('o'); + await waitForUpdatesAsync(); + expect(pageObject.getDisplayText()).toBe('One one'); + }); }); describe('PageObject', () => { diff --git a/packages/storybook/src/nimble/select/select-matrix.stories.ts b/packages/storybook/src/nimble/select/select-matrix.stories.ts index 300bd0e829..e2c8761fd8 100644 --- a/packages/storybook/src/nimble/select/select-matrix.stories.ts +++ b/packages/storybook/src/nimble/select/select-matrix.stories.ts @@ -1,13 +1,15 @@ import type { StoryFn, Meta } from '@storybook/html'; import { html, ViewTemplate } from '@microsoft/fast-element'; +import { keyArrowDown } from '@microsoft/fast-web-utilities'; import { controlLabelFont, controlLabelFontColor, standardPadding } from '@ni/nimble-components/dist/esm/theme-provider/design-tokens'; import { listOptionTag } from '@ni/nimble-components/dist/esm/list-option'; -import { selectTag } from '@ni/nimble-components/dist/esm/select'; +import { Select, selectTag } from '@ni/nimble-components/dist/esm/select'; import { DropdownAppearance } from '@ni/nimble-components/dist/esm/patterns/dropdown/types'; +import { waitForUpdatesAsync } from '@ni/nimble-components/dist/esm/testing/async-helpers'; import { createStory } from '../../utilities/storybook'; import { createMatrixThemeStory, @@ -100,6 +102,29 @@ export const blankListOption: StoryFn = createStory( ` ); +const playFunction = async (): Promise => { + await Promise.all( + Array.from(document.querySelectorAll