spatialaudio / nbsphinx

:ledger: Sphinx source parser for Jupyter notebooks
https://nbsphinx.readthedocs.io/
MIT License
449 stars 128 forks source link

theme-specific border for input blocks #733

Open 2bndy5 opened 1 year ago

2bndy5 commented 1 year ago

https://github.com/spatialaudio/nbsphinx/blob/a619a5091c29815c1605166d6adca26700fae6e9/src/nbsphinx/_static/nbsphinx-code-cells.css_t#L128-L133

This adds a border around the div containing the input code-block. However, it does not work as desired with all sphinx themes (or possibly with user-defined extra CSS) as the CSS for typical code-blocks could have additional padding/margins around the actual div for the code-block.

Observe the weird border (that doesn't match the theme's color palette) in the sphinx-immaterial theme: image

It looks fine with the insipid theme, but I think that could be added to the theme CSS instead of nbsphinx CSS.

See also screenshots in #734 for an example of how this border looks odd with the furo theme.

mgeier commented 1 year ago

the CSS for typical code-blocks could have additional padding/margins around the actual div for the code-block

I guess they could, but it seems no other theme actually does this. At least in the ~30 themes in the nbsphinx docs I couldn't find any.

It looks fine with the insipid theme, but I think that could be added to the theme CSS instead of nbsphinx CSS.

The insipid theme doesn't have borders around code blocks, see https://insipid-sphinx-theme.readthedocs.io/en/0.4.1/showcase/code-blocks.html

The borders are part of the Jupyter look-and-feel.

See also screenshots in #734 for an example of how this border looks odd with the furo theme.

Are there also problems with margin/padding? Can you please point them out to me because I don't see them?

2bndy5 commented 1 year ago

I didn't see any problems with padding/margins in other themes (besides sphinx-immaterial). I suppose we could add a CSS rule in sphinx-immaterial. I wasn't aware this border matches a standard Jupyter notebook style.

I'll close this, leaving the color choice in #734.

2bndy5 commented 1 year ago

I just found out the extra padding is added by nbsphinx CSS: https://github.com/spatialaudio/nbsphinx/blob/a619a5091c29815c1605166d6adca26700fae6e9/src/nbsphinx/_static/nbsphinx-code-cells.css_t#L191-L199

I think this would require an exception added to avoid the extra padding for input blocks in sphinx-immaterial:

div.nbinput.container div.input_area div[class*=highlight] > pre:not(:has(code))

But maybe that needs to go in the theme CSS as it isn't specific enough for sphinx-immaterial. Note, among the various and numerous money-patches to sphinx, the sphinx-immaterial theme encapsulates the div.highlight pre contents in a <code> element to mimic how mkdocs generates code-blocks. This means that it could be specific enough to not apply to other sphinx themes as long as they don't also monkey-patch sphinx' literal blocks similarly.

We could also try a new rule that uses sphinx-immaterial specific CSS class (md-typeset) used in all generated DOM:

/* exclude input block padding in sphinx-immaterial */
.md-typeset div.nbinput.container div.input_area div[class*=highlight] > pre {
  padding: 0;
}

but this seems to negatively impact the sphinx-material theme as well. Although, the current rule in nbsphinx overrides the padding in sphinx-material CSS...

The first idea (with :not(:has(code))) still works better for both mkdocs-material derivative sphinx themes. So, it might be best to combine the 2 ideas:

/* exclude input block padding in sphinx-immaterial */
.md-typeset div.nbinput.container div.input_area div[class*=highlight] > pre:has(code) {
  padding: 0;
}
2bndy5 commented 1 year ago

Looks like Firefox only supports the :has() CSS selector by setting the layout.css.has-selector.enabled flag to true. Although, the :not(:has(code)) seems to work in Firefox without altering the flag (which defaults to false).