refactor(cli): improve output aesthetics #6997

innocenzi · 2022-02-20T02:22:55Z

Description

This PR is a minor improvement to the CLI output of the development server. While working on a plugin, I wanted to add some useful information to the terminal, and I thought the current output could be improved a bit.

Note: don't feel bad for straight up closing this PR - this is a small proposal and it would be totally fine if it was refused. Also I'm totally open on iterating over the design.

Output after this PR

Initial suggestion

Current output

What is the purpose of this pull request?

Bug fix
New Feature
Documentation update
Other

Before submitting the PR, please make sure you do the following

Read the Contributing Guidelines.
Read the Pull Request Guidelines and follow the Commit Convention.
Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
Ideally, include relevant tests that fail without this PR but pass with it.

patak-dev · 2022-02-20T06:10:49Z

Nice! Good to try new things here. Some comments about the changes:

I like the new arrows. I think we could keep them.
I think that it is the right move to dim the "use host to expose" string, that info is not relevant to everyone. And now enough time has passed since we change the default. Probably it is too much dimmed though, I'm afraid we may have a11y issues. Maybe we could light it up a bit?
I like that the addresses are aligned now! Maybe we could also drop the : to make it cleaner?
dimming the starting time is interesting. I don't think we should do it yet. IIUC, the idea is to make it prominent to contrast with other bundlers. Maybe at one point, we could directly remove it? I vote to leave it as it was for now.
For the title, I prefer that it stays as it was. Maybe we should capitalize Vite though.

bluwy · 2022-02-20T07:13:09Z

Perhaps another thing to consider is how this would look with plugins that add other CLI entries, e.g. vite-plugin-inspect and vite-plugin-qrcode. With this change they may look slightly out of place.

Design-wise I still prefer the old one, but mostly for nostalgia reasons 😄

patak-dev · 2022-02-20T09:25:06Z

That is a good point @bluwy. Maybe we could merge this PR (with modifications), or other PRs when we are in the Vite 3.0 beta? I think it may help then to have a refreshed CLI look. And plugins and frameworks would also be able to better target the style for the major

Shinigami92 · 2022-02-20T09:36:16Z

The background color of VITE v2.8.4 is to bright and laggs accessibility in contrast

Otherwise I like it, especially that it now is a bit (2 lines) smaller

packages/vite/src/node/cli.ts

innocenzi · 2022-02-20T14:39:40Z

Hey all, there's a lot of valid feedback, thanks!

@bluwy
Perhaps another thing to consider is how this would look with plugins that add other CLI entries

I had the intent to PR vite-plugin-inspect if this was merged to update the design - I didn't know about vite-plugin-qrcode though (pretty cool, after taking a look!). That was when I worked on such a feature that I had the idea for this PR actually! Here is an example:

On this note, I found out that the new line at the end of the default logging design denies a potentially interesting design for plugins, which would be to simply add a line after the default ones:

I'm thinking that Vite could offer a basic API to add plugin-specific logging like this, instead of having to hook into configureServer and use server.config.logger.info directly. Maybe I'll explore this in another PR, this is out of scope for this one.

@Shinigami92
The background color of VITE v2.8.4 is to bright and laggs accessibility in contrast

@patak-dev
Probably it is too much dimmed though, I'm afraid we may have a11y issues.

Good point!

I'm not sure how to handle this because I only use standard terminal escape codes, which don't render the same depending on your terminal theme. In this case, I used gray for the dimmed parts to make it consistent between VSC and WT, because VSC doesn't render a different color using the standard dim, while WT uses a different opacity.

1️⃣ Here is what it looks like with VSC's default theme:

2️⃣ Here is an alternative design which uses bold for non-dimmed parts instead:

@patak-dev
For the title, I prefer that it stays as it was. Maybe we should capitalize Vite though.

Honestly that was the hardest part of the design haha. I took inspiration from Vitest on this one. I like the contrast between the header part and the rest of the data.

3️⃣ Here is something that looks more like the current output:

4️⃣ Or with an offset:

@patak-dev
I like that the addresses are aligned now! Maybe we could also drop the : to make it cleaner?

5️⃣ Something like this?

Personal thoughts on your feedback (thanks again!):

My favorite design in this comment is 4️⃣.
I think it's okay to keep the dimmed parts gray because people with accessibility issues would most likely have themes with more contrasted colors.

innocenzi · 2022-02-20T14:47:16Z

For now I changed this PR to use design 4️⃣.

Example: vite-plugin-inspect and vite-plugin-laravel with this design:

Shinigami92 · 2022-02-20T14:56:49Z

From the screenshots, I also like 4️⃣ the most

patak-dev · 2022-02-20T16:20:50Z

I think design 4 looks great (also 5 to me looks equally good)! And I like your idea of letting plugins like inspect provide other URLs to add to the list.

About using a background, I think it would look better if the font color will be black instead of white. I think Vitest is using that design as a play with the typical PASS, FAIL.

gray being ok for a11y because people may have their terminal theme adjusted could work... I don't know much about a11y in terminals to know, maybe we need to ask for help on this one.

IMO, Vite looks nicer than VITE (maybe it is just that I'm used to it). But the all caps version may work better after 3.0, because we are going to change the default dev port to 5173

VITE
5173

innocenzi · 2022-02-20T16:57:58Z

@patak-dev
About using a background, I think it would look better if the font color will be black instead of white. I think Vitest is using that design as a play with the typical PASS, FAIL.

I personally have the feeling like white is clearer - but both gray and white don't pass WCAG AAA anyway :/

@patak-dev
IMO, Vite looks nicer than VITE

I am neutral about the casing of Vite vs VITE, so if you would like me to update the PR to title case instead of upper case, I can :)

(I didn't realize the new port was a leet version of Vite haha)

patak-dev · 2022-02-20T16:59:39Z

It is kind of a secret, dont tell others. I vote for all caps 😁

bholmesdev · 2022-03-10T19:00:53Z

Sorry to throw a wrench in this with another idea, but I'm really digging the |s we added to the new Astro CLI! Here's our output for reference. Also a big fan of using a block background for the framework name; adds some nice emphasis across light and dark themes.

antfu

Love it! Also nice to have it as 3.0 so ppl could perceive more on the improvements 🫣

packages/vite/src/node/cli.ts

patak-dev · 2022-05-19T17:14:10Z

Love it! Also nice to have it as 3.0 so ppl could perceive more on the improvements 🫣

Totally! With this and the new docs, the impression of Vite will be different and that is good not only for the perceived progress. I think it is also good for users to easily identify if they are running v2 or v3 (or are reading the v2 or v3 docs)

ydcjeff · 2022-05-20T12:48:57Z

packages/vite/src/node/logger.ts

+  )
+  urls.forEach(({ label, url: text }) => {
+    info(
+      `    ${colors.green('➜')}  ${colors.bold(label)}: ${' '.repeat(


Love the design. I prefer 2 spaces here like in No.3 design since I think the extra 2 spaces don't provide special meaning here.

I also prefer 2 spaces here. It is just a detail but aligning with the start of VITE feels nicer.

patak-dev · 2022-05-20T21:07:28Z

We talked about the PR in today's team meeting, let's do this! ❤️
@innocenzi let's discuss the 2 vs 4 spaces proposal from @ydcjeff, and then we can merge it. If we have other ideas later, we can continue to iterate. @antfu mentioned today about an API for plugins to inject extra lines like vite-plugin-inspector. We can explore that in a future PR.

innocenzi · 2022-05-20T21:14:16Z

Personally I prefer it indented since it creates a sort of block. I like the look of the indentation. That being said I'm totally fine with only two spaces.

Whichever you chose, I'm not home until Monday so I can't update the PR - anyone with write access to the target branch, feel free to update my PR :p

I also dig the idea to provide an API to inject lines to the output! And I agree it's out of scope there and should be discussed in another PR.

patak-dev · 2022-05-20T21:22:11Z

I can update the PR to what we decide 👍🏼, don't worry. Thanks again for pushing for this change!

I see your point about creating a block. But to me with fewer spaces you also get a block because the arrows are small and your eyes focus on the start of Local and Network. I just tested and to me, 3 spaces could also work 👀

innocenzi · 2022-05-20T21:33:01Z

Haha, I'm fine with either honestly. So if the majority prefers 2, let's do 2. I like 3 as well. :p

Maybe we can vote?

2 spaces, 👍
3 spaces, 🚀
4 spaces, 👀

bholmesdev · 2022-05-20T23:07:57Z

Haha, I'm fine with either honestly. So if the majority prefers 2, let's do 2. I like 3 as well. :p

Maybe we can vote?

2 spaces, 👍

3 spaces, 🚀

4 spaces, 👀

I'll take the "I'm feeling lucky, let's ship it" option 🎲

patak-dev

Looks like 2 spaces won 👀
At least until we have an API for plugins to use to inject their own lines, I think it isn't a bad idea to use 2 so we don't need to update them right away
We could review again once we expose that API

Final state of the CLI with this PR

innocenzi · 2022-05-24T17:45:38Z

Nice. Thanks for merging!

patak-dev · 2022-05-24T17:59:40Z

Thank you for pushing this to the finish line @innocenzi 🙌🏼
It will make a big diff once we launch v3!

patak-dev added this to the 3.0 milestone Feb 20, 2022

patak-dev added the p2-nice-to-have Not breaking anything but nice to have (priority) label Feb 20, 2022

Shinigami92 reviewed Feb 20, 2022

View reviewed changes

packages/vite/src/node/cli.ts Outdated Show resolved Hide resolved

Shinigami92 previously approved these changes Feb 20, 2022

View reviewed changes

innocenzi mentioned this pull request May 10, 2022

Asset Not Loaded When Integrating With Existing Backend #7778

Closed

7 tasks

innocenzi dismissed Shinigami92’s stale review via 56e16af May 18, 2022 11:08

innocenzi added 4 commits May 18, 2022 13:11

refactor(cli): improve output aesthetics

2c398c4

refactor(cli): rename c to colors

62825c8

refactor(cli): slightly change output design

572dc01

refactor(cli): update output offsets

87c0faa

Shinigami92 self-requested a review May 18, 2022 14:56

Shinigami92 previously approved these changes May 18, 2022

View reviewed changes

antfu previously approved these changes May 19, 2022

View reviewed changes

antfu reviewed May 19, 2022

View reviewed changes

packages/vite/src/node/cli.ts Outdated Show resolved Hide resolved

refactor: remove parenthesis around ready time

b7f3360

innocenzi dismissed stale reviews from antfu and Shinigami92 via b7f3360 May 19, 2022 16:57

ydcjeff reviewed May 20, 2022

View reviewed changes

patak-dev added 2 commits May 24, 2022 17:53

chore: merge main

745536f

chore: use 2 spaces

1d30997

patak-dev approved these changes May 24, 2022

View reviewed changes

ydcjeff approved these changes May 24, 2022

View reviewed changes

patak-dev merged commit 809ab47 into vitejs:main May 24, 2022

innocenzi deleted the refactor/cli-output branch May 24, 2022 17:45

sapphi-red mentioned this pull request Jul 8, 2022

refactor!: move side effect of restart server to the caller #8746

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(cli): improve output aesthetics #6997

refactor(cli): improve output aesthetics #6997

innocenzi commented Feb 20, 2022 •

edited

patak-dev commented Feb 20, 2022

bluwy commented Feb 20, 2022

patak-dev commented Feb 20, 2022

Shinigami92 commented Feb 20, 2022 •

edited

innocenzi commented Feb 20, 2022 •

edited

innocenzi commented Feb 20, 2022 •

edited

Shinigami92 commented Feb 20, 2022

patak-dev commented Feb 20, 2022 •

edited

innocenzi commented Feb 20, 2022

patak-dev commented Feb 20, 2022

bholmesdev commented Mar 10, 2022

antfu left a comment

patak-dev commented May 19, 2022

ydcjeff May 20, 2022

patak-dev May 20, 2022

patak-dev commented May 20, 2022

innocenzi commented May 20, 2022

patak-dev commented May 20, 2022

innocenzi commented May 20, 2022

bholmesdev commented May 20, 2022

patak-dev left a comment

innocenzi commented May 24, 2022

patak-dev commented May 24, 2022

refactor(cli): improve output aesthetics #6997

refactor(cli): improve output aesthetics #6997

Conversation

innocenzi commented Feb 20, 2022 • edited

Description

Output after this PR

Current output

What is the purpose of this pull request?

Before submitting the PR, please make sure you do the following

patak-dev commented Feb 20, 2022

bluwy commented Feb 20, 2022

patak-dev commented Feb 20, 2022

Shinigami92 commented Feb 20, 2022 • edited

innocenzi commented Feb 20, 2022 • edited

innocenzi commented Feb 20, 2022 • edited

Shinigami92 commented Feb 20, 2022

patak-dev commented Feb 20, 2022 • edited

innocenzi commented Feb 20, 2022

patak-dev commented Feb 20, 2022

bholmesdev commented Mar 10, 2022

antfu left a comment

Choose a reason for hiding this comment

patak-dev commented May 19, 2022

ydcjeff May 20, 2022

Choose a reason for hiding this comment

patak-dev May 20, 2022

Choose a reason for hiding this comment

patak-dev commented May 20, 2022

innocenzi commented May 20, 2022

patak-dev commented May 20, 2022

innocenzi commented May 20, 2022

bholmesdev commented May 20, 2022

patak-dev left a comment

Choose a reason for hiding this comment

innocenzi commented May 24, 2022

patak-dev commented May 24, 2022

innocenzi commented Feb 20, 2022 •

edited

Shinigami92 commented Feb 20, 2022 •

edited

innocenzi commented Feb 20, 2022 •

edited

innocenzi commented Feb 20, 2022 •

edited

patak-dev commented Feb 20, 2022 •

edited