From 83f7fd9e601eed789e8f6eb61a9d4a87a1d1835b Mon Sep 17 00:00:00 2001 From: Jonathan Meyer <26874831+atmgrifter00@users.noreply.github.com> Date: Wed, 20 Mar 2024 11:26:21 -0500 Subject: [PATCH 1/2] Combobox registerOption reversion to address Angular bug (#1949) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Pull Request ## ๐Ÿคจ Rationale [AzDO bug 'Nimble combo box | Issue in showing list options'](https://ni.visualstudio.com/DevCentral/_workitems/edit/2687002) ## ๐Ÿ‘ฉโ€๐Ÿ’ป Implementation Simple reversion of problem code. I've also created a [tech debt item](https://github.com/ni/nimble/issues/1948) to track the work needed to ultimately re-align the `Combobox` and `Select` implementations. ## ๐Ÿงช Testing Just removing the related test. ## โœ… Checklist - [ ] I have updated the project documentation to reflect my changes or determined no changes are needed. --- ...-54ec9080-33bb-47a6-a37d-d926c42de859.json | 7 +++++ .../nimble-components/src/combobox/index.ts | 25 ++-------------- .../src/combobox/tests/combobox.spec.ts | 29 +------------------ 3 files changed, 10 insertions(+), 51 deletions(-) create mode 100644 change/@ni-nimble-components-54ec9080-33bb-47a6-a37d-d926c42de859.json diff --git a/change/@ni-nimble-components-54ec9080-33bb-47a6-a37d-d926c42de859.json b/change/@ni-nimble-components-54ec9080-33bb-47a6-a37d-d926c42de859.json new file mode 100644 index 0000000000..897eb39604 --- /dev/null +++ b/change/@ni-nimble-components-54ec9080-33bb-47a6-a37d-d926c42de859.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Remove ListOptionOwner from Combobox to address issue found in Angular", + "packageName": "@ni/nimble-components", + "email": "26874831+atmgrifter00@users.noreply.github.com", + "dependentChangeType": "patch" +} diff --git a/packages/nimble-components/src/combobox/index.ts b/packages/nimble-components/src/combobox/index.ts index 4f607c5439..e3da226ded 100644 --- a/packages/nimble-components/src/combobox/index.ts +++ b/packages/nimble-components/src/combobox/index.ts @@ -17,14 +17,10 @@ import { iconExclamationMarkTag } from '../icons/exclamation-mark'; import { styles } from './styles'; import type { ErrorPattern } from '../patterns/error/types'; -import type { - DropdownPattern, - ListOptionOwner -} from '../patterns/dropdown/types'; +import type { DropdownPattern } from '../patterns/dropdown/types'; import { DropdownAppearance } from '../patterns/dropdown/types'; import type { AnchoredRegion } from '../anchored-region'; import { template } from './template'; -import type { ListOption } from '../list-option'; declare global { interface HTMLElementTagNameMap { @@ -37,7 +33,7 @@ declare global { */ export class Combobox extends FoundationCombobox - implements DropdownPattern, ErrorPattern, ListOptionOwner { + implements DropdownPattern, ErrorPattern { @attr public appearance: DropdownAppearance = DropdownAppearance.underline; @@ -211,23 +207,6 @@ export class Combobox return returnValue; } - /** - * @internal - */ - public registerOption(option: ListOption): void { - if (this.options.includes(option)) { - return; - } - - // Adding an option to the end, ultimately, isn't the correct - // thing to do, as this will mean the option's index in the options, - // at least temporarily, does not match the DOM order. However, it - // is expected that a successive run of `slottedOptionsChanged` will - // correct this order issue. See 'https://github.com/ni/nimble/issues/1915' - // for more info. - this.options.push(option); - } - protected override focusAndScrollOptionIntoView(): void { if (this.open) { super.focusAndScrollOptionIntoView(); diff --git a/packages/nimble-components/src/combobox/tests/combobox.spec.ts b/packages/nimble-components/src/combobox/tests/combobox.spec.ts index 7b282bd515..15973059fc 100644 --- a/packages/nimble-components/src/combobox/tests/combobox.spec.ts +++ b/packages/nimble-components/src/combobox/tests/combobox.spec.ts @@ -9,7 +9,7 @@ import { waitAnimationFrame } from '../../utilities/tests/component'; import { checkFullyInViewport } from '../../utilities/tests/intersection-observer'; -import { ListOption, listOptionTag } from '../../list-option'; +import { listOptionTag } from '../../list-option'; async function setup( position?: string, @@ -127,33 +127,6 @@ describe('Combobox', () => { await disconnect(); }); - it('option added directly to DOM synchronously registers with Combobox', async () => { - const { element, connect, disconnect } = await setup(); - await connect(); - element.selectedIndex = 0; - await waitForUpdatesAsync(); - const newOption = new ListOption('foo', 'foo'); - const registerOptionSpy = spyOn( - element, - 'registerOption' - ).and.callThrough(); - registerOptionSpy.calls.reset(); - element.insertBefore(newOption, element.options[0]!); - - expect(registerOptionSpy.calls.count()).toBe(1); - expect(element.options).toContain(newOption); - - // While the option is registered synchronously as shown above, - // properties like selectedIndex will only be correct asynchronously - // See https://github.com/ni/nimble/issues/1915 - expect(element.selectedIndex).toBe(0); - await waitForUpdatesAsync(); - // This assertion shows that after 'slottedOptionsChanged' runs, the - // 'selectedIndex' state has been corrected to expected DOM order. - expect(element.selectedIndex).toBe(1); - await disconnect(); - }); - const ariaTestData: { attrName: string, propSetter: (x: Combobox, value: string) => void From 699749620c558e3119e22889a4c6f800a1dd4e51 Mon Sep 17 00:00:00 2001 From: rajsite Date: Wed, 20 Mar 2024 16:45:59 +0000 Subject: [PATCH 2/2] 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 ++-- ...ents-54ec9080-33bb-47a6-a37d-d926c42de859.json | 7 ------- package-lock.json | 8 ++++---- packages/nimble-blazor/package.json | 2 +- packages/nimble-components/CHANGELOG.json | 15 +++++++++++++++ packages/nimble-components/CHANGELOG.md | 10 +++++++++- packages/nimble-components/package.json | 2 +- 9 files changed, 56 insertions(+), 17 deletions(-) delete mode 100644 change/@ni-nimble-components-54ec9080-33bb-47a6-a37d-d926c42de859.json diff --git a/angular-workspace/projects/ni/nimble-angular/CHANGELOG.json b/angular-workspace/projects/ni/nimble-angular/CHANGELOG.json index 9e9b860a35..f170144ec8 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": "Wed, 20 Mar 2024 16:45:59 GMT", + "version": "20.5.1", + "tag": "@ni/nimble-angular_v20.5.1", + "comments": { + "patch": [ + { + "author": "beachball", + "package": "@ni/nimble-angular", + "comment": "Bump @ni/nimble-components to v22.1.1", + "commit": "not available" + } + ] + } + }, { "date": "Mon, 18 Mar 2024 17:12:34 GMT", "version": "20.5.0", diff --git a/angular-workspace/projects/ni/nimble-angular/CHANGELOG.md b/angular-workspace/projects/ni/nimble-angular/CHANGELOG.md index f7aafd80de..1a2aa3c1ca 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, 18 Mar 2024 17:12:34 GMT and should not be manually modified. +This log was last generated on Wed, 20 Mar 2024 16:45:59 GMT and should not be manually modified. +## 20.5.1 + +Wed, 20 Mar 2024 16:45:59 GMT + +### Patches + +- Bump @ni/nimble-components to v22.1.1 + ## 20.5.0 Mon, 18 Mar 2024 17:12:34 GMT diff --git a/angular-workspace/projects/ni/nimble-angular/package.json b/angular-workspace/projects/ni/nimble-angular/package.json index ffb4b78ba8..cc13ba5b80 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": "20.5.0", + "version": "20.5.1", "description": "Angular components for the NI Nimble Design System", "scripts": { "invoke-publish": "cd ../../../ && npm run build:library && cd dist/ni/nimble-angular && npm publish" @@ -31,7 +31,7 @@ "@angular/forms": "^15.2.10", "@angular/localize": "^15.2.10", "@angular/router": "^15.2.10", - "@ni/nimble-components": "^22.1.0" + "@ni/nimble-components": "^22.1.1" }, "dependencies": { "tslib": "^2.2.0" diff --git a/change/@ni-nimble-components-54ec9080-33bb-47a6-a37d-d926c42de859.json b/change/@ni-nimble-components-54ec9080-33bb-47a6-a37d-d926c42de859.json deleted file mode 100644 index 897eb39604..0000000000 --- a/change/@ni-nimble-components-54ec9080-33bb-47a6-a37d-d926c42de859.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "type": "patch", - "comment": "Remove ListOptionOwner from Combobox to address issue found in Angular", - "packageName": "@ni/nimble-components", - "email": "26874831+atmgrifter00@users.noreply.github.com", - "dependentChangeType": "patch" -} diff --git a/package-lock.json b/package-lock.json index 7a23d00821..f8fe26c4e2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -74,7 +74,7 @@ }, "angular-workspace/projects/ni/nimble-angular": { "name": "@ni/nimble-angular", - "version": "20.5.0", + "version": "20.5.1", "license": "MIT", "dependencies": { "tslib": "^2.2.0" @@ -85,7 +85,7 @@ "@angular/forms": "^15.2.10", "@angular/localize": "^15.2.10", "@angular/router": "^15.2.10", - "@ni/nimble-components": "^22.1.0" + "@ni/nimble-components": "^22.1.1" } }, "node_modules/@11ty/dependency-tree": { @@ -33939,7 +33939,7 @@ }, "packages/nimble-blazor": { "name": "@ni/nimble-blazor", - "version": "14.5.0", + "version": "14.5.1", "hasInstallScript": true, "license": "MIT", "devDependencies": { @@ -33975,7 +33975,7 @@ }, "packages/nimble-components": { "name": "@ni/nimble-components", - "version": "22.1.0", + "version": "22.1.1", "license": "MIT", "dependencies": { "@microsoft/fast-colors": "^5.3.1", diff --git a/packages/nimble-blazor/package.json b/packages/nimble-blazor/package.json index e804ada040..ff76936429 100644 --- a/packages/nimble-blazor/package.json +++ b/packages/nimble-blazor/package.json @@ -1,6 +1,6 @@ { "name": "@ni/nimble-blazor", - "version": "14.5.0", + "version": "14.5.1", "description": "Blazor components for the NI Nimble Design System", "scripts": { "postinstall": "node build/generate-playwright-version-properties/source/index.js", diff --git a/packages/nimble-components/CHANGELOG.json b/packages/nimble-components/CHANGELOG.json index c060a73b09..8b2a7bfc67 100644 --- a/packages/nimble-components/CHANGELOG.json +++ b/packages/nimble-components/CHANGELOG.json @@ -1,6 +1,21 @@ { "name": "@ni/nimble-components", "entries": [ + { + "date": "Wed, 20 Mar 2024 16:45:59 GMT", + "version": "22.1.1", + "tag": "@ni/nimble-components_v22.1.1", + "comments": { + "patch": [ + { + "author": "26874831+atmgrifter00@users.noreply.github.com", + "package": "@ni/nimble-components", + "commit": "83f7fd9e601eed789e8f6eb61a9d4a87a1d1835b", + "comment": "Remove ListOptionOwner from Combobox to address issue found in Angular" + } + ] + } + }, { "date": "Wed, 20 Mar 2024 15:46:00 GMT", "version": "22.1.0", diff --git a/packages/nimble-components/CHANGELOG.md b/packages/nimble-components/CHANGELOG.md index 28514fbac5..8825f33110 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, 18 Mar 2024 17:12:34 GMT and should not be manually modified. +This log was last generated on Wed, 20 Mar 2024 16:45:59 GMT and should not be manually modified. +## 22.1.1 + +Wed, 20 Mar 2024 16:45:59 GMT + +### Patches + +- Remove ListOptionOwner from Combobox to address issue found in Angular ([ni/nimble@83f7fd9](https://github.com/ni/nimble/commit/83f7fd9e601eed789e8f6eb61a9d4a87a1d1835b)) + ## 22.1.0 Mon, 18 Mar 2024 17:12:34 GMT diff --git a/packages/nimble-components/package.json b/packages/nimble-components/package.json index 946cac8bb2..eedf89911c 100644 --- a/packages/nimble-components/package.json +++ b/packages/nimble-components/package.json @@ -1,6 +1,6 @@ { "name": "@ni/nimble-components", - "version": "22.1.0", + "version": "22.1.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 && npm run build-storybook",