facebook / docusaurus

Easy to maintain open source documentation websites.
https://docusaurus.io
MIT License
56.97k stars 8.58k forks source link

[infima] restyle admonitions #2930

Closed maximousblk closed 4 years ago

maximousblk commented 4 years ago

💥 Proposal

The current (alpha56) admonitions do not look very good. Specially the "caution" admonition. Text on Yellow loses contrast in light mode and links using the default primary color lose contrast in "important" admonition.

And over all the colors are too bright for dark mode.

Have you read the Contributing Guidelines on issues?

yes

Pitch

I have experimented with some styles and created a demo. [code]

Preview:

84570593-11a18680-adac-11ea-9ab9-d488def89a42

You can compare differences between the new style and the current style.

The summary example is for #2924

slorber commented 4 years ago

so before we had:

image

image

after:

image

image


I like this new design, but fear that errors/warnings become less visible.

Wonder what others think.

maximousblk commented 4 years ago

If visibility is a concern then the v1 admonitions should work well.

image

I tried to recreate them but the colored background did not look very good in the dark mode (opinion). So I used the default background colors, but as stated, they became less visible. Then I opted for a color slightly off from the background which infima already had preset for (--ifm-color-emphasis-100).

r-tae commented 4 years ago

I like the new style, but I think doubling the left border would do a lot for visibility while still maintaining the style. Based on emulation, people with colour vision deficiencies will also have an easier time than with the thinner border.

maximousblk commented 4 years ago

I updated the demo according to @actual-size's idea any it certainly pops out more.

slorber commented 4 years ago

yes this is more visible

About the summary/details view, I liked the left border too, it seems you have removed it

maximousblk commented 4 years ago

@slorber yes that was certainly a mistake. It worked in my local environment but somehow broke in production.

I was trying to have some styles shared between the other admonitions and the details admonition which I have reverted and it works now.

slorber commented 4 years ago

After discussions with @yangshun we'd like to update our admonitions styles to your redesign proposal.

However Infima is not a public github repo for now, so the best option is that you give me your css and I update the styles there

maximousblk commented 4 years ago

@slorber That is a great news!

All I did was use infima's variables in the custom.css file to simulate what it should look like.

You can find the custom.css file here

The required properties are marked with a comment saying /* #2930 */

markerikson commented 4 years ago

FWIW, you can see how I restyled the admonitions for the Redux docs in this PR:

https://github.com/reduxjs/redux/pull/3740

Here's the specific file:

https://github.com/reduxjs/redux/blob/87c4edd77011f7f064d3653c595a5168558c9a5f/website/src/css/custom.css

Example appearances:

image

image

I believe I borrowed the colors from the Cypress.io docs, although I may have tweaked them a bit further:

image

slorber commented 4 years ago

Thanks @markerikson , that looks quite similar to the restyle proposal here

Do you have a page where these admonitions are used? I only found some pages with notes un purple.

BTW the style of Redux and Redux toolkit doc seems different regarding admonitions :)

markerikson commented 4 years ago

@slorber That PR isn't merged yet, and that's where I updated our DS version and added that admonitions styling. So, the PR preview link is the only place where it's visible yet:

https://deploy-preview-3740--redux-docs.netlify.app/tutorials/quick-start/quick-start-part-1

RTK's docs were still on alpha.43 until I pushed some doc updates yesterday, and we don't actually use "admonitions" in the RTK docs yet - just styled blockquotes.

slorber commented 4 years ago

Thanks,

Noticed you removed the dark mode in this PR (wanted to check the admonitions in dark mode). Curious if it's related to a D2 issue, or an explicit choice (might be related to https://github.com/facebook/docusaurus/pull/2939 ?)

markerikson commented 4 years ago

I didn't "remove" dark mode, I just created this branch months ago before dark mode actually got enabled in the Redux docs site :)

slorber commented 4 years ago

@maximousblk your link doesn't work anymore, can you host it somewhere else please? https://github.com/maximousblk/docusaurus-playground/blob/master/src/css/custom.css

@yangshun can we safely change the styles of infima alerts without affecting other FB projects?

Anyone wants to work on this?

Seems we need to change alerts in Infima, + the style preset of this repo: https://github.com/elviswolcott/remark-admonitions/blob/master/styles/infima.css

@elviswolcott what's the motivation to handle the preset on your repo instead of letting us maintain it?

maximousblk commented 4 years ago

@slorber so sorry for that. here is the css:

CSS ```css /* src/css/custom.css */ /* stylelint-disable docusaurus/copyright-header */ /** * Any CSS included here will be global. The classic template * bundles Infima by default. Infima is a CSS framework designed to * work well for content-centric websites. */ /* You can override the default Infima variables here. */ :root { --ifm-color-primary: #fa7548; --ifm-color-primary-dark: #fa6938; --ifm-color-primary-darker: #f95d29; --ifm-color-primary-darkest: #f95219; --ifm-color-primary-light: #fa8158; --ifm-color-primary-lighter: #fb8d67; --ifm-color-primary-lightest: #fb9877; --ifm-code-font-size: 95%; --ifm-color-success: var(--ifm-color-success-light); --ifm-color-info: var(--ifm-color-info-light); --ifm-color-warning: var(--ifm-color-warning-light); --ifm-color-danger: var(--ifm-color-danger-light); --ifm-global-radius: 0.2rem; --ra-admonition-icon-color: var(--ifm-alert-border-color); --ifm-alert-color: var(--ifm-alert-border-color); } .docusaurus-highlight-code-line { background-color: rgb(72, 77, 91); display: block; margin: 0 calc(-1 * var(--ifm-pre-padding)); padding: 0 var(--ifm-pre-padding); } .alert { background-color: var(--ifm-background-surface-color); border-color: var(--ifm-alert-border-color); color: var(--ifm-font-color-base); } .alert a { color: var(--ifm-color-primary); text-decoration: var(--ifm-link-decoration); } .alert .admonition-heading { color: var(--ifm-font-color-base); } .admonition-icon svg { fill: var(--ifm-alert-border-color); stroke: var(--ifm-alert-border-color); margin-right: 12px; } a:hover { text-decoration: underline; } .admonition { border: none; background-color: var(--ifm-color-emphasis-100); border-radius: 0; border-left-width: 0.3rem; border-left-style: solid; border-color: var(--ifm-alert-border-color); } details { background-color: var(--ifm-color-emphasis-100); border-radius: 0; border-left-width: 0.3rem; border-left-style: solid; border-color: var(--ifm-color-emphasis-900); padding: var(--ifm-alert-padding-vertical) var(--ifm-alert-padding-horizontal); margin-bottom: 1em; } summary { outline: none; font-weight: bold; cursor: pointer; } details[open] summary { margin-bottom: 1em; } .alert--secondary { --ifm-alert-background-color: var(--ifm-color-emphasis-900); --ifm-alert-border-color: var(--ifm-color-emphasis-900); } ```
slorber commented 4 years ago

thanks :)

Thor-x86 commented 4 years ago

I like this new design, but fear that errors/warnings become less visible.

I think those will be better if the title's color match with left-side line, but make it darker on light mode and lighter on dark mode

EDIT:

light

dark

elviswolcott commented 4 years ago

@slorber the docusaurus preset lives in this repo. All the remark-admonitions package does is provide the remark plugin and optional styles. There's nothing stopping docusaurus from maintaining it's own stylesheet here.

slorber commented 4 years ago

Thanks, we should probably do that, as it would be more convenient to maintain

jcs98 commented 4 years ago

@slorber I would love to take this up as a first-issue for docusaurus as part of mlh Can you please help me with a starting point?

slorber commented 4 years ago

Hi @jcs98 ,

This is part of Infima alerts(https://facebookincubator.github.io/infima/docs/components/alert), and kind of related to if we open-source it or not (https://github.com/facebook/docusaurus/issues/3575). We'll figure this out soon but this is not a good time to work on this until then.

HonkingGoose commented 4 years ago

I really like the improvements that are suggested here (colored bar left side of adminition, colored icon, and colored title), keep up the good work. :+1: :heart:

I hope this lands into the default Docaurus framework soon, so that I can benefit from this in my project! :smile:

yangshun commented 4 years ago

Refer to issue moved to the Infima repo - https://github.com/facebookincubator/infima/issues/55