jbms / sphinx-immaterial

Adaptation of the popular mkdocs-material material design theme to the sphinx documentation system
https://jbms.github.io/sphinx-immaterial/
Other
177 stars 28 forks source link

Rendering of :kbd: role is a bit off #322

Closed ruslo closed 4 months ago

ruslo commented 4 months ago

Here is how :kbd: rendered in a light theme:

Screenshot from 2024-02-07 16-51-13

It seems that the top of the object is missing.

It's slightly better with a dark theme:

Screenshot from 2024-02-07 16-51-23

Same code for the read-the-docs theme:

Screenshot from 2024-02-07 16-58-56

Live samples:

2bndy5 commented 4 months ago

Can you supply the RST code to reproduce?

It looks like the undesired offset happens when there's nested <kbd> elements.

<kbd>
  <kbd>Ctrl</kbd>
  +
  <kbd>F</kbd>
</kbd>

Have you also looked into the keys role? The keys role's CSS is exclusive to this theme because the CSS is inherited from mkdocs-material theme...

ruslo commented 4 months ago

Can you supply the RST code to reproduce?

See: highlight_text.rst

Have you also looked into the keys role?

Okay, it helped with the nested case :+1: But the top of the object is still not distinct:

Screenshot from 2024-02-08 17-34-03

Compare with:

Screenshot from 2024-02-07 16-58-56

2bndy5 commented 4 months ago

CSS for the keys role is inherited from mkdocs-material source: src/assets/stylesheets/main/extensions/pymdownx/_keys.scss.

We also don't use any CSS from sphinx basic theme: sphinx/themes/basic/static/basic.css_t. But I don't see any CSS rules specific to kbd elements there.

Lastly, the RTD sphinx theme adds its own CSS specific to kbd elements: See their src/sass/_theme_rst.sass file.

So, to solve this we would need to add some CSS for kbd elements that are generated by the :kbd: role and be careful to not alter the rules defined for the :keys: role.

Just my opinion

I have some reservations about making any CSS changes because this theme is meant to be a port of the mkdocs-material theme to sphinx, not a material-design adaptation of the RTD sphinx theme... Looking at how the kbd elements are styled in upstream mkdocs-material, I feel like the current style is expected behavior.

There's also nothing stopping users from adding their own CSS to a project via sphinx, but having the different :keys: and :kbd: roles may complicate that idea too much for some doc authors.

2bndy5 commented 4 months ago

There also seems to be slight differences in how different browsers render the kbd elements. In Firefox, the offset looks fine, but Chrome seems to position the kbd elements' contents a bit higher than Firefox.

ruslo commented 4 months ago

I have some reservations about making any CSS changes because this theme is meant to be a port of the mkdocs-material theme to sphinx, not a material-design adaptation of the RTD sphinx theme... Looking at how the kbd elements are styled in upstream mkdocs-material, I feel like the current style is expected behavior

Okay, I see, indeed the issue is the same there.

In Firefox, the offset looks fine, but Chrome seems to position the kbd elements' contents a bit higher than Firefox

For me, Firefox looks similar:

Screenshot from 2024-02-08 18-40-26

2bndy5 commented 4 months ago

I think the upstream artistic style can still be preserved while adding the requested visual distinction by simply darkening the kbd elements' background-color for light mode only:

[data-md-color-scheme="default"] kbd {
  background-color: #dfdfdf;
}

which results in image

The background-color for kbd elements and the main body element is practically the same color in light mode.


EDIT: I also noticed that the pymdown.keys docs also override the background-color to use #ebebeb instead of the mkdocs-material's default #fafafa.

pymdown.keys mkdocs-material
image image
ruslo commented 4 months ago

I think the upstream artistic style can still be preserved while adding the requested visual distinction by simply darkening the kbd elements' background-color for light mode only

Look good:

Code: https://github.com/ruslo/read_the_docs_template/commit/6951d7abccb1e2f9de88b584fd5ff53c568b797e

Thank you!