facebook / docusaurus

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

Details can't use `alert--success` styling #6632

Closed csestito closed 2 years ago

csestito commented 2 years ago

Have you read the Contributing Guidelines on issues?

Prerequisites

Description

Details uses the alert styling, and by default adds the alert alert--info className to the element as seen here. When using the details component and adding alert alert--success to change the coloring of the alert, the default alert--info styles take over. The element in this case has alert alert--info alert alert--success in the className. While other alert styles such as alert--danger work, the alert--success falls victim to css.

Steps to reproduce

  1. Use a details component with added className alert alert--success

This can be reproduced on docusaurus.io by adding those classNames to the details element on this page .

Expected behavior

Details turns green

Actual behavior

Details stays blue.

It look like it's because of the ordering of the css for alert--success and alert--info, but root cause is the alert--info being added to the details by default.

Happy to fix this, just want to understand if it is preferred to reorder css to avoid this issue, or refactor details component to take status' into account.

Your environment

Reproducible demo

No response

Self-service

Josh-Cena commented 2 years ago

Hi! This is something about the CSS being called "cascading". When you have two class names that try to define the same property, the one that comes later in the style sheet "wins". And it happens that alert--success comes before alert--info in the style sheet, hence its style gets overwritten.

If you are unhappy with the default style, you should swizzle Details and disable its built-in style altogether, maybe using a prop to optionally apply the default style.

csestito commented 2 years ago

Hi @Josh-Cena, I am aware this is a cascading issue. It's an easy fix for our site just by defining the alert--success in our custom.css to win the css battle. My issue may have been more of a feature request which I am willing to take on.

Given that details is just reusing the alert styling, it makes sense to me to allow it to take advantage of the different alert types in order to easily provide different, yet standard, colors to each of the details, similar to admonitions, buttons, and alerts. An easy win can be reordering the alert styles so that if someone adds a different alert--<status> it will change the styling. Another option is to build out a separate details style so we don't have to default to alert--info.

If this isn't something the project is interested in I can leave this closed.

What it looks like today

Screen Shot 2022-02-08 at 7 25 43 AM

What it could look like

Screen Shot 2022-02-08 at 7 26 48 AM
slorber commented 2 years ago

Interesting.

Curious, what's the concrete use-case to have such styling?

Can you share actual doc pages or screenshots of real content?


Note our details/summary design is a bit too opinionated and we'll move back to something soberer

https://github.com/facebookincubator/infima/issues/197 https://github.com/facebook/docusaurus/discussions/5749

Do you think it's valuable keep a way to style those details similarly to admonitions?

csestito commented 2 years ago

@slorber The project that initially ignited the idea was API Docs to display responses in color-coding. Responses can be long and the ability to collapse is convenient.

When we realized how details is currently working, we realized it is pretty low effort to give users the ability to color-code their details similar to admonitions, alerts, buttons, etc. We agree that the alert--info isn't the best looking, but given that every site can have different styles it would be nice to have a default of alert--primary and the ability to quickly restyle it.

Screen Shot 2022-02-09 at 8 14 31 AM

See the pull request here for some inspiration: https://github.com/cloud-annotations/docusaurus-openapi/pull/161

slorber commented 2 years ago

I understand thanks, that's worth reopening and maybe implement this.

it would be nice to have a default of alert--primary

As I said we are looking to provide a soberer design by default si we'll probably not use that style but rather something that looks closer to how default details view work in html

Josh-Cena commented 2 years ago

So my understanding is that there's little to do with Docusaurus but more to do with Infima, and we will fix it by providing admotion-specific styles?

slorber commented 2 years ago

Yes

Those are not really admonitions, I'd rather just call them "details/summary styles" even if technically it would look quite similar and we can share some CSS

Josh-Cena commented 2 years ago

Ah, typo, s/admotion/details 😄 (It wasn't even correctly spelled)

But admonition styles are another thing I've been considering, cf #5848 about decoupling the admotions Remark plugin from Infima. Totally unrelated though

Josh-Cena commented 2 years ago

Closing in favor of https://github.com/facebookincubator/infima/issues/197. Cross-linked the two discussions so whoever is working on the details styling issue can get more context.