Skip to content

Limit admonition types #200

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

Closed
hoetmaaiers opened this issue Jun 10, 2020 · 6 comments
Closed

Limit admonition types #200

hoetmaaiers opened this issue Jun 10, 2020 · 6 comments

Comments

@hoetmaaiers
Copy link
Collaborator

Coming from #190 , where a part of the discussion (cc @jorisvandenbossche ) is about different admonition types. Or more specific, limit the visual differences to:

  • success,
  • danger
  • warning
  • info

These 4 types are inspired by the Bootstrap variables. Semantically they also make sense to me.
Limiting the options to these 4 also allows a clear visual color difference.

Though when trying to define these 4 classes, I bump into Sphinx / RST limitations?

Error:

/Users/robinhoudmeyers/Workground/OSS/pydata-sphinx-theme/docs/demo/demo.rst:386: WARNING: Unknown directive type "Success".

.. Success:: Great succes.

Is the admonition directive Sphinx specific and limited to:

  • note
  • important
  • tip
  • attention
  • caution
  • warning
  • danger
  • error
  • hint

Or can we replace / extend the list above with our 4 Bootstrap inspired admonitions?

@choldgraf
Copy link
Collaborator

I think I'd prefer re-using the Sphinx directives rather than introducing new directives of our own. In general I think we should avoid people using pydata-sphinx-theme and writing documentation that won't immediately work in other themes as well, unless they really benefit from using some theme-specific functionality.

What's the problem with allowing the longer list of sphinx directives? It seems like it is a superset of the 4 you describe, so can't people just not use the extra ones they don't want to use?

@hoetmaaiers
Copy link
Collaborator Author

I think the discussion arised in #190 with many theme variables being around for admonitions. @jorisvandenbossche suggested to stick to the Bootstrap types.

After a closer look I see no 100% overlap between bootstraps alert type/colors and the admonitions. Mostly the 'success' type is not present in the admonition list, the other ones can be matched between Bootstrap and Sphinx.

@jorisvandenbossche
Copy link
Member

To clarify: my suggestion to limit the number of colors / potentially use bootstrap's existing CSS variables for this, was only related to the "theming" functionality:

  • For writing docs in rst with sphinx, we continue using the standard sphinx admonitions. So indeed nothing to worry about for normal users of the theme
  • For our theming functionality, it is the question whether it is needed that you can override each single admonition type, like what was done in the PR:

https://github.com/pandas-dev/pydata-sphinx-theme/blob/aa925282eb23426aff6014389511bbf3d659dd86/pydata_sphinx_theme/static/css/theme.css#L53-L72

It's for this theming whether I was wondering if it is not sufficient to stick to only a few colors, and potentially reuse the bootstrap color variables for this.

If an advanced user really wants more control, they can still override the CSS directly instead of using CSS variables. Or, we could also still have variables for all of the admonition types, but define them in terms of 4 basic colors, so as a theme user who wants to change the colors, you can also only update those 4, and have it automatically apply to all admonition types.

@choldgraf
Copy link
Collaborator

re: theming, that makes sense to me. I think restricting to a subset of colors (or a subset of themeable colors) will make that easier 👍. I just don't want there to be "rST files written for the pydata theme, and rST files written for other themes" as much as possible

@jorisvandenbossche
Copy link
Member

I just don't want there to be "rST files written for the pydata theme, and rST files written for other themes" as much as possible

Yep, fully agreed

@hoetmaaiers
Copy link
Collaborator Author

If the admonition types are Sphinx standards, then I also fully agree on not adding specific types for this theme.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants