Skip to content

Conversation

@nasirhjafri
Copy link
Contributor

MCKIN-12203 AF#91-MRQ-Ensure custom controls are keyboard accessible

Description

[Issue]
The feedback icons aren't keyboard accessible since div elements do not natively receive keyboard focus.

[User Impact]
When custom elements do not provide keyboard access, keyboard only users and users of assistive technology may not be able to use or interact with the custom control.

[Code Reference]

Environment

[Recommended Solution]
Use the element to ensure keyboard focus by default.

@nasirhjafri
Copy link
Contributor Author

@xitij2000 Please review.

@nasirhjafri
Copy link
Contributor Author

@xitij2000 Reminder: to review this one, thanks.

@lgp171188
Copy link
Contributor

@nasirhjafri, I am reviewing this today and hope to have a review pass done before my EOD today.

@lgp171188 lgp171188 self-requested a review November 26, 2019 14:28
@lgp171188
Copy link
Contributor

@nasirhjafri, I tested this on the Ironwood devstack by adding a problem-builder XBlock MRQ to a course and then tabbing through the page elements with the keyboard and verified that the buttons with the choice-result class are accessible via the keyboard and highlighted when focussed.

PB_MRQ_keyboard_accessibility

However I see a couple of issues, which are highlighted in the screenshot. The dotted border for the highlight extends slightly beyond the edges of the highlight background color and that looks odd. Also with the dotted border, the button appears to overlap a bit with the checkbox which makes it look ugly. Perhaps the margin/padding need to be updated? Increasing the margin from the browser developer tools worked as a temporary fix for me.

nasirhjafri and others added 2 commits November 26, 2019 23:39
Co-Authored-By: Guruprasad <lgp171188@users.noreply.github.com>
Copy link
Contributor

@lgp171188 lgp171188 left a comment

Choose a reason for hiding this comment

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

@nasirhjafri,:+1:. This is good to merge after addressing the version bump review comment

  • I tested this on the Ironwood devstack and verified that the feedback controls are keyboard accessible. I also verified that the issues raised in my previous comments are now fixed.
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation NA

@nasirhjafri
Copy link
Contributor Author

@lgp171188 Thanks for reviewing.

@lgp171188 lgp171188 merged commit f51d421 into open-craft:master Nov 26, 2019
@nasirhjafri nasirhjafri deleted the MCKIN-12203 branch December 10, 2019 12:51
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.

2 participants