Don't use a select widget to select from >10 projects #1493
Conversation
mythmon
force-pushed
the
mythmon/240628/too-many-projects
branch
from
fe9ba59 to
110d92f
Compare
|
Can I see a screenshot of what this looks like? |
I'm not sure we need to list all the names like this, as the list might expand into the hundreds or thousands. If we do list them, they should be ordered alphabetically. |
|
This PR allowed me to redeploy a project, so fixes the issue I had. 👍 |
|
The UI that I'd really like to have (for both cases) would show a partial list, and allow filtering it by typing. Then once you've narrowed it down you could use arrow keys to select one. Sort of like Given that we require them to type a valid slug, I wanted some way to convey the available options. I agree that this version of the UI definitely isn't very elegant in that regard. Maybe we could print the URL to the workspace's project listing instead? Sorting them would be a good improvement. If we want to keep the list in the CLI, I'll add that. |
| effects.clack.intro(`${inverse(" observable deploy ")} ${faint(`v${process.env.npm_package_version}`)}`); | ||
| effects.clack.intro( | ||
| `${inverse(" observable deploy ")}${ | ||
| process.env.npm_pacakge_version ? faint(` v${process.env.npm_package_version}`) : "" |
There was a problem hiding this comment.
This gets baked into the code during build, so it shouldn’t be necessary to check this.
Line 17 in 2bba99b
| @@ -772,6 +776,38 @@ export async function promptDeployTarget( | |||
| } else if (chosenProject !== null) { | |||
| return {create: false, workspace, project: existingProjects.find((p) => p.slug === chosenProject)!}; | |||
| } | |||
| } else if (existingProjects.length >= 10) { | |||
There was a problem hiding this comment.
Any harm in nesting if (existingProjects.length < 10) {…} else {…} here so that we don’t have to repeat the 10 twice and risk a chance that one of the conditions won’t be handled?
| const project = existingProjects.find((p) => p.slug === chosenProject); | ||
| if (!project) throw new Error("selected a non-existent project"); |
There was a problem hiding this comment.
Is this possible given the check earlier on L799? Just for safety?
|
I just noticed this appears to be fixed in the top-of-trunk version of clack, but that version has not yet been published to npm? But despite not happening automatically, it looks like in the latest released version we are using, it can be set manually, so maybe we should just set it to https://github.com/bombshell-dev/clack/releases/tag/%40clack%2Fprompts%400.7.0 |
|
Thanks for the review! I didn't realize that was a built-in Clack option. It might take me a little bit to get back to this though. |
|
Closing in favor of #1618 |
This should help when deploying projects to workspaces with many projects.