-
Notifications
You must be signed in to change notification settings - Fork 744
refactor(toolkit): add support for async showWhen and skipping prompter backward and forward direction #6166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
7912123
6b41090
71ad18d
4d8ff98
4c5e48d
83cc0ff
77f402f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,24 +3,17 @@ | |
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { StepEstimator } from '../../wizards/wizard' | ||
| import { StepEstimator, WIZARD_SKIP } from '../../wizards/wizard' | ||
| import { Prompter, PromptResult } from '../prompter' | ||
|
|
||
| /** Pseudo-prompter that immediately returns a value (and thus "skips" a step). */ | ||
| /** Prompter that return SKIP control signal to parent wizard */ | ||
| export class SkipPrompter<T> extends Prompter<T> { | ||
| /** | ||
| * @param val Value immediately returned by the prompter. | ||
| */ | ||
| constructor(public readonly val: T) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous implementation supports resolving the value
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Since we have SKIP signal. Resolving value was a workaround.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would there be a case we actually want it to resolve some default value?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A straight forward case would be in With previous solution we will be able to do as the follows
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The previous workaround for skipping prompter was to automatically resolve the value and skip displaying UI to customer. However, that did not work for backward direction since the auto resolve always revert the backward flow to forward flow. So we are splitting the skipping case from setting default value case. For how to use the SkipPrompter, please follow this example: aws-toolkit-vscode/packages/core/src/dev/activation.ts at master · vicheey/aws-toolkit-vscode
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can consider resolving the value when forward and skip when backwards right? |
||
| constructor() { | ||
| super() | ||
| } | ||
|
|
||
| protected async promptUser(): Promise<PromptResult<T>> { | ||
| const promptPromise = new Promise<PromptResult<T>>((resolve) => { | ||
| resolve(this.val) | ||
| }) | ||
|
|
||
| return await promptPromise | ||
| return WIZARD_SKIP | ||
justinmk3 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| public get recentItem(): any { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| /*! | ||
| * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| import { Wizard, WizardOptions } from '../wizards/wizard' | ||
| import { WizardPrompter } from './wizardPrompter' | ||
| import { createHash } from 'crypto' | ||
|
|
||
vicheey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| export abstract class NestedWizard<T> extends Wizard<T> { | ||
vicheey marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This name seems confusing, unless I'm mistaken. This class is a Wizard that contains nested wizards, it's not itself nested. Right? So maybe a better name is |
||
| // Map to store wizard instances | ||
| private wizardInstances: Map<string, any> = new Map() | ||
vicheey marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| protected constructor(options: WizardOptions<T>) { | ||
| super(options) | ||
| } | ||
|
|
||
| protected createWizardPrompter<T>(constructor: new (...args: any[]) => T, ...args: any[]): WizardPrompter<T> { | ||
vicheey marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const memoizeKey = createHash('sha256') | ||
| .update(constructor.name + JSON.stringify(args)) | ||
| .digest('hex') | ||
| if (!this.wizardInstances.get(memoizeKey) as T) { | ||
| this.wizardInstances.set(memoizeKey, new constructor(...args)) | ||
| } | ||
| return new WizardPrompter(this.wizardInstances.get(memoizeKey)) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.