diff options
author | Kamil Musin <kamilm@google.com> | 2023-08-21 16:50:01 +0200 |
---|---|---|
committer | Luca Milanesio <luca.milanesio@gmail.com> | 2024-04-10 19:37:19 +0000 |
commit | 1c8af9bcf033ef870b0d522d572713097a126dca (patch) | |
tree | 2c1f21b977fb514e32984725a34a5528e0ff87c9 | |
parent | 2c44778ab9aed093f1f63fb086f986c462d9dc50 (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
(cherry picked from commit ae6aa0f26a985d7ac34b3eef5be6ef24c1bbda86)
-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 465afce951..aac917281e 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}); + } } /** |