Skip to content
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

ENH: Allow admonitions to be style as a sidebar #836

Merged
merged 5 commits into from Jul 24, 2022

Conversation

drammock
Copy link
Collaborator

This makes it easy to style docutils .. sidebar:: to match our admonition style, by adding classes like this:

.. sidebar:: Some title
   :class: admonition warning

   Some content

@drammock
Copy link
Collaborator Author

Copy link
Collaborator

@choldgraf choldgraf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty good to me! Two quick questions:

  1. Given that the sidebar is a bit more limited in space, I wonder if it'd be better to not apply the left margin to the content. E.g. here's what it looks like with that removed:

    image

  2. Do you think this should just be the default styling for the sidebar? Maybe it would be a grey color if no :admonition: is given, and it could be colored by adding note etc?

  3. Alternatively, we could keep the sidebar as-is, and add a helper class for admonitions that would make it behave like a sidebar. E.g. we could ask people to do:

    ```{note}
    :class: sidebar
    My note
    ```

@drammock
Copy link
Collaborator Author

not apply the left margin to the content

Good idea

3. keep the sidebar as-is, and add a helper class for admonitions that would make it behave like a sidebar.

No strong feeling here. Both seem equally usable. Only difference I can think of is the actual HTML (whether it's an aside or not), which makes me lean slightly toward the current implementation.

@drammock
Copy link
Collaborator Author

Do you think this should just be the default styling for the sidebar?

I gave this some thought, and I think no. We don't know what users will do with the sidebar - maybe it's just an image, we should leave this container flexible by default. They should be able to get the grey one via :class: admonition (without also specifying a semantic color)

@choldgraf
Copy link
Collaborator

I think the biggest benefit of going the opposite direction and supporting:

```{note}
:class: sidebar
foo
```

would be on the implementation side. E.g. if we just add these 4 CSS rules to our admonition (e.g. triggered via .admonition.sidebar, then we get pretty much the same effect:

{
    max-width: 40%;
    float: right;
    clear: both;
    margin-left: 0.5rem;
}

results in:

image

@drammock
Copy link
Collaborator Author

Ok I'm convinced. Feel free to push that change, I'm out of office until Tuesday.

@drammock
Copy link
Collaborator Author

Although I think you also need a rule for the inner content left margin. But still, that's a much nicer implementation

@choldgraf
Copy link
Collaborator

Sounds good, I will try and give it a shot if I have a moment. I am hoping to do a little flurry of PR merges so that we can make a release relatively soon.

@choldgraf choldgraf changed the title allow styling sidebars like admonitions Allow admonitions to be style as a sidebar Jul 24, 2022
@choldgraf
Copy link
Collaborator

choldgraf commented Jul 24, 2022

OK, the latest pushes re-work this to trigger the behavior with a sidebar class on an admonition, rather than an admonition class on a sidebar. I also updated the docs accordingly!

I will merge this once the RTD docs pass and I re-confirm that it looks as-expected, since @drammock said he's out for several days :-)

@choldgraf
Copy link
Collaborator

Ah one extra change here - I realized that we were starting to put "instructional" information in the theme-elements.md file. In addition, a few of the other "theme-specific styles" pages in the demo/ folder were also acting more like instructions for how to set up various other extensions with the theme. So I decided to move these into the user_guide in a `"content and features" section, since I think that's a natural place for people to look to find out how to do specific things with this theme

@choldgraf choldgraf changed the title Allow admonitions to be style as a sidebar ENH: Allow admonitions to be style as a sidebar Jul 24, 2022
@choldgraf choldgraf merged commit e233d04 into pydata:main Jul 24, 2022
@jarrodmillman jarrodmillman added this to the 0.10 milestone Jul 26, 2022
@drammock drammock deleted the style-docutils-sidebars branch July 26, 2022 20:29
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

Successfully merging this pull request may close these issues.

None yet

3 participants