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.

Sign up for GitHub

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

Jump to bottom

Theme switcher should not added via a script tag, in general do we support browsers w/o js ? #1920

Open
Carreau opened this issue Jul 8, 2024 · 2 comments
Open

Theme switcher should not added via a script tag, in general do we support browsers w/o js ? #1920

Carreau opened this issue Jul 8, 2024 · 2 comments

Comments

@Carreau
Copy link
Collaborator

Carreau commented Jul 8, 2024

the theme switcher does:

<script>
document.write(`
  <button ...>
...
  </button>
`);
</script>

I believe this is bad practice as it will block rendering while js is executed to insert this. Which makes the theme slower. We could use I belive:

<head>
    <noscript><style> .jsonly { display: none } </style></noscript>
</head>

And just have whatever is js only be not displayed when there is no JS.

But in general we do rely on a bunch of other things that need JS (More dropdown in navbar requires js).

So maybe we should assume we always have JS ?
@drammock
Copy link
Collaborator

drammock commented Jul 8, 2024

The logic here was "the switcher won't work at all without JS, so don't even render it if JS is unavailable". I'm open to trying out your proposed approach.

I'll add that the current approach may be bad practice for another reason: document.write is strongly discouraged in general. I just haven't had time to think hard about the alternative approaches.

Carreau added a commit to Carreau/pydata-sphinx-theme that referenced this issue Jul 9, 2024
@Carreau
Maint: do not use document.write
c3d7006 
See pydata#1920, this removes every usage of document.write in favor of
display:none with a noscript tag.

I did have to be  a little more specific for buttons as the css rule in
boostrap were overwriting the *.jsonly.

Note that this does not solves pydata#1920 as some things (like the more
dropdown in nav bar) still require JS to work.
@Carreau Carreau mentioned this issue Jul 9, 2024
Merged
Carreau added a commit to Carreau/pydata-sphinx-theme that referenced this issue Jul 18, 2024
@Carreau
Maint: do not use document.write
b595b4c 
See pydata#1920, this removes every usage of document.write in favor of
display:none with a noscript tag.

I did have to be  a little more specific for buttons as the css rule in
boostrap were overwriting the *.jsonly.

Note that this does not solves pydata#1920 as some things (like the more
dropdown in nav bar) still require JS to work.
@gabalafou
Copy link
Collaborator

gabalafou commented Jul 18, 2024 •
edited
Loading

So... my personal dream would be that one could load a PST-themed docs site in Lynx and reach all the same info as someone loading the site in Chrome. In other words, everything beyond basic HTML is considered a progressive enhancement.

In that same spirit, I would love to have fallbacks for all JS-powered components, such as the theme switcher. But perhaps that is not realistic. Or perhaps it doesn't stand up to a cost-benefit analysis.

At any rate, I think that our current implementation could use improvement. Since we have core components that absolutely require JavaScript, we should more prominently warn users who have disabled JavaScript that the site won't completely work and disclose exactly what functionality requires JavaScript.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gabalafou @Carreau @drammock