Maint: do not use document.write

[Carreau]

See #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 #1920 as some things (like the more dropdown in nav bar) still require JS to work.

[Carreau]

I'm not sure how to interpret the failures. Guidance appreciated

[drammock]

looks to me like:

Unexpected warning: WARNING: dot command 'dot' cannot be run (needed for graphviz output), check the graphviz_dot setting

is probably the culprit behind the a11y test failure.

[Carreau]

Oh, it's also failing on main also. I thought it was failing because of my changes.

[github-actions]

Coverage report

This PR does not seem to contain any modification to coverable code.

[gabalafou]

This is great! Just have a few comments

[gabalafou]

Is the leading asterisk there for precedence? Maybe we should do:

.jsonly {
  display: none !important;
}

Also, should we prefix it with pst- and put a dash between js and only?

.pst-js-only

I'm thinking by analogy to the utility class sr-only.

[Carreau]

+1 to pst-, but I woulld prefer not to use !important, I have been told it is bad practice.

[gabalafou]

So that goes back to my original question: is the leading asterisk for precedence?

My concern is that it might be too easy right now for some other CSS with display: flex or some other display rule to override the pst-js-only class.

I'll need to take a closer look at selector precedence and come back to this.

[Carreau]

So yes, it's for precedence.

But I think we use pst-js-only in so few places – and I believe it should be used sparingly – that it should be ok to review each of them independently.

Alternative is to not have a class but a lit of ids of elements to hide when no js.

[gabalafou]

Okay, so I took a little look into this.

My concern here is that in most of our day-to-day development, we're not going to see this class in action. So it could be fairly easy to break it without realizing it.

For example let's take the search-button component, which uses the pst-navbar-icon class. Someone might be working to fix a style issue and they realize that a Bootstrap rule is being applied that they need to override, so they increase the specificity of the set of pst-navbar-icon rules by giving the search-button an id, and targeting via that id, which then overrides your display: none rule. The developer never realizes it, though. The example is a little bit contrived, but not that much.

As for the alternative idea of targeting ids, I don't like it because I think the pst-js-only class really helps with discoverability and readability.

I really think that in this case, by the same logic you invoked earlier about pst-js-only class used in so few places and for such a specific purpose, that it's okay to use !important. The only other preventative measure I can think of would be to write tests that load the page without JavaScript.

[gabalafou]

One thing to note about !important is that it reverses the order of specificity: important user-agent rules come before important user rules come before important site author rules, which means that if a user or browser wants to override our pst-js-only rule, they can.

[Carreau]

removed the *, and added !important. the .btn was still needed though.

[gabalafou]

Typo in the CSS rule. That's why you needed the .btn prefix.

[Carreau]

Thanks !