[cli] vc project ls visual updates
#8157
Conversation
MatthewStanciu
changed the title
vc project ls visual updatesvc project ls visual updates
MatthewStanciu
requested review from
TooTallNate,
EndangeredMassa and
styfle
as code owners
|
Would be great IMO if we could use a clickable link to the project page under the project name section, instead of having an entire column for that. I'd prefer to print something like the production domain URL in that new column instead (also clickable 😄). It's also not super clear to me what "Status" section is referring to, I'm not entirely convinced we need that column (the project card on the Vercel dashboard doesn't have the status dot, for comparison). |
| .map(project => [ | ||
| [ | ||
| project.name, | ||
| chalk.bold(getInspectUrl(contextName, project.name)), | ||
| stateString(project.latestDeployments[0]?.readyState), | ||
| chalk.gray(ms(Date.now() - project.updatedAt)), | ||
| ], | ||
| ]) | ||
| // flatten since the previous step returns a nested | ||
| // array of the deployment and (optionally) its instances | ||
| .flat(), |
There was a problem hiding this comment.
could you just remove the nested array now? It looks like its unnecessary
There was a problem hiding this comment.
The nested array is necessary—it's creating an array containing the pieces of data to appear under the header for each project, generating:
['project name', 'manage', 'updated'],
['name 1', 'vercel.com/user/name-1', '35s'],
['name 2', 'vercel.com/user/name-2', '1m']
// ...etc
|
|
||
| export function stateString(s: string) { | ||
| const CIRCLE = '● '; | ||
| const sTitle = s && title(s); |
There was a problem hiding this comment.
s can't be undefined or null here (by type declaration) so remove the s && part.
Also, fwiw this kinda thing is a code smell IMO. && should really only be used for conditional expressions. I'd rather see an explicit s ? title(s) : 's is null/undefined case' expression instead
There was a problem hiding this comment.
👍 Decided to remove the state string altogether after seeing Nate's comment
|
@TooTallNate @Ethan-Arrowood curious to hear your thoughts on this new behavior with
My concern is that I'm not sure if it's bad to have the command behave differently based on whether the user's terminal supports hyperlinks. But also I think the command needs to look good on terminals that don't support hyperlinks. |
| [ | ||
| chalk.bold(project.name), | ||
| getLatestProdUrl(project), | ||
| chalk.gray(ms(Date.now() - project.updatedAt)), |
There was a problem hiding this comment.
Was it intentional to drop the "ago" part?
Related to #8102 & #8151. This PR updates the look of
vc project ls:Before
After
📋 Checklist
Tests
yarn test-unitCode Review