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

[refs #721] Add menu and search toggles when JS is disabled #777

Closed
wants to merge 1 commit into from

Conversation

jtuds
Copy link

@jtuds jtuds commented Oct 1, 2021

This commit adds toggle functionality for the menu and search header buttons
when the user has JS disabled, via input checkbox toggles.
See #721

Description

Checklist

This commit adds toggle functionality for the menu and search header buttons
when the user has JS disabled, via input checkbox toggles.
See nhsuk#721
@chrimesdev
Copy link
Member

Hey @jtuds thanks for opening this pull request, the team will put some time in to review this soon.

@andymantell
Copy link
Contributor

Thanks for putting this together @jtuds. It's a nice solution, but I have some concerns about trying to replicate this without JavaScript:

Firstly, I have tested it on Android with TalkBack enabled and I'm unable to open and close the menu still. It works fine if I turn TalkBack off again. I haven't been able to check Voiceover on iOS unfortunately.

I also wonder, if someone has explicitly disabled JavaScript whether they might actually prefer this sort of behaviour to not be present. I.e. I am questioning the expectation set out in #721 - I think we should bottom that out before we make a change like this.

With JavaScript off, I think it would be much simpler if the menu simply remained open and visible without the need for interaction. It keeps the code simpler, and makes the experience simpler for someone that has opted for a more streamlined experience. And the same for the search box. I think this is something that should be fed into #711

I don't know whether we want to put in a fix for the current header to do something like this - i..e showing the menu and search by default when JS is disabled, or whether we want to just roll this all into #711. I'll defer to @chrimesdev on that decision...

Sorry to put a negative spin on this! We really do appreciate the work you've put in and absolutely welcome contributions

@jtuds
Copy link
Author

jtuds commented Oct 6, 2021

No problem thanks for the feedback :) I completely agree the simpler approach to this is to just show the navigation and search without any interaction, but I suppose that just depends on the desired functionality with JS disabled which I don't have enough context for.

Happy to wait for more thinking around this. If it is decided that this approach is the right one I would perhaps suggest raising the accessibility issues as a separate ticket in order to get a fix out for the non-Javascript functionality - though the importance/urgency of this fix depends on the stats on users coming to the site without JS enabled.

@andymantell
Copy link
Contributor

Thanks @jtuds
I'm going to close this for the moment, but if we come back in this direction it's all cross-linked from the issues.
Cheers

@andymantell andymantell closed this Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants