Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

Disable JavaScript #60

Merged
merged 1 commit into from
Aug 25, 2017
Merged

Disable JavaScript #60

merged 1 commit into from
Aug 25, 2017

Conversation

chosak
Copy link
Member

@chosak chosak commented Aug 25, 2017

This commit disables JavaScript on this app's pages by removing (commenting out) the inclusion of the JS library.

While this app is in pre-release mode and is still being developed, removing the use of JS will help to simplify testing. The app should also support non-JS users as well, so removing JS for now will help to guarantee that all functionality works when JS is disabled.

Removals

  • JS, from the base template.

Checklist

  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the front end playbook
  • Passes all existing automated tests
  • Visually tested in supported browsers and devices

This commit disables JavaScript on this app's pages by removing
(commenting out) the inclusion of the JS library.

While this app is in pre-release mode and is still being developed,
removing the use of JS will help to simplify testing. The app should
also support non-JS users as well, so removing JS for now will help to
guarantee that all functionality works when JS is disabled.
@ascott1
Copy link
Member

ascott1 commented Aug 25, 2017

This seems reasonable during active development. The current state of the JS is pretty broken and this would remove that blocker.

@grapesmoker
Copy link
Contributor

Hmm, I don't know about this one. On the one hand, it's not wrong to say that there are some problems with the JS and that disabling it would make backend stuff a little easier to troubleshoot. On the other, I don't know how much of the functionality of the eRegs frontend will gracefully degrade (the SxS stuff, for example). I guess I don't have any real problems with this, but we should not be complacent about fixing the frontend problems. There aren't that many of them, but they kind of make it hard to test all the aspects of the backend (the AJAXy ones) and we're going to want to do that before we get on build.

@chosak
Copy link
Member Author

chosak commented Aug 25, 2017

@grapesmoker can you clarify: is there functionality that requires JS to work? If so, is that also the case in the existing eRegs, or is that requirement new to eregs-2.0?

@grapesmoker
Copy link
Contributor

I actually don't know. I'm not sure if the sidebars require JS, because they have these dynamic dropdowns that look like they might, but maybe that's all CSS-transition stuff. @ascott1 knows, so if he thinks this is fine, it's probably fine.

@chosak chosak merged commit 6fa0845 into master Aug 25, 2017
@chosak chosak deleted the no-js branch August 25, 2017 16:36
@ascott1
Copy link
Member

ascott1 commented Aug 25, 2017

Everything should resolve to an actual URL, including the dynamic sidebar stuff, and things like the interpretation expandables should default to open without js.

@grapesmoker
Copy link
Contributor

Cool, thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants