ifmeorg / ifme

Free, open source mental health communication web app to share experiences with loved ones

http://www.if-me.org/

GNU Affero General Public License v3.0

1.46k stars 734 forks source link

Keyboard navigation in mobile web (Issue #2159) #2166

Closed LMulvey closed 1 year ago

LMulvey commented 1 year ago

Description

Few a11y fixes related to the Header component

Fixed bug where toggle was being called every time onKeyDown was triggered on the hamburger button

navigation-bug-fixed

Introduced focus trapping for the mobile navigation bar while open - you can no longer focus out of the nav via the keyboard without closing it

trap-focus

Upgraded @testing-library/jest-dom, @testing-library/react and @testing-library/user-event - the dependency update was largely to use the updated userEvent.keyboard event support from @testing-library/user-event but I wanted to ensure the dependencies were in sync.

Corresponding Issue

2159

Reviewing this pull request? Check out our Code Review Practices guide if you haven't already!

LMulvey commented 1 year ago

Obviously broke some things with the test upgrades. I'm working through the changes - there were a few API updates with @testing-library over some versions.

@julianguyen - happy to update all existing tests to still function. The benefit here is that we get a more realistic testing environment from user-event (See: Differences from fireEvent) but I definitely don't want to disrupt things too much so let me know if you'd like me to step back here and I can update the Header.spec tests I've added to try and get them to work with the older APIs.

julianguyen commented 1 year ago

@LMulvey Thanks for the summary and for taking this on 🎉 ! Yeah, let's update the existing tests like you said.

LMulvey commented 1 year ago

Okay! Tests are updated but there are a few caveats.

I've had to skip three tests for now - I need to think about an approach a bit. I left a comment on all three tests describing why they were skipped. Still, in summary, @testing-library/user-event v14 dropped support for event.keyCode (See: https://github.com/testing-library/user-event/issues/842).

react-autosuggest is pretty outdated and still exclusively looks for event.keyCode when detecting whether or not a user hit the Enter key. Because of this, our Enter inputs from tests are not working with any components that use react-autosuggest.

I've tried manually dispatching an event with JSDom (you can see my scraps in the InputTag.spec tests) but it wasn't working. I can try to submit a PR to react-autosuggest but it seems to be unmaintained at this point.

Does anyone have any ideas? 🤔

cc: @julianguyen

LMulvey commented 1 year ago

@julianguyen Thank you for all the feedback! I've pushed up another round of fixes and created #2170 to cover off the react-autosuggest changeover task. I should be able to take that on next week once this PR is wrapped up 🤝