Skip to content

Fix fund colors#1593

Closed
ruyadorno wants to merge 2 commits intonpm:release/v7.0.0-betafrom
ruyadorno:fix-fund-colors
Closed

Fix fund colors#1593
ruyadorno wants to merge 2 commits intonpm:release/v7.0.0-betafrom
ruyadorno:fix-fund-colors

Conversation

@ruyadorno
Copy link
Copy Markdown
Contributor

⚠️ Merge #1582 first (this builds on top of that)

Colors were always missing from fund, since they were somewhat messed up on npm ls it never bothered me that much but now that they're fixed there it hurts my eye to see the raw output from npm fund - I'm adding at least the default chalk color so that it looks better with terminal color schemes.

Before

Screen Shot 2020-07-31 at 4 28 18 PM

After

Screen Shot 2020-07-31 at 4 28 28 PM

npm ls (for ref)

Screen Shot 2020-07-31 at 4 44 37 PM

Refactored `npm fund` tests to use new `test/lib/` unit tests structure.

ref: npm/statusboard#151
@ruyadorno ruyadorno requested a review from isaacs July 31, 2020 20:45
@ruyadorno ruyadorno requested a review from a team as a code owner July 31, 2020 20:45
@ruyadorno ruyadorno added Enhancement new feature or improvement Release 7.x work is associated with a specific npm 7 release labels Jul 31, 2020
@isaacs
Copy link
Copy Markdown
Contributor

isaacs commented Aug 4, 2020

Are you ok with throwing a bgBlack in there as well? Setting a foreground color without a background color can be a problem if someone has a light terminal background.

isaacs pushed a commit that referenced this pull request Aug 4, 2020
PR-URL: #1593
Credit: @ruyadorno
Close: #1593
Reviewed-by: @isaacs

EDIT(@isaacs): Added black background to the highlighted text so that it
won't disappear on light terminal settings.
@isaacs
Copy link
Copy Markdown
Contributor

isaacs commented Aug 4, 2020

Landed on 4906ee5, thanks!

@isaacs isaacs closed this Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement new feature or improvement Release 7.x work is associated with a specific npm 7 release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants