Avoid stripping table classes

[danwos]

Why do the table-classes get all stripped?

I know I can have a whitelist for classes, which should stay (table_classes), but the overall behavior makes no sense for me, as it
destroys a lot of information which is important for the tables and their styles.

Up to now I personal don't see any technical reason for this. Please tell me if I'm wrong.

Proposal

Leave all tables classes as they are set by sphinx/docutils/extensions

Use case

I'm the maintainer of a Sphinx-extension called Sphinx-Needs.

This extension uses multiple tables to present data.
Some tables are used to define requirements or specification. Some are used as a "normal" table to give an overview about e.g. 30 requirements. And some are even used and get transformed by a JS-lib called DataTables.

All these tables can be styled. E.g. an open requirement table is green, a closed one red.
And all these styles and different kind of tables are handled by various css classes and the user can even define own ones.
So there are dozens of css-classes defined for tables.

Sphinx-Material completely destroys this mechanism by ignoring such information, which is coming from the docutils-nodetree.

Last words

I'm totally open to help here and provide a PR, mainly because I love this theme a lot ❤️
But it would be helpful for me to understand the reasons for this behavior and maybe discuss already technical solutions.

[bashtage]

If you leave them you don't get mkdocs styling. Mkdocs 3, from which the css was taken, is strict about which tables it will apply styling to. I don't think this can be changes without making a heroic effort to move forward for a full update.

[danwos]

Thanks for the fast answer and your help.

Based on your hint, I took a look into the css-code and indeed there are 12 css definitions in /sphinx_material/static/stylesheets/application.css , which uses selectors like table:not([class]).
So this selector is valid, as long as no class is set. (I don't get it, why they solved it this way, but okay...).

I don't see any reason (yet), why this css-defintions should not be set if a class is available for a table.

So maybe there is the chance, that everything is still working, if we copy these 12 occurrences to application-fixes.css and remove this not([class]) stuff.

Would this be a valid solution? For sure, only if everything looks the same afterwards.
If yes, I would be happy to create the PR for this.

[danwos]

I was wrong. The problem is not part of any css code.
It is this line https://github.com/bashtage/sphinx-material/blob/main/sphinx_material/__init__.py#L242

I understand the point, that we only get material-styling (css), if the tables do not have any css class.
The problem is, that a theme can not make a general assumption here and simply remove every class from every table.

I would like to use the implemented logic the other way around.
There is a predefined list of table css classes. If only these classes can be found, then table['class'] gets deleted.
If more as the predefined classes are found, table['class'] stays untouched or maybe just the predefined classes get removed (have to figure out).

And as we know sphinx and doctuils, it shouldn't be so hard to get the needed classe names for this list.

This would allow any extension to set table classes and provide css code on its own.

Any feedback or hints, why this would not work?

[bashtage]

I suppose I don't really want to break this theme for anyone who uses it by inverting the selection. I don't disagree that this is a hacky fix.

In a perfect world there would be a truly sphinx-specific rewrite of the CSS (or SCSS and then derive CSS from this). That requires an effort beyond what I can do for this project. There is another thread where there is vocal support for updating, although no one was come forward with enough bandwidth to formally provide a well-supported alternative.

[danwos]

The problem is that the theme can not be used with any extension, which uses tables to structure its own data.
So this them is not an option, if it shall be used with an extension like sphinx-needs.

But I totally agree that a fix must be backwards compatible.
There was a similar issue with the ReadTheDocs theme, as some Javascript manipulates table classes as well.
See readthedocs/sphinx_rtd_theme#1179.

There we found the solution to check if a specific css-class is set (.rtd-exclude-wy-table).
If this is the case, this specific table stays untouched.

I think this can be a solution here as well and I as an extension developer must care about setting this class, to avoid theme specific table handling.

So here my new proposal :)

If table["css"] contains no_material_handling, then all tables css classes are kept.
Feel free to change this name...

So everything stays as it is, as long as nobody set this specific table-class.

Test

I made a quick test and deactivated the handling for sphinx-needs tables:
Before:

After deactivating handling for specific tables:

Would this be a valid way?
And for sure I would be happy to create the PR, if I get your GO.

[bashtage]

A sentinel value does sound like a very good and very-likely to be backward compatible. Now to gnash teeth on what the value should be :-). no-material-strip or more specific no-sphinx-material-strip (or handle). It should also be settable to a list of classes, even if there is a default value (also so that the default could be set to nothing, i.e., a empty list.

[danwos]

Ok, so here the (final?) proposal:

html_theme_options = {
   table_classes = []  # already exists and stays untouched by this change

   table_no_strip = []  # new
}

If table_no_strip is not defined, it will be set to ['no-sphinx-material-strip'].

If it contains a list, e.g. ['no_handling'], then also no-sphinx-material-strip will be added. Final result table_no_strip = ['no_handling', 'no-sphinx-material-strip'].

If table_no_strip is set to [] or None, the list stays empty and even no-sphinx-material-strip will not bet set.

Hopefully I got all your points.
I will try to provide a PR till end of this week.

[bashtage]

Sounds like a good fix.