Skip to content

MCKIN-12194 To close MCQs MRQs dialogue via keyboard to make them accessible through keyboard - #248

Merged
lgp171188 merged 6 commits into
open-craft:masterfrom
musmanmalik:usman/MCKIN-12194
Nov 28, 2019
Merged

MCKIN-12194 To close MCQs MRQs dialogue via keyboard to make them accessible through keyboard#248
lgp171188 merged 6 commits into
open-craft:masterfrom
musmanmalik:usman/MCKIN-12194

Conversation

@musmanmalik

@musmanmalik musmanmalik commented Nov 13, 2019

Copy link
Copy Markdown
Contributor

The feedback tooltip non-modal dialogs cannot be closed via keyboard. The screen reader does not announce anything when the button receives keyboard focus. Users of assistive technology and keyboard-only users don't have access to controlling the dialogs.

When we submit response of MCQ or MRQ having tips dialogue. The dialogue had close control which cannot be closed with keyboard. In this Pull Request I have made them accessible by changing div to button and some supporting style.

@xitij2000 @Agrendalath Please review this PR .

@lgp171188

Copy link
Copy Markdown
Contributor

@musmanmalik, I am reviewing this PR now.

@lgp171188
lgp171188 self-requested a review November 25, 2019 16:23
@lgp171188

Copy link
Copy Markdown
Contributor

@musmanmalik, I added an MCQ XBlock with a tip message for a choice to a test course on the Ironwood devstack. However, I could not get the close button to show up at all. There is CSS to hide it and I could only get it to show up by manually disabling that style using the browser developer console. Is this expected or did I miss configuring something? If it is the latter, can you post a detailed list of testing instructions?

With the close button visible, I was able to see the style to highlight the close button on keyboard focus.

Without keyboard focus
BB-1866_unfocussed

With keyboard focus
BB-1866_focussed

@musmanmalik

Copy link
Copy Markdown
Contributor Author

Hi @lgp171188 thank you so much for your response.
By default the close control is hidden by using css which you pointed; yes its expected behavior of close button and you did not miss any thing here and we need to enable it manually for testing as you have done.

Previously it was div element which can not get keyboard focus. This have been changed to button which is accessible via keyboard. So there is need to test close button is accessible or not.

So I have tested it by using xblock-sdk.

@lgp171188

Copy link
Copy Markdown
Contributor

@musmanmalik, testing using the XBlock SDK makes no sense if the actual usage of the XBlock doesn't show the close button for which these changes have been made. So should this PR be tested in the solutions devstack or the associated custom LMS frontend?

@musmanmalik

musmanmalik commented Nov 27, 2019

Copy link
Copy Markdown
Contributor Author

@lgp171188 By default its hidden in xblock. We are using custom LMS and we have enabled this close button in our environment.
We are working on accessibility. So in order to meet accessibility target we need to change this to button instead of div to make them keyboard accessible.
Let me know if anything required further from my side.

cc: @xitij2000

@lgp171188

Copy link
Copy Markdown
Contributor

@musmanmalik, since I don't have a working solutions devstack and don't want to spend a lot of time trying to get it to work, is this PR available somewhere on some QA/integration or similar environments so that I can test it directly there?

@musmanmalik

Copy link
Copy Markdown
Contributor Author

@lgp171188 No, This could not be available on QA/Integration or any environment at our side until not merged in xblock.

cc: @xitij2000

@lgp171188

Copy link
Copy Markdown
Contributor

@musmanmalik, I asked for a teammate's help to test and verify this on the custom frontend.

@Agrendalath Agrendalath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍

  • I tested this: tested the problem-builder with and without this change on the custom frontend
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation

@musmanmalik

musmanmalik commented Nov 28, 2019

Copy link
Copy Markdown
Contributor Author

@lgp171188 , @Agrendalath Please create a release tag as well.

@lgp171188

Copy link
Copy Markdown
Contributor

@musmanmalik, since the v3.3.14 has already been created, can you merge/rebase the master branch and bump the version to 3.3.15? I will merge this PR and create a release tag once that is done.

@musmanmalik

musmanmalik commented Nov 28, 2019

Copy link
Copy Markdown
Contributor Author

@lgp171188 version updated; kindly merge it and create. Many thanks.

@lgp171188
lgp171188 merged commit 7f628cf into open-craft:master Nov 28, 2019
@lgp171188

Copy link
Copy Markdown
Contributor

@musmanmalik, I have merged this PR and tagged the v3.3.15 release.

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