move parent theme css earlier in the inclusion order

[akhmerov]

Closes #447.

I achieve the desired result by making the theme css file empty (this overrides the basic.css provided by the parent theme), and @importing the basic.css in our own theme.css.

This change should enable removing most of the !important properties (not done yet) and simplify implementing derived themes.

[akhmerov]

I now also went ahead and removed the !important parameters. The style changes are minimal, and can be easily undone if desired.

[akhmerov]

I added a more specific selector div.admonition, because this is how admonition styling is specified in basic.css. I leave the more catch-all selector for compatibility and in case someone (me) wants to reuse the styling for colapsible admonitions that use a <details> tag.

[akhmerov]

This bit now overrides the parent theme. Previously this margin was ignored. I am happy to undo this style change if necessary.

[choldgraf]

This seems like a nice change! I think I am a bit confused about what exactly the "blank" stuff is doing, perhaps you could explain here or via a commit to the code?

If I understand, because we weren't specifying a stylesheet we were just inheriting the one that the Sphinx basic template uses. So this explicitly specifies the stylesheet so we don't inherit from Sphinx, but sets it to blank because we really just want our own HTML theming to include the stylesheet manually. Is that right?

[choldgraf]

can you provide more detail here about why this is blank? Imagine a new developer in the future that hasn't seen any of these issues coming across the CSS file, can we provide enough context that they understand why this is here?

[akhmerov]

I explained it now.

[akhmerov]

If I understand, because we weren't specifying a stylesheet we were just inheriting the one that the Sphinx basic template uses. So this explicitly specifies the stylesheet so we don't inherit from Sphinx, but sets it to blank because we really just want our own HTML theming to include the stylesheet manually. Is that right?

Close, but not exactly. One way or another we use the parent stylesheet. Whether we want to remove it entirely is out of scope for this MR. The problem I'm addressing is that the parent stylesheet is included in the template after ours, leading to problems in overriding the defaults (some were actually overlooked I think). Plus the filename of ours is computed dynamically to avoid cache problems. To avoid this I do two things:

1. Provide a blank stylesheet to override the inclusion of the parent one in the place where sphinx inserts it.
2. Manually include the parent stylesheet before ours (that's the @import statement).

[choldgraf]

I think this LGTM - agreed that it's better to have the upstream CSS loaded first so that we can override it if need be. I'll leave open for a little bit in case others have thoughts.

[akhmerov]

Thanks. Should I do anything about the lighthouse failing? (I've no idea how my changes are related.)

[akhmerov]

@choldgraf didn't notice that the comment syntax was wrong (// doesn't work in css), which in turn broke the render in the latest edit.

[choldgraf]

Ah whoops, my bad

[drammock]

Should I do anything about the lighthouse failing? (I've no idea how my changes are related.)

lighthouse is broken on all PRs right now, not related to your changes

[choldgraf]

ok gonna merge this in since the lighthouse thing is for all projects not just this one. many thanks @akhmerov !

[jorisvandenbossche]

Thanks for the PR @akhmerov !

Browsing through the demo docs, I noticed some changes that might be related to the CSS changes here (or the order switch):

• The xarray output is no longer showing up (https://pydata-sphinx-theme.readthedocs.io/en/latest/demo/demo.html#html vs https://pydata-sphinx-theme.readthedocs.io/en/stable/demo/demo.html#html)
• The glossary no longer has indentation for the explanation paragraph (https://pydata-sphinx-theme.readthedocs.io/en/latest/demo/demo.html#glossary vs https://pydata-sphinx-theme.readthedocs.io/en/stable/demo/demo.html#glossary)
• Some changes in the autodoc API docs (https://pydata-sphinx-theme.readthedocs.io/en/latest/demo/api.html#numpy.linalg.eig vs https://pydata-sphinx-theme.readthedocs.io/en/stable/demo/api.html#numpy.linalg.eig: indentation seems to be removed, and some color changes). See also https://pydata-sphinx-theme.readthedocs.io/en/latest/demo/generated/pandas.DataFrame.drop.html vs https://pydata-sphinx-theme.readthedocs.io/en/stable/demo/generated/pandas.DataFrame.drop.html, indentation in the parameter section is also changed, which might be related to the glossary mentioned above. Also font of the signature changed.

[jorisvandenbossche]

So bootstrap has the following CSS for definition lists:

dt {
 font-weight:700
}
dd {
 margin-bottom:.5rem;
 margin-left:0
}

and that is now overwriting what sphinx basic theme does (instead of the other way around):

dd {
    margin-top: 3px;
    margin-bottom: 10px;
    margin-left: 30px;
}

[akhmerov]

The xarray output is no longer showing up (https://pydata-sphinx-theme.readthedocs.io/en/latest/demo/demo.html#html vs https://pydata-sphinx-theme.readthedocs.io/en/stable/demo/demo.html#html)

Likely a separate issue. Inspecting the outputs shows that the matching div has a hidden attribute: <div class="xr-wrap" hidden="">. Removing the attribute fixes the problem. Not sure what is the root cause.

[jorisvandenbossche]

For xarray, I think it is caused by removing the !important from .xr-wrap[hidden]. Adding that back also fixes it. Not fully understanding why though (without the !important, it gets overridden by the [hidden] rule from bootstrap, but I thought that our css came after bootstrap's anyway)

[jorisvandenbossche]

Not fully understanding why though (without the !important, it gets overridden by the [hidden] rule from bootstrap, but I thought that our css came after bootstrap's anyway)

Ah, but it seems that bootstrap also has an !important there, so that's probably the reason it was needed to overrule it