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
doc: add 'not recommended' blockquotes #52814
Conversation
Review requested:
|
While I'm fine with the idea, the white text on the red background is difficult for my eyes to read. |
Good to note! What color scheme would you recommend? |
I honestly don't know what to recommend :-/ ... |
I'll experiment. Right now it's the same as |
That's a LOT more readable to my eyes. |
(Request Review) @nodejs/documentation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume I'm a bit out of the loop here, but why are we not sticking with existing concepts such as <strong class="critical">
and maybe restyle that a bit? And if you really want to introduce a custom syntax for blockquotes, why not adopt GitHub's mechanism?
eab9ac6
to
4e8dd52
Compare
I decided to go with the same syntax that the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UI/UX is LGTM in my opinion !!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel that the styling is a bit over-the-top for the "severity" of these "warnings", which I'd say are more recommendations than actual "warnings".
To be honest, the idea behind is legitimate, but I'm not sure about adoption + how useful this would be for the reader. I do believe some distinction could bring benefits, but not sure what sort of change is warranted here.
Not to mention you're technically changing the spec we have for our docs but not documenting it anywhere. We have a guide for the API docs, you should update that guide too.
Lastly, unrelated, and this PR can land regardless, but, we're about to start a revamp on the tooling behind the API docs, so this code will be discarded; If this PR lands I'll create a counterpart code to handle this, but I believe we should handle this differently.
I'm extremely against using magic "string searches" with prefixes, such as a string that starts with "Warning:" to then apply styling on it. It's a bad design pattern, and should be avoided. I strongly agree with @tniessen here; We could simply surround these with HTML elements that represent such "quotes" or do it as GitHub does with the symbols in the beginning...
True, your right that the the redesign will incorporate a lot of changes, so I'll close this, but I recommend something similar be included in the redesign |
Feel free to go over the Figmas and check if we have something similar like this already, otherwise, feel free to make suggestions :) |
Oh! I didn't know we had figmas, I'll take a look! |
Fixes: #52743
This PR adds the ability to add a warning blockquote via the following syntax:
> Warning: ...
Which will be rendered as: