Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace level list with table in readme.md #367

Merged
merged 2 commits into from
Oct 1, 2019

Conversation

pedrottimark
Copy link
Contributor

@pedrottimark pedrottimark commented Sep 30, 2019

First of all, thank you for this excellent package <3

To use 256 background colors when available, I tripped over 0-based versus 1-based level

Although readme.md has 0-based numbered list, here is picture of the list on npm page:

chalk-level-list

Here is picture of the proposed table in markdown preview of code editor:

chalk-level-table

Updated picture according to review comments:

chalk-level-table-2

readme.md Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
@@ -132,12 +132,12 @@ If you need to change this in a reusable module, create a new instance:
const ctx = new chalk.Instance({level: 0});
```

Levels are as follows:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed like the heading row of the table communicates the same information.

I am happy to put it back if you prefer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, I guess they do. I don't have a strong opinion about it so let's remove it for now unless somebody else wants to keep it.

@Qix-
Copy link
Member

Qix- commented Oct 1, 2019

Good catch, thank you :)

@Qix- Qix- merged commit eef8c8c into chalk:master Oct 1, 2019
@pedrottimark pedrottimark deleted the level-table branch October 1, 2019 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants