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

Recommendation / Fixable "Badges" #4355

Closed
knpwrs opened this Issue Nov 7, 2015 · 33 comments

Comments

Projects
None yet
9 participants
@knpwrs
Copy link
Contributor

knpwrs commented Nov 7, 2015

As brought up in #4354, I think it would be a good idea to discuss how to make recommended / fixable rules stand out more on the rules list. My pull request implemented the following:

image

image

image

My implementation makes an attempt at still using markdown for that page. My original idea was to just wrap recommendation and fixable in backticks to get them to be inline code blocks and then target those in css using li > code; however, then you have no guarantee you are selecting a recommended badge vs a fixable badge.

@eslintbot

This comment has been minimized.

Copy link

eslintbot commented Nov 7, 2015

Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Nov 7, 2015

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Nov 8, 2015

I think it's an idea worth exploring. I'm not a fan of this treatment, but if someone can come up with a nice treatment for it, seems like a nice addition.

@knpwrs

This comment has been minimized.

Copy link
Contributor Author

knpwrs commented Nov 8, 2015

Just curious, are you not a fan of the design, implementation, or both? Design-wise I just took the inline code blocks and hue-shifted them.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Nov 9, 2015

I'm not a fan all around. The colors look awful to me, it makes the page look like holiday lights.

@platinumazure

This comment has been minimized.

Copy link
Member

platinumazure commented Nov 9, 2015

The design looks okay to me, but I can see an argument for fewer colors for ease of contrast. What sort of thing would you like to see instead?

For example, would it look better if it were more tabular, where there would be a column for fixable and a column for recommended?

@knpwrs

This comment has been minimized.

Copy link
Contributor Author

knpwrs commented Nov 9, 2015

Tables are something I had considered as well. I think the main concern there is responsive design.

@IanVS

This comment has been minimized.

Copy link
Member

IanVS commented Nov 10, 2015

How about a subtle wrench icon to indicate fixable rules?

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Nov 10, 2015

In that case we should also change recommended to something like a check mark.

@IanVS

This comment has been minimized.

Copy link
Member

IanVS commented Nov 10, 2015

I agree with being consistent about icons/words, but not sure if a check mark is immediately obvious. What about a thumbs-up?

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Nov 10, 2015

Sure, I'm not really set on any icon in particular. We should probably see what's available in default bootstrap icon font. Maybe something like glyphicon-thumbs-up or glyphicon-star

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Nov 11, 2015

I'm not a designer, I just know what I like and what I don't when I see it. :)

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Jan 16, 2016

I like this idea - still looking for someone to take this on? I like the idea of having icons that represent these categories. Bootstrap's glyphicon-wrench and glyphicon-star make a lot of sense to me.

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Jan 17, 2016

@kaicataldo Yes. Do you want to make local change and post a screenshot?

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Jan 21, 2016

I was thinking something along these lines? Thoughts?

screen shot 2016-01-20 at 11 06 51 pm

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Jan 21, 2016

Here's the same thing with the icons included in the intro above:

screen shot 2016-01-20 at 11 10 22 pm

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Jan 21, 2016

I think that looks pretty good. The only thing I would do is separate legend from the text, just before "Possible Errors" I would put "recommended - checkmark, fixable - wrench" in larger bold font.

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Jan 21, 2016

That's a much better solution :) I mocked this up quickly in the browser for the screenshots. Do you want me to make a PR? I'm a bit confused as to where these changes need to be made. I read the note about files with this in them:
<!-- Note: No pull requests accepted for this file. See README.md in the root directory for details. -->
But wasn't clear on what I should do. Thanks!

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Jan 21, 2016

The file is located here: https://github.com/eslint/eslint/blob/master/docs/rules/README.md but it's a markdown file, so most of this has to be done through css. You can take a look at how @knpwrs did that in the original PR here: #4354 and https://github.com/eslint/eslint.github.io/pull/153/files

kaicataldo added a commit to kaicataldo/eslint.github.io that referenced this issue Jan 21, 2016

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Jan 21, 2016

Created a PR - mostly just search and replace :D TIL you can write HTML in markdown files, so I took that approach rather than the pure CSS approach above. This is a screenshot from my locally served version of the site:
screen shot 2016-01-21 at 1 56 31 am
Definitely makes it easier to see which rules are recommended and fixable. Let me know if you need anything else.

@IanVS

This comment has been minimized.

Copy link
Member

IanVS commented Jan 21, 2016

I like it a lot, personally. Thanks for stepping up, @kaicataldo!

I do wonder a little though, without seeing the full result, how visually distinct are the wrench and checkmark? They both have a very similar shape, so if there are a few rules next to each other with alternating checks and wrenches, are they hard to tell apart?

@hzoo

This comment has been minimized.

Copy link
Member

hzoo commented Jan 21, 2016

Maybe you could differentiate better with colors or switching the orientation of the wrench

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Jan 21, 2016

@IanVS That was a concern of mine as well when picking the icons, but I felt like they were easy enough to differentiate once I had implemented it. I can take some more screenshots of other sections with both icons when I get back to my computer.

I initially tried the star icon, but felt like it was visually noisier (being solid and a larger icon). I chose the checkmark because a) I felt like it conveyed the intention clearly and b) the thinner profile of the icon made it less distracting from the text. My goal was to improve the clarity of which rules are recommended or fixable without distracting too much from the rule itself.

Changing the icon is super easy though, so happy to try out some other ones if others don't feel the same way!

@IanVS

This comment has been minimized.

Copy link
Member

IanVS commented Jan 21, 2016

Sure, I'm fine with that approach. It was really more of a question of how it looks, since I couldn't tell from the screenshot.

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Jan 21, 2016

I'm happy to upload more screenshots when I get home tonight :)

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Jan 21, 2016

Also, just realized I never actually opened a PR - remedied!

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Jan 21, 2016

Just mentioning what I mentioned over on the eslint.github.io repo:

I think we should keep using "(recommended)" and "(fixable)" in the .md files in this repo and then replace them with images during the build process. I'd prefer not to ask people to write HTML in the .md files, and also, avoiding HTML allows us to more easily make changes later.

And, we need to be sure the images are accessible, so either use actual <img> tags with alt or something else that provides descriptive text (aria-label, title, etc.).

@BYK

This comment has been minimized.

Copy link
Member

BYK commented Jan 21, 2016

@nzakas - I feel like https://github.com/wooorm/remark can do that (it can also replace our markdown linter via https://github.com/wooorm/remark-lint)

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Jan 22, 2016

@BYK We are using github pages, I don't think they can use remark.

@BYK

This comment has been minimized.

Copy link
Member

BYK commented Jan 22, 2016

@ilyavolodin can't we use remark as a processing stage? Like master would be the raw version and "remarked version" would go to gh-pages. Dunno, just thinking aloud here.

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Jan 22, 2016

In all honesty, I wouldn't spend too much time on those enhancements. Once we have metadata in all of the rules, this page will be auto-generated anyways.

kaicataldo added a commit to kaicataldo/eslint.github.io that referenced this issue Jan 22, 2016

kaicataldo added a commit to kaicataldo/eslint.github.io that referenced this issue Jan 22, 2016

@kaicataldo

This comment has been minimized.

Copy link
Member

kaicataldo commented Jan 22, 2016

Updated the PR to simply replace '(recommended)' and '(fixable)' with their respective icons. Much nicer solution. The only other question I have is if you want me to change the index.md content to remove the icons from the intro and make a legend like my original PR did. If so, I'll open a PR to the main repo with the changes.

kaicataldo added a commit to kaicataldo/eslint.github.io that referenced this issue Jan 22, 2016

kaicataldo added a commit to kaicataldo/eslint.github.io that referenced this issue Jan 22, 2016

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Jan 23, 2016

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Jan 23, 2016

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Jan 23, 2016

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Jan 23, 2016

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Jan 23, 2016

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Jan 23, 2016

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Jan 23, 2016

ilyavolodin added a commit that referenced this issue Jan 23, 2016

Merge pull request #5041 from kaicataldo/fixes4355
Docs: Update readme for legend describing rules icons (refs #4355)

nzakas added a commit that referenced this issue Jan 24, 2016

gyandeeps added a commit that referenced this issue Jan 24, 2016

Merge pull request #5057 from eslint/revert-5041-fixes4355
Revert "Docs: Update readme for legend describing rules icons (refs #4355)"

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.