MCKIN-12227 Survey table labels and roles

[murad-hubib]

• Added table roles
• Added meaningful aria-labels of radio buttons for better accessibility

These are supportive attributes for accessibility and doesn't have any visual change

Ticket:

https://edx-wiki.atlassian.net/browse/MCKIN-12227

@xitij2000 Please review and merge it as per your schedule. Also please let me know if we need to bump the version as well

[xitij2000]

We'll get to the full review next sprint, but for the version I think you should keep the patch number, so 1.9.0

[Agrendalath]

@xitij2000, this doesn't look like a big change, so shouldn't we just bump the patch version (1.8.10)?
I think bumping minor would be more suitable for a PR like #66.

[xitij2000]

@Agrendalath Yeah, I wasn't commenting on the version bump itself, but mainly the format of the version number. I think 1.8.10 makes more sense here.

[murad-hubib]

@Agrendalath I have bumped the version as well. Please go ahead

[Agrendalath]

cc: @xitij2000 as the reviewer here.

[xitij2000]

@wittjeff This is a PR to improve accessibility of an XBlock that's included in edx-platform. Would you be able to have a quick look and see if it's going in the right direction?
I've tested on NVDA, but I'm not sure it's helping.

[msaqib52]

@murad-hubib some test cases are failing, could you fix those in the meantime.

[xitij2000]

@msaqib52 @murad-hubib Note that there are some failures that are not caused by this PR.
We have a task to fix those but it is unlikely we will get to it this sprint. If you'd like you can include those fixes as well, otherwise we will pick up that work when it's prioritised.

[wittjeff]

@murad-hubib @xitij2000 You code seems mostly sensible but I don't feel confident in saying so without seeing it live somewhere. Also it just seems to make the code "look" semantically more like the Poll component I see in Studio, so I don't know where these forked. Is there a sandbox with your changes?

And by the way, I don't mind looking over changes like this, but please understand that my inbox has a steady heavy flow of Github PR updates, and I may not notice when you tag me. So if I don't respond, just email me directly.

[wittjeff]

Here's DOM output from a poll in current Studio:

<h3 class="poll-header">Poll</h3>

<form>
  <fieldset class="poll-container">
    <legend class="poll-question">
      <p>What is your favorite color?</p>
    </legend>
    <div class="poll-answers">
      
        <div class="poll-answer">
          <div class="poll-input-container">
            <input type="radio" name="choice" id="poll-ca0352665a474c4a94cfc8707666fed6-answer-R" value="R">
          </div>
          
          <label class="poll-answer-text" for="poll-ca0352665a474c4a94cfc8707666fed6-answer-R"><p>Red</p></label>
        </div>
      
        <div class="poll-answer">
          <div class="poll-input-container">
            <input type="radio" name="choice" id="poll-ca0352665a474c4a94cfc8707666fed6-answer-B" value="B">
          </div>
          
          <label class="poll-answer-text" for="poll-ca0352665a474c4a94cfc8707666fed6-answer-B"><p>Blue</p></label>
        </div>
      
        <div class="poll-answer">
          <div class="poll-input-container">
            <input type="radio" name="choice" id="poll-ca0352665a474c4a94cfc8707666fed6-answer-G" value="G">
          </div>
          
          <label class="poll-answer-text" for="poll-ca0352665a474c4a94cfc8707666fed6-answer-G"><p>Green</p></label>
        </div>
      
        <div class="poll-answer">
          <div class="poll-input-container">
            <input type="radio" name="choice" id="poll-ca0352665a474c4a94cfc8707666fed6-answer-O" value="O">
          </div>
          
          <label class="poll-answer-text" for="poll-ca0352665a474c4a94cfc8707666fed6-answer-O"><p>Other</p></label>
        </div>
      
    </div>
  </fieldset>
  <input class="input-main" type="button" name="poll-submit" value="Submit" disabled="">
</form>

<div class="poll-voting-thanks
    poll-hidden">
  <span>Thank you.</span>
</div>

<div class="poll-submissions-count poll-hidden">
  
    You have used <span class="poll-current-count">{{ submissions_count }}</span> out of <span class="poll-max-submissions">{{ max_submissions }}</span> submissions.
  
</div>

[xitij2000]

@murad-hubib @xitij2000 You code seems mostly sensible but I don't feel confident in saying so without seeing it live somewhere. Also it just seems to make the code "look" semantically more like the Poll component I see in Studio, so I don't know where these forked. Is there a sandbox with your changes?

And by the way, I don't mind looking over changes like this, but please understand that my inbox has a steady heavy flow of Github PR updates, and I may not notice when you tag me. So if I don't respond, just email me directly.

@wittjeff Thanks for having a look!
Note that this is the Poll XBlock that's available on the edx-plaform (as seen here). The edx-platform version is just one patch version behind.

I will try to set up a sandbox with this version of the component soon and will link it here when it's ready.

[xitij2000]

@wittjeff Here is the sandbox for this PR:

• LMS: https://xblock-poll-test.opencraft.hosting/
• Studio: https://studio.xblock-poll-test.opencraft.hosting/

I am giving this conditional approval based on testing on Orca which seems to work better with this.
👍

• I tested this: tested using NVDA/Windows and Orca/Linux
• I read through the code
• I checked for accessibility issues (some issues with NVDA)

[wittjeff]

Code looks like proper HTML + ARIA.

[xitij2000]

Thanks @wittjeff!

[musmanmalik]

@xitij2000 We have fixed the test that relates to this PR. But some other test is failing. Can you please check

If this is Flake Please rebuild this.

[xitij2000]

@xitij2000 We have fixed the test that relates to this PR. But some other test is failing. Can you please check

If this is Flake Please rebuild this.

Yes, this is a flaky test ATM, we have a future task to fix this. If you're like to contribute a fix, it will be welcome. I will not block this PR for that test, and since the other tests seem to have passed, this is good to merge.