Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow specific deactivation of table handling #1218

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

danwos
Copy link

@danwos danwos commented 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 #1179

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
Copy link
Author

danwos commented Aug 24, 2021

@nienn : I hope this PR is fine and solves #117 as expected.

I have also added some documentation and a changelog entry.
As this is my first PR on this project, please let me know if anything is missing.

Local tests together with Sphinx-Needs are looking great.

@agjohnson
Copy link
Collaborator

Thanks for the contribution, we'll see about getting this in version 1.1. I'll block this PR for now, we're still wrapping up a 1.0 release.

@agjohnson agjohnson added the Status: blocked Issue is blocked on another issue label Aug 24, 2021
@danwos
Copy link
Author

danwos commented Aug 25, 2021

@agjohnson Thanks for your feedback and for the overall work on RTD.

The change itself is really small, one line.
And it does not have any side-effects to the theme or any code.
So really no chance to get this small bugfix into 1.0?

Not having a possibility of deactivating the automatic table handling makes RTD-Theme incompatible with some other sphinx-extensions. I, as an affected extension developer, would be happy to avoid this incompatibility as fast as possible.

The other solution would be to ask my users to use another theme or use the fork, which contains my changes.
Something, I really don't like.

@agjohnson
Copy link
Collaborator

So really no chance to get this small bugfix into 1.0?

We'll look to getting this in for 1.1, we don't want to keep holding back a 1.0 release.

@twodrops
Copy link

twodrops commented Oct 6, 2021

This issue is extremely critical for us. The fix solves a major problem that is affecting 100s of tables that we have in RTD theme. Any chance to get this in soon?

@nienn
Copy link
Contributor

nienn commented Oct 12, 2021

@agjohnson I was looking for a way to accomplish the same thing from the sphinx side but could not find it.

Found the following documentation that gives options for setting tables and table cells widths.
In the end however, everything gets wrapped in the div.wy-table-responsive and the custom widths get ignored in favor of the css white-space: nowrap:

Also, the js handling the responsive tables is on the theme side and it applies to every table that is not .field-list, .footnote or .citation:

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

Although this is not an ideal solution, I think it's a good option to add.

Maybe instead of .rtd-exclude-wy-table we could use a more semantic class like .unresponsive.


This would also fix: #1246

@danwos
Copy link
Author

danwos commented Oct 25, 2022

Any updates on this?
1.1 is on the way and this PR is set to blocked
Anything I can do here?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Status: blocked Issue is blocked on another issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Table handling for sphinx-extensions (incl. proposal)
4 participants