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

genindex.html, Hlists (and possibly more table-based items) become funky after 0.15.3 upgrade #1855

Closed
abjarna opened this issue Jun 4, 2024 · 14 comments · Fixed by #1864
Closed
Labels
kind: bug Something isn't working tag: CSS CSS and SCSS related issues tag: design Items related to design tasks or improvements

Comments

@abjarna
Copy link

abjarna commented Jun 4, 2024

This page uses version 0.15.3:
https://pydata-sphinx-theme.readthedocs.io/en/v0.15.3/genindex.html

This page uses version 0.15.2:
https://pydata-sphinx-theme.readthedocs.io/en/v0.15.2/genindex.html

The style in genindex.html is as expected in version 0.15.2, but in version 0.15.3 the indextable has dark grey color and purple color when hovering.

See attached screenshots:

Screenshot 2024-06-04 at 12 10 06
Screenshot 2024-06-04 at 12 10 22

@trallard
Copy link
Collaborator

trallard commented Jun 4, 2024

Thanks for raising this @abjarna.
This was introduced in #1757, when the table styling was improved. So while not a bug it is an unexpected result from this PR.

Adding a screenshot of the dark version:

Index_—_PyData_Theme_0_15_3_documentation

I do not think the grey background is an issue since this is a table (albeit a special case of a table), but perhaps the hover colour (purple) could be removed as the focus item for interaction will be the links inside a given cell.

Thoughts @gabalafou @smeragoel

@trallard trallard added tag: design Items related to design tasks or improvements needs: more information Needs more information from the author before we can move forward labels Jun 4, 2024
@abjarna
Copy link
Author

abjarna commented Jun 4, 2024

Ok, thank you! I wasn't aware of #1757, but in my opinion this makes the genindex.html look really bad. In addition, the genindex.html now looks totally different from previous versions, which was probably not the intention of #1757.

Also, the grey background either overlaps between left and right column, or creates a thin table border. This results in a thin line in front of the right column in the genindex.html. See attachment.
Screenshot 2024-06-04 at 13 51 45

I haven't yet tried to fix this in my custom.css, but it's probably an easy fix.

However, I recommend a more permanent fix to prevent the above changes to genindex.html.

@abjarna
Copy link
Author

abjarna commented Jun 4, 2024

I also found similar issues when using Hlists.

This page uses version 0.15.3:
https://pydata-sphinx-theme.readthedocs.io/en/v0.15.3/examples/kitchen-sink/lists.html

This page uses version 0.15.2:
https://pydata-sphinx-theme.readthedocs.io/en/v0.15.2/examples/kitchen-sink/lists.html

Screenshot 2024-06-04 at 16 00 25
Screenshot 2024-06-04 at 15 59 44

Are you @trallard aware of any ways to disable these grey colors and purple color when hovering, besides using custom CSS?

@abjarna abjarna changed the title genindex.html after 0.15.3 upgrade becomes funky genindex.html, Hlists (and possibly more table-based items) become funky after 0.15.3 upgrade Jun 4, 2024
@trallard
Copy link
Collaborator

trallard commented Jun 4, 2024

Aha! Another item we did not realise was a table.

Edit the correct PR that introduced this is #1757

Are you @trallard aware of any ways to disable these grey colors and purple color when hovering, besides using custom CSS?

Unfortunately, right now, you'd need to add custom styles specifically to override these lines

tbody {
tr {
&:nth-child(odd) {
background-color: var(--pst-color-table-row-zebra-low-bg);
}
&:nth-child(even) {
background-color: var(--pst-color-table-row-zebra-high-bg);
}
&:hover {
background-color: var(--pst-color-table-row-hover-bg);

tough this would affect all tables on your site.
However, it seems we'd need to make some further improvements to these at the theme level anyway.

@trallard trallard added tag: CSS CSS and SCSS related issues kind: bug Something isn't working and removed needs: more information Needs more information from the author before we can move forward labels Jun 4, 2024
@abjarna
Copy link
Author

abjarna commented Jun 5, 2024

Thank you for the quick response @trallard !

And also thanks for all your hard work on the PyData theme. Best Sphinx theme out there by far! 🙏

@trallard
Copy link
Collaborator

trallard commented Jun 5, 2024

Thank you for reporting these style mishaps 🙏🏽

I am tagging @gabalafou and @smeragoel here as they were the main folks working on tables restyling: folks let's chat about this and scope any further improvements needed

@gabalafou
Copy link
Collaborator

Quick fix: #1864.

Ideally, we would change the output for both hlists and the index page to not use tables. These are using tables only for layout, not semantically to actually communicate tabular data.

@drammock
Copy link
Collaborator

drammock commented Jun 5, 2024

Ideally, we would change the output for both hlists and the index page to not use tables.

Are you suggesting we do that in the theme? Or as an upstream change to sphinx?

@gabalafou
Copy link
Collaborator

Upstream, if we can

@trallard
Copy link
Collaborator

trallard commented Jun 5, 2024

So that would more than likely be at the Sphinx level

@abjarna
Copy link
Author

abjarna commented Jun 7, 2024

Hi again, I see this has been fixed in the latest version (v0.15.4dev0).

Both the latest version (v0.15.4dev0) and version 0.15.3, show table-hover behaviour in "kitchen-sink/tables.html":
https://pydata-sphinx-theme.readthedocs.io/en/v0.15.3/examples/kitchen-sink/tables.html
https://pydata-sphinx-theme.readthedocs.io/en/latest/examples/kitchen-sink/tables.html

And only version 0.15.3 show table-hover behaviour in "genindex.html", but NOT the latest version (v0.15.4dev0):
https://pydata-sphinx-theme.readthedocs.io/en/v0.15.3/genindex.html
https://pydata-sphinx-theme.readthedocs.io/en/latest/genindex.html

And finally, only version 0.15.3 show table-hover behaviour in "kitchen-sink/lists.html", but NOT the latest version (v0.15.4dev0):
https://pydata-sphinx-theme.readthedocs.io/en/v0.15.3/examples/kitchen-sink/lists.html
https://pydata-sphinx-theme.readthedocs.io/en/latest/examples/kitchen-sink/lists.html

I've been trying to replicate the quick fix #1864 from @gabalafou in my local environment, by removing the line "@include table-colors;" from _tables.scss in my virtual environment folder, and then rebuilding with "make clean && make html", but still no luck 😢

Am I missing something obvious, or are there additional steps I need to make to replicate the behaviour in latest version (v0.15.4dev0)?

@abjarna
Copy link
Author

abjarna commented Jun 7, 2024

And a quick note to other users who might also be struggling with the same issue, this addition to custom.css temporarily undoes the table-hover behaviour in "genindex.html":

table.indextable.genindextable {
    border: none !important;
}

table.indextable.genindextable td {
    border-left: none !important;
}

table.indextable.genindextable tr {
    background-color: inherit !important;
}

@drammock
Copy link
Collaborator

drammock commented Jun 7, 2024

I've been trying to replicate the quick fix #1864 from @gabalafou in my local environment, by removing the line "@include table-colors;" from _tables.scss in my virtual environment folder, and then rebuilding with "make clean && make html", but still no luck 😢

the .scss files need to get compiled into .css files for the changes to propogate to the built site. make clean && make html won't do that; it's handled (along with minifying our javascript assets) by webpack.

If you're testing on the theme's own website, you could do tox -e docs-dev for example, which will run webpack first. To do it for your own site, you'd need to figure out what CSS rules the @include actually includes, and then remove them manually from the relevant spot(s) in the compiled .css file

@abjarna
Copy link
Author

abjarna commented Jun 7, 2024

Thank you @drammock! In that case, I’ll stick to the custom.css in my previous comment for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working tag: CSS CSS and SCSS related issues tag: design Items related to design tasks or improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants