Remove 'no-js' non-JS fallback mechanism

[jrochkind]

There was from way back a mechanism used in Blacklight code where body tag has a 'no-js' class, and Javascript once loaded removes that class and adds a 'js' class. This is meant to support having CSS fallbacks to make sure display and interactive elements work properly even if the user-agent has no JS, that then get "enhanced" to JS versions once JS loads. Sometimes called "unobtrusive Javscript" or other names.

However, I could only find one place in code that attempts to use this mechanism, around the sort-and-per-page dropdowns. AND I don't believe it is actually working properly, I don't believe it succesfully provides a working non-JS alternative. See screenshots at #2850. I first noticed it not working some time ago, I beleive it's been non-working for a while.

there are plenty of other UI elements which simply do not work without JS -- facet sidebar for instance. BL in general has not kept up trying to provide non-JS fallbacks, generally BL does not work without JS.

Another downside of this approach is that if you are on a slow computer or network connection, you can get a flash of the "non-JS" alternative before the JS loads to change it to the JS alternative. Which in this case with the non-working non-JS alternative, means you get a flash of something which simply doesn't look right, and also causes the page to shift around a whole lot when the JS finally does load. Which would be considered a trade-off to weigh against the benefit of non-JS support, I guess, if the non-JS support actually worked, but.

So I propose removing this mechanism entirely. It is not working, has been not working for some time, let's remove it to simplify code. If someone wants to add working non-JS fallback again in the future, that can be another PR.

[adjam]

I am -0 on this, as I know of at least one downstream application that depends on this mechanism being available from upstream. This should not be taken to imply that it should not be removed, but its removal should be highlighted in release notes along with some pointers about reinstating the behavior for downstream applications that rely on it.

[jrochkind]

(I think you mean -1, not sure what -0 means, heh!) Can you mention the downstream thing you know, for full context?

If we leave it, I guess we should create an issue to make it work for the sort-and-pagination area, where it currently does not?

Hm, another option would be leaving the mechanism there, but simply removing the non-working no-js CSS for sort-and-pagination. So the mechanism would still be there for anything downstream that uses it, but nothing in blacklight core would actually use it? (Right now, one thing tries to use it, but not succesfully). Opinions on that @adjam or others?

[adjam]

"-0" means "I don't like this change/this change has negative effects, but I'm not voting against it." The context is that the project has CSS rules that match on .no-json the body tag. I can see reasons not to want to maintain the functionality in blacklight if it's not being directly used there, but if so, it's better to call out the removal of this feature (or any other) in the release notes in case downstream projects are relying on it.

[jrochkind]

Would you be interested in helping to write those release notes, @adjam , with your understanding of the use case for them?

I am not sure where we put release notes to "hold" them until the release, I don't know how this works in BL release practices.

[cbeer]

Adam might be -0, but I'm -1. Completely removing the ability for users without javascript enabled to use controls seems like a step backwards.

When I disable javascript, I see these controls:

and they seem to work for me.

The flash of unstyled content seems like a regression (only for certain asset pipeines?) and we could fix it easily by

<script>
  document.querySelector('html').classList.remove('no-js');
</script>

(e.g. #2901, which seems to work for me using importmaps 🤷‍♂️ )

[jrochkind]

a) @cbeer there are other controls in BL that users without JS cannot use, such as the facet sidebars, agree? the -only- controls that have an attempt at non-JS fallback are sort-and-per-page. That made me think this is not something that BL community actually cared about, it was just leftover from another era and not maintained. But are you saying instead that sort-and-per-page controls are especially important to have non-JS fallbacks?

b) Interesting, your screenshot does not match mine! Here is what I am seeing on current demo.projectblacklight.org, on MacOS Chrome, with Javascript disabled. Note how for me the menu is overlapping the first search result; I would consider this broken; that was another thing that made me suspect this was not something the BL community currently cared about or was maintaining or actually checking that it worked for non-JS users. I am surprised to find interest in supporting non-JS users, when as far as I can tell the code has not been doing so effectively for years.

But your screenshot does not have that overlap. Also mine ends up really squishing the "previous/next" area undesirably, and yours doesn't. (I think the giant square non-interactive blocks are undesirable in both of ours?) Very odd. Are you using a different non-Chrome browser? I just reproduced same as above on Firefox Mac too. I am baffled how you are seeing something different! What browser did you make that screenshot from, and was it on demo.projectblacklight.org?

The fact that to me the non-JS fallback looked bad and wrong suggested, and has for years, suggested to me that non-JS fallback wasn't actually a priority for anyone here.

c) I don't believe the flash of different content (with page layout moving around) is due to using an unusual method of including javascript. You reproduce it yourself on demo.projectblacklight.org, if in Chrome Dev Tools "Network" tab you choose to "throttle" to "Slow 3G".

[tpendragon]

@jrochkind The committers met today and discussed both this and #2902. We talked about the burden of supporting no-JS when we're using Bootstrap and just generally, and the recognition that this has been broken long enough that it feels like progressive enhancement is not a core value of this software that we want to support. In that discussion we made a few decisions:

1. Historically progressive enhancement has been a valued philosophy of Blacklight. This PR is evidence that we're not fulfilling that value, and importantly is evidence that our values aren't well documented and understood. On the committer call agenda now is to have discussions about defining and documenting the values we're building this software with, and a second item about how we might regularly audit our adherence to those values, especially with regards to accessibility (recognizing that the accessibility implications of no-js aren't the same as they were 5-6 years ago.)

2. Fixing the bug that's been identified here is done adequately in Make some CSS tweaks for non-js users #2902 and Use an inline script to remove the .no-js class to avoid an unstyled … #2901 with a small amount of code change. Your report here was invaluable - thank you! We're looking forward to the "blink" being gone, and being able to continue to adhere to progressive enhancement ideals. We're going to close this PR, and merge 2901/2902. I'm sorry these lines of code won't be making it into the code, I know that can be disheartening.

I want to personally recognize your commitment towards maintainability of software we all depend on, and I'm so thankful for the issues this has brought to the forefront.

[jrochkind]

Thanks for the update and clear info on decision!