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

2075: some dialogs email settings unenroll not keyboard navigable #2079

Conversation

collinpreston
Copy link
Contributor

What are the relevant tickets?

#2075

Description (What does it do?)

Resolves in issue with using the keyboard to navigate and control the browser when the either the unenroll modal or email settings modal is open for a course enrollment.

How can this be tested?

You should have a course created and be enrolled.

  1. Visit http://mitxonline.odl.local:8013/dashboard/.
  2. Using only your keyboard, open the unenroll modal or email settings modal from the course enrollment card.
  3. Verify that you can move focus to one of the buttons on the modal. When you press enter, the focused button should perform it's expected action (close the modal, or submit).

Comment on lines +547 to +548
{this.renderRunUnenrollmentModal(enrollment)}
{this.renderEmailSettingsDialog(enrollment)}
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki Feb 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick explanation: Reactstrap Dropdowns attach event handlers to all react children (not dom children); the handlers call preventDefault on keydown events, see https://github.com/reactstrap/reactstrap/blob/6085ac0bb340f3e2cdae1aec4e6abec38c37ebad/src/Dropdown.js#L354. Normally, a keydown event on a button element is followed by a click event; but if you call preventDefault on the keydown event, the click event never happens.

There's no reason, afaik, that the modal needs to be a child of the dropdown, so moving it outside the dropdown fixes the issue.

@ChristopherChudzicki ChristopherChudzicki self-assigned this Feb 2, 2024
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Working as expected now.

@ChristopherChudzicki
Copy link
Contributor

ChristopherChudzicki commented Feb 2, 2024

@collinpreston Actually, looking at the code, I suspect that renderProgramUnenrollmentModal has the same issue. Might be good to change that here, too.

I'm not sure how to trigger that dialog, though.

@collinpreston collinpreston merged commit dfde0ff into main Feb 2, 2024
3 checks passed
@collinpreston collinpreston deleted the 2075-some-dialogs-email-settings-unenroll-not-keyboard-navigable branch February 2, 2024 15:15
@odlbot odlbot mentioned this pull request Feb 2, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some dialogs ("Email Settings", "Unenroll") not keyboard navigable.
2 participants