diff options
author | Kamil Musin <kamilm@google.com> | 2023-08-21 16:50:01 +0200 |
---|---|---|
committer | Kamil Musin <kamilm@google.com> | 2023-08-22 10:49:50 +0200 |
commit | ae6aa0f26a985d7ac34b3eef5be6ef24c1bbda86 (patch) | |
tree | 5d89d7ef5f1ad73dadb3441be08dedfc4f3d9253 | |
parent | 7d5574739498a09f56fa3ba29d4f66dce2146728 (diff) |
Fix gr-dropdown-list not updating on items change
handleValueChanged is dependent on contents of `items` therefore it
needs to be recalculated if it changes.
items were being received/set later than path when loading gr-diff-view
from a url, resulting in no path shown in the gr-dropdown-list
We only emit an event if the new value matches one of the items. It
matches the behaviour before the change. It might make sense to emit the
value anyway, but that broke gr-admin-view tests, because they don't set
up rest api calls. So we keep it at the old behaviour.
Also, spotted some small indexing errors in admin-view tests and fixed
that.
Google-Bug-Id: b/296106236
Release-Notes: skip
Change-Id: I6b5457f71b573c9c56c425372cb0f4ec242e10f0
-rw-r--r-- | polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts | 7 | ||||
-rw-r--r-- | polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts | 12 |
2 files changed, 12 insertions, 7 deletions
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts index ee4a17961e..0a2e4f2458 100644 --- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts +++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view_test.ts @@ -111,10 +111,10 @@ suite('gr-admin-view tests', () => { assert.isNotOk(element.filteredLinks![0].subsection); // Groups - assert.isNotOk(element.filteredLinks![0].subsection); + assert.isNotOk(element.filteredLinks![1].subsection); // Plugins - assert.isNotOk(element.filteredLinks![0].subsection); + assert.isNotOk(element.filteredLinks![2].subsection); }); test('filteredLinks non admin authenticated', async () => { @@ -123,7 +123,7 @@ suite('gr-admin-view tests', () => { // Repos assert.isNotOk(element.filteredLinks![0].subsection); // Groups - assert.isNotOk(element.filteredLinks![0].subsection); + assert.isNotOk(element.filteredLinks![1].subsection); }); test('filteredLinks non admin unathenticated', async () => { @@ -262,6 +262,7 @@ suite('gr-admin-view tests', () => { test('Needs reload when changing from repo to group', async () => { element.repoName = 'Test Repo' as RepoName; + element.view = GerritView.REPO; stubRestApi('getAccount').returns( Promise.resolve({ name: 'test-user', diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts index b6af123d13..6b2cca64a5 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts +++ b/polygerrit-ui/app/elements/shared/gr-dropdown-list/gr-dropdown-list.ts @@ -187,8 +187,8 @@ export class GrDropdownList extends LitElement { } protected override willUpdate(changedProperties: PropertyValues): void { - if (changedProperties.has('value')) { - this.handleValueChange(); + if (changedProperties.has('value') || changedProperties.has('items')) { + this.handleDataChange(changedProperties.has('value')); } } @@ -307,7 +307,7 @@ export class GrDropdownList extends LitElement { }, 1); } - private handleValueChange() { + private handleDataChange(valueChanged: boolean) { if (this.value === undefined || this.items === undefined) { return; } @@ -318,7 +318,11 @@ export class GrDropdownList extends LitElement { this.text = selectedObj.triggerText ? selectedObj.triggerText : selectedObj.text; - fireNoBubble(this, 'value-change', {value: this.value}); + // Only emit the event if the value actually changed and matches one of the + // items. + if (valueChanged) { + fireNoBubble(this, 'value-change', {value: this.value}); + } } /** |