From 23f6dee03b6c1967da7987958cec057234ce90fa Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Sep 2024 16:41:08 -0400 Subject: [PATCH 1/5] ensure previously selected items stay selected --- .../src/shared/ui/common/regionSubmenu.ts | 68 ++++++++++++------- 1 file changed, 44 insertions(+), 24 deletions(-) diff --git a/packages/core/src/shared/ui/common/regionSubmenu.ts b/packages/core/src/shared/ui/common/regionSubmenu.ts index fde9f0545e4..19fb3b696c6 100644 --- a/packages/core/src/shared/ui/common/regionSubmenu.ts +++ b/packages/core/src/shared/ui/common/regionSubmenu.ts @@ -5,11 +5,13 @@ import * as vscode from 'vscode' import globals from '../../extensionGlobals' import { isValidResponse, StepEstimator } from '../../wizards/wizard' -import { createQuickPick, ExtendedQuickPickOptions, ItemLoadTypes } from '../pickerPrompter' +import { createQuickPick, DataQuickPickItem, ExtendedQuickPickOptions, ItemLoadTypes } from '../pickerPrompter' import { Prompter, PromptResult } from '../prompter' import { createRegionPrompter } from './region' import { QuickPickPrompter } from '../pickerPrompter' import { Region } from '../../regions/endpoints' +import { createRefreshButton } from '../buttons' +import { getLogger } from '../../logger' const switchRegion = Symbol('switchRegion') @@ -22,6 +24,23 @@ export class RegionSubmenu extends Prompter> { private currentState: 'data' | 'region' = 'data' private steps?: [current: number, total: number] public activePrompter?: QuickPickPrompter | QuickPickPrompter + private readonly defaultItems: DataQuickPickItem[] = [ + { + label: 'Actions', + kind: vscode.QuickPickItemKind.Separator, + data: undefined, + }, + { + label: 'Switch Region', + data: switchRegion, + description: `current region: ${this.currentRegion}`, + }, + { + label: this.separatorLabel, + kind: vscode.QuickPickItemKind.Separator, + data: undefined, + }, + ] public constructor( private readonly itemsProvider: (region: string) => ItemLoadTypes, @@ -33,30 +52,31 @@ export class RegionSubmenu extends Prompter> { super() } + private refresh(prompter: QuickPickPrompter): void { + const activeBefore = prompter.quickPick.activeItems + prompter + .clearAndLoadItems(this.itemsProvider(this.currentRegion)) + .then(() => { + prompter.quickPick.items = [...this.defaultItems, ...prompter.quickPick.items] + prompter.quickPick.activeItems = activeBefore + }) + .catch((e) => { + getLogger().error('clearAndLoadItems failed: %s', (e as Error).message) + }) + } + private createMenuPrompter() { - const prompter = createQuickPick( - this.itemsProvider(this.currentRegion), - this.dataOptions as ExtendedQuickPickOptions - ) - - prompter.quickPick.items = [ - { - label: 'Actions', - kind: vscode.QuickPickItemKind.Separator, - data: undefined, - }, - { - label: 'Switch Region', - data: switchRegion, - description: `current region: ${this.currentRegion}`, - }, - { - label: this.separatorLabel, - kind: vscode.QuickPickItemKind.Separator, - data: undefined, - }, - ...prompter.quickPick.items, - ] + const refreshButton = createRefreshButton() + const items = this.itemsProvider(this.currentRegion) + const prompter = createQuickPick(items, { + ...this.dataOptions, + buttons: [refreshButton], + } as ExtendedQuickPickOptions) + + prompter.quickPick.items = [...this.defaultItems, ...prompter.quickPick.items] + + refreshButton.onClick = () => this.refresh(prompter) + return prompter } From 93652743559cb0a4b58b0866182f974456c2cce1 Mon Sep 17 00:00:00 2001 From: hkobew Date: Thu, 19 Sep 2024 16:42:26 -0400 Subject: [PATCH 2/5] update changelog --- .../Feature-8f11662c-6a2c-45cf-82e4-bb85e0b3adb0.json | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 packages/toolkit/.changes/next-release/Feature-8f11662c-6a2c-45cf-82e4-bb85e0b3adb0.json diff --git a/packages/toolkit/.changes/next-release/Feature-8f11662c-6a2c-45cf-82e4-bb85e0b3adb0.json b/packages/toolkit/.changes/next-release/Feature-8f11662c-6a2c-45cf-82e4-bb85e0b3adb0.json new file mode 100644 index 00000000000..cba6f31bccc --- /dev/null +++ b/packages/toolkit/.changes/next-release/Feature-8f11662c-6a2c-45cf-82e4-bb85e0b3adb0.json @@ -0,0 +1,4 @@ +{ + "type": "Feature", + "description": "region-related quickpicks now have a refresh button" +} From cc983015ec781562e4b78cd7dc1dd132d5b8c9fd Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 20 Sep 2024 11:59:50 -0400 Subject: [PATCH 3/5] avoid overwriting existing buttons --- packages/core/src/shared/ui/common/regionSubmenu.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/src/shared/ui/common/regionSubmenu.ts b/packages/core/src/shared/ui/common/regionSubmenu.ts index 19fb3b696c6..fd202beb80a 100644 --- a/packages/core/src/shared/ui/common/regionSubmenu.ts +++ b/packages/core/src/shared/ui/common/regionSubmenu.ts @@ -53,6 +53,7 @@ export class RegionSubmenu extends Prompter> { } private refresh(prompter: QuickPickPrompter): void { + // This method cannot be async due to onClick() specifications. Thus we are forced to use .then, .catch as workaround. const activeBefore = prompter.quickPick.activeItems prompter .clearAndLoadItems(this.itemsProvider(this.currentRegion)) @@ -70,7 +71,7 @@ export class RegionSubmenu extends Prompter> { const items = this.itemsProvider(this.currentRegion) const prompter = createQuickPick(items, { ...this.dataOptions, - buttons: [refreshButton], + buttons: [...(this.dataOptions?.buttons ?? []), refreshButton], } as ExtendedQuickPickOptions) prompter.quickPick.items = [...this.defaultItems, ...prompter.quickPick.items] From 5552976d97f73eb3bdf40a50df925494509dd659 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 20 Sep 2024 12:49:09 -0400 Subject: [PATCH 4/5] add tests for new refresh button --- .../src/shared/ui/common/regionSubmenu.ts | 2 +- .../shared/ui/prompters/regionSubmenu.test.ts | 33 ++++++++++++++++++- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/packages/core/src/shared/ui/common/regionSubmenu.ts b/packages/core/src/shared/ui/common/regionSubmenu.ts index fd202beb80a..c8c054c57ea 100644 --- a/packages/core/src/shared/ui/common/regionSubmenu.ts +++ b/packages/core/src/shared/ui/common/regionSubmenu.ts @@ -52,7 +52,7 @@ export class RegionSubmenu extends Prompter> { super() } - private refresh(prompter: QuickPickPrompter): void { + public refresh(prompter: QuickPickPrompter): void { // This method cannot be async due to onClick() specifications. Thus we are forced to use .then, .catch as workaround. const activeBefore = prompter.quickPick.activeItems prompter diff --git a/packages/core/src/test/shared/ui/prompters/regionSubmenu.test.ts b/packages/core/src/test/shared/ui/prompters/regionSubmenu.test.ts index c0b2613d268..10b9ceb1d90 100644 --- a/packages/core/src/test/shared/ui/prompters/regionSubmenu.test.ts +++ b/packages/core/src/test/shared/ui/prompters/regionSubmenu.test.ts @@ -4,6 +4,7 @@ */ import assert from 'assert' +import * as sinon from 'sinon' import { RegionSubmenu } from '../../../../shared/ui/common/regionSubmenu' import { DataQuickPickItem, QuickPickPrompter } from '../../../../shared/ui/pickerPrompter' import { createQuickPickPrompterTester } from '../../../shared/ui/testUtils' @@ -12,7 +13,7 @@ describe('regionSubmenu', function () { let submenuPrompter: RegionSubmenu const region1Data = ['option1a', 'option2a', 'option3a'] - const region2Data = ['option1b', 'option2b', 'option3b'] + let region2Data = ['option1b', 'option2b', 'option3b'] before(async function () { const mockDataProvider = function (regionCode: string) { @@ -64,4 +65,34 @@ describe('regionSubmenu', function () { data: 'option2b', }) }) + + it('only has a refresh button', function () { + const activeButtons = submenuPrompter.activePrompter!.quickPick.buttons + assert.strictEqual(activeButtons.length, 1) + }) + + it('refresh button calls refresh once onClick', function () { + const refreshButton = submenuPrompter.activePrompter!.quickPick.buttons[0] + const refreshStub = sinon.stub(RegionSubmenu.prototype, 'refresh') + refreshButton.onClick!() + sinon.assert.calledOnce(refreshStub) + sinon.restore() + }) + + it('refresh reloads items', async function () { + const itemsBeforeLabels = submenuPrompter.activePrompter!.quickPick.items.map((i) => i.label) + region2Data = ['option1c', 'option2c', 'option3c'] + + // Note that onDidChangeBusy event fires with busy=false when we load new items in. + // Since regionSubmenu retroactively adds the default items, they won't be there yet. + // So we don't check for them in test to avoid looking at implementation level details. + submenuPrompter.activePrompter!.onDidChangeBusy((b: boolean) => { + if (!b) { + const itemsAfterLabels = submenuPrompter.activePrompter!.quickPick.items.map((i) => i.label) + region2Data.forEach((item) => assert(itemsAfterLabels.includes(item))) + assert.notStrictEqual(itemsBeforeLabels, itemsAfterLabels) + } + }) + submenuPrompter.refresh(submenuPrompter.activePrompter! as QuickPickPrompter) + }) }) From 9ba4dc2aa3c3bdae1651185fcc2d2c008fcafa71 Mon Sep 17 00:00:00 2001 From: hkobew Date: Fri, 20 Sep 2024 13:41:20 -0400 Subject: [PATCH 5/5] add ui tag to changelog --- .../Feature-8f11662c-6a2c-45cf-82e4-bb85e0b3adb0.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/toolkit/.changes/next-release/Feature-8f11662c-6a2c-45cf-82e4-bb85e0b3adb0.json b/packages/toolkit/.changes/next-release/Feature-8f11662c-6a2c-45cf-82e4-bb85e0b3adb0.json index cba6f31bccc..8c51547a599 100644 --- a/packages/toolkit/.changes/next-release/Feature-8f11662c-6a2c-45cf-82e4-bb85e0b3adb0.json +++ b/packages/toolkit/.changes/next-release/Feature-8f11662c-6a2c-45cf-82e4-bb85e0b3adb0.json @@ -1,4 +1,4 @@ { "type": "Feature", - "description": "region-related quickpicks now have a refresh button" + "description": "UI: region-related quickpicks now have a refresh button" }