eslint-doc-generator icon indicating copy to clipboard operation
eslint-doc-generator copied to clipboard

Tweak emoji placement in rule doc config notice when combination of enabled/warn/disabled present

Open bmish opened this issue 2 years ago β€’ 3 comments

Received feedback that the two leading emojis in this example rule doc configs notice could be confusing (https://github.com/jsx-eslint/eslint-plugin-react/pull/3499#discussion_r1034192984).

Proposal 1

We could move the emojis next to their respective sentences to keep this as a consolidated notice.

Pros:

  • Keep to a single line for minimizing vertical space needed
  • More consistent with our other consolidated fixableAndHasSuggestions notice

Cons:

  • The notice's leading emoji doesn't fully represent the contents of the line anymore

Proposal 2

We could split these into separate notices/lines, similar to how we separated the rule list configs column into multiple columns based on severity.

Pros:

  • Maximize clarity
  • Maximize aesthetics
  • Avoids attempting to convey multiple distinct pieces of information in a single line

Cons:

  • Uses additional vertical space
  • Does not give the user the option to choose whether they want the severities consolidated into one line or split into multiple lines

Proposal 3

Split configs notice into configsError,configsWarn,configsOff, same as how we did for the configs column, so the user can control exactly where these notices go.

Pros:

  • Same benefits as Proposal 2
  • Allows us to keep the configs notice type for people who want to consolidate all the severities into a single line, while having separate notice types for those who want to use separate lines (similar to fixableAndHasSuggestions vs fixable and hasSuggestions)

Cons:

  • Same benefits as Proposal 2
  • More added complexity having to manage these notices separately instead of all together
  • If we want to change the default behavior of this, it would require a breaking change
  • Possibly overkill for now and we can switch to this later if there's a need

(current)

πŸ’ΌπŸš« This rule is enabled in the β˜‘οΈ recommended config. This rule is disabled in the jsx-runtime config.


(Proposal 1: consolidated with emojis next to corresponding sentence)

πŸ’Ό This rule is enabled in the β˜‘οΈ recommended config. 🚫 This rule is disabled in the jsx-runtime config.


(Proposal 2/3: separate lines)

πŸ’Ό This rule is enabled in the β˜‘οΈ recommended config.

🚫 This rule is disabled in the jsx-runtime config.

bmish avatar Nov 29 '22 01:11 bmish

Why not separate lines? Adding a newline doesn't seem that complex.

ljharb avatar Nov 29 '22 03:11 ljharb

@ljharb I've implemented separate lines (proposal 2 in the issue description) in #311. The implementation is trivial. The situation would have been more complex had I gone with Proposal 3 which would give the user the flexibility to choose whether they want to have everything on a single line or separate lines, but that's probably overkill for now.

bmish avatar Nov 29 '22 15:11 bmish

I'm going to ponder on this a bit more to decide if I want to bite the bullet and try out Proposal 3 for better future-proofing.

bmish avatar Dec 01 '22 14:12 bmish