From a56ab4ded97afa97e3d488b95f18b77bca3f6bb8 Mon Sep 17 00:00:00 2001 From: Austin Merrick Date: Thu, 4 Apr 2024 15:20:38 -0700 Subject: [PATCH] [cli] Improve UX for text input validation (#11388) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Originally, I was focused on fixing this visual bug: Screenshot 2024-04-04 at 9 50 56 AM When digging into this issue, I realized there was an opportunity to improve text input validation UX and remove hand-rolled validation code. ## Before: https://github.com/vercel/vercel/assets/8485687/5e264696-8b60-4863-8dc2-1a797f508074 https://github.com/vercel/vercel/assets/8485687/2104603a-331e-4679-9ec2-a4235fad252e ## After: https://github.com/vercel/vercel/assets/8485687/72c51831-2b94-4cbe-b1f6-a5822bf9f1a8 https://github.com/vercel/vercel/assets/8485687/a371c7be-f7f5-4d3a-9451-58e7f572d25b ## Additional considerations [input-root-directory](https://github.com/vercel/vercel/blob/main/packages/cli/src/util/input/input-root-directory.ts#L17) remains an instance of hand-rolled validation since it's trickier to force into inquirer's validation pattern. Also, we have a [hand-rolled text input function](https://github.com/vercel/vercel/blob/main/packages/cli/src/util/input/text.ts#L43) that will get similar benefits when replaced with `client.input.text`. --- .changeset/orange-houses-rest.md | 5 ++ packages/cli/src/commands/bisect/index.ts | 30 +++---- packages/cli/src/commands/env/add.ts | 14 +--- packages/cli/src/commands/env/rm.ts | 12 +-- packages/cli/src/util/input/input-project.ts | 87 +++++++------------- 5 files changed, 54 insertions(+), 94 deletions(-) create mode 100644 .changeset/orange-houses-rest.md diff --git a/.changeset/orange-houses-rest.md b/.changeset/orange-houses-rest.md new file mode 100644 index 000000000..2bc9a5e1f --- /dev/null +++ b/.changeset/orange-houses-rest.md @@ -0,0 +1,5 @@ +--- +'vercel': minor +--- + +improve UX for text input validation diff --git a/packages/cli/src/commands/bisect/index.ts b/packages/cli/src/commands/bisect/index.ts index 4ecc863a2..e6c6a5633 100644 --- a/packages/cli/src/commands/bisect/index.ts +++ b/packages/cli/src/commands/bisect/index.ts @@ -45,10 +45,16 @@ export default async function bisect(client: Client): Promise { let bad = argv['--bad'] || - (await prompt(client, `Specify a URL where the bug occurs:`)); + (await client.input.text({ + message: `Specify a URL where the bug occurs:`, + validate: val => (val ? true : 'A URL must be provided'), + })); let good = argv['--good'] || - (await prompt(client, `Specify a URL where the bug does not occur:`)); + (await client.input.text({ + message: `Specify a URL where the bug does not occur:`, + validate: val => (val ? true : 'A URL must be provided'), + })); let subpath = argv['--path'] || ''; let run = argv['--run'] || ''; const openEnabled = argv['--open'] || false; @@ -97,10 +103,10 @@ export default async function bisect(client: Client): Promise { } if (!subpath) { - subpath = await prompt( - client, - `Specify the URL subpath where the bug occurs:` - ); + subpath = await client.input.text({ + message: `Specify the URL subpath where the bug occurs:`, + validate: val => (val ? true : 'A subpath must be provided'), + }); } output.spinner('Retrieving deployments…'); @@ -335,15 +341,3 @@ function getCommit(deployment: Deployment) { deployment.meta?.bitbucketCommitMessage; return { sha, message }; } - -async function prompt(client: Client, message: string): Promise { - // eslint-disable-next-line no-constant-condition - while (true) { - const val = await client.input.text({ message }); - if (val) { - return val; - } else { - client.output.error('A value must be specified'); - } - } -} diff --git a/packages/cli/src/commands/env/add.ts b/packages/cli/src/commands/env/add.ts index 04d6b14ee..fc2da3f5f 100644 --- a/packages/cli/src/commands/env/add.ts +++ b/packages/cli/src/commands/env/add.ts @@ -63,14 +63,11 @@ export default async function add( envTargets.push(envTargetArg); } - while (!envName) { + if (!envName) { envName = await client.input.text({ message: `What’s the name of the variable?`, + validate: val => (val ? true : 'Name cannot be empty'), }); - - if (!envName) { - output.error('Name cannot be empty'); - } } const { envs } = await getEnvRecords( @@ -100,11 +97,9 @@ export default async function add( if (stdInput) { envValue = stdInput; } else { - const inputValue = await client.input.text({ + envValue = await client.input.text({ message: `What’s the value of ${envName}?`, }); - - envValue = inputValue || ''; } while (envTargets.length === 0) { @@ -124,10 +119,9 @@ export default async function add( envTargets.length === 1 && envTargets[0] === 'preview' ) { - const inputValue = await client.input.text({ + envGitBranch = await client.input.text({ message: `Add ${envName} to which Git branch? (leave empty for all Preview branches)?`, }); - envGitBranch = inputValue || ''; } const type = opts['--sensitive'] ? 'sensitive' : 'encrypted'; diff --git a/packages/cli/src/commands/env/rm.ts b/packages/cli/src/commands/env/rm.ts index b982ff7a5..cd6313892 100644 --- a/packages/cli/src/commands/env/rm.ts +++ b/packages/cli/src/commands/env/rm.ts @@ -40,17 +40,11 @@ export default async function rm( let [envName, envTarget, envGitBranch] = args; - while (!envName) { - const inputName = await client.input.text({ + if (!envName) { + envName = await client.input.text({ message: `What’s the name of the variable?`, + validate: val => (val ? true : 'Name cannot be empty'), }); - - if (!inputName) { - output.error(`Name cannot be empty`); - continue; - } - - envName = inputName; } if (!isValidEnvTarget(envTarget)) { diff --git a/packages/cli/src/util/input/input-project.ts b/packages/cli/src/util/input/input-project.ts index 2fe9b7143..16c5a7548 100644 --- a/packages/cli/src/util/input/input-project.ts +++ b/packages/cli/src/util/input/input-project.ts @@ -75,64 +75,37 @@ export default async function inputProject( if (shouldLinkProject) { // user wants to link a project - let project: Project | ProjectNotFound | null = null; - - while (!project || project instanceof ProjectNotFound) { - const projectName = await client.input.text({ - message: 'What’s the name of your existing project?', - }); - - if (!projectName) { - output.error(`Project name cannot be empty`); - continue; - } - - output.spinner('Verifying project name…', 1000); - try { - project = await getProjectByIdOrName(client, projectName, org.id); - } finally { - output.stopSpinner(); - } - - if (project instanceof ProjectNotFound) { - output.error(`Project not found`); - } - } - - return project; + let toLink: Project; + await client.input.text({ + message: 'What’s the name of your existing project?', + validate: async val => { + if (!val) { + return 'Project name cannot be empty'; + } + const project = await getProjectByIdOrName(client, val, org.id); + if (project instanceof ProjectNotFound) { + return 'Project not found'; + } + toLink = project; + return true; + }, + }); + return toLink!; } // user wants to create a new project - let newProjectName: string | null = null; - - while (!newProjectName) { - newProjectName = await client.input.text({ - message: `What’s your project’s name?`, - default: !detectedProject ? slugifiedName : undefined, - }); - - if (!newProjectName) { - output.error(`Project name cannot be empty`); - continue; - } - - output.spinner('Verifying project name…', 1000); - let existingProject: Project | ProjectNotFound; - try { - existingProject = await getProjectByIdOrName( - client, - newProjectName, - org.id - ); - } finally { - output.stopSpinner(); - } - - if (existingProject && !(existingProject instanceof ProjectNotFound)) { - output.print(`Project already exists`); - newProjectName = null; - } - } - - return newProjectName; + return await client.input.text({ + message: `What’s your project’s name?`, + default: !detectedProject ? slugifiedName : undefined, + validate: async val => { + if (!val) { + return 'Project name cannot be empty'; + } + const project = await getProjectByIdOrName(client, val, org.id); + if (!(project instanceof ProjectNotFound)) { + return 'Project already exists'; + } + return true; + }, + }); }