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

Table handling for sphinx-extensions (incl. proposal) #1179

Open
danwos opened this issue Jul 21, 2021 · 4 comments · May be fixed by #1218
Open

Table handling for sphinx-extensions (incl. proposal) #1179

danwos opened this issue Jul 21, 2021 · 4 comments · May be fixed by #1218
Labels
Needed: replication Bug replication is required

Comments

@danwos
Copy link

danwos commented Jul 21, 2021

Problem

The rtd theme wrapps all tables in a div with class wy-table-responsive via javascript.
Unfortunately some html-tables in sphinx-projects are not coming from "normal" rst-tables, but are created by sphinx-extensions, which provide their own table setup/manipulation.

An example is the Sphinx-Needs extension, and there especially the directives need and needtable.
See also bug report #767 from the past.

Here a table comparison for needtable in the themes alabaster and rtd:
image

The first table is the important one, which gets handled by the JS-lib DataTables.
With rtd, head and body are separated and the columns are not aligned.

Reproducible Project

You can take a look into the documentation on rtd. I have activated a branch which is using the rtd theme:

needtable docs with alabaster
needtable docs with rtd

Sphinx-Needs doc sources

Problem source

As I'm the maintainer of Sphinx-Needs, I tried to provide my own css-configuration, which contains rules for RTD only.
This works in some cases, but not in all.

Problem is also that rtd changes the DOM by inserting a div-wrapper with class wy-table-responsive.
Which may confuse other JS-code.

Proposal / Fix

For sure rtd-theme can not handle all the exceptions needed by other extensions in its code.
But maybe there is a way to exclude certain table-configurations from the default rtd table-handling.

theme.js is already doing this for classes like citation or footnote.
See:

$("table.docutils:not(.field-list,.footnote,.citation)")

Maybe we can extend this line by a specific class-name .rtd_exclude, which can be set by sphinx-extensions to avoid the rtd-specific handling for these tables:

$("table.docutils:not(.field-list,.footnote,.citation,.rtd_exclude)")
            .wrap("<div class='wy-table-responsive'></div>");

There may be some more positions in the code, where such an exception-handling is needed (haven't checked everything yet).

Would this be a proper solution?
I would be happy to create a PR, but first please give me your feedback :)

@danwos danwos added Bug A bug Needed: replication Bug replication is required labels Jul 21, 2021
@danwos danwos changed the title Table handling for sphinx-extensions Table handling for sphinx-extensions (incl. proposal) Jul 21, 2021
@agjohnson agjohnson removed the Bug A bug label Jul 22, 2021
@danwos
Copy link
Author

danwos commented Aug 17, 2021

@agjohnson or @Blendify or @nienn:
Any thoughts on this concept?

Would we lucky to support here with an PR.

@nienn
Copy link
Contributor

nienn commented Aug 17, 2021

I'm 👍 on adding some exclude class. It can be an useful feature for other extensions or custom settings to use and I don't see any drawbacks.

I would select a more specific class to this specific purpose thought, as the same class should not be used to any other unrelated styling. Maybe .rtd-exclude-wy-table?

danwos added a commit to useblocks/sphinx_rtd_theme that referenced this issue Aug 24, 2021
Setting the table class "rtd-exclude-wy-table"
deactivates the normal table handling for this
table.

So the table gets not wrapped into a
"div.wy-table-responsive" container.

Needed by other Sphinx extensions, which
care about their tables on their own.

Fixes readthedocs#1179
@danwos danwos linked a pull request Aug 24, 2021 that will close this issue
@twodrops
Copy link

This issue is really getting critical for us. The fix is available from @danwos. Any chance of merging this soon? We are working with a fork of RTD now because of this.

@jimhavrilla
Copy link

I will say that wrapping my HTML imported tables with a div with class wy-table-responsive allowed me to properly horizontally scroll long tables in Sphinx-generated HTMLs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needed: replication Bug replication is required
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants