Fix accessibility issues in SGA #87
Conversation
|
@amir-qayyum-arbisoft it looks like this needs a rebase? FYI, @bdero is going to put this on sandbox and we'll see if we can get it reviewed by an MIT accessibility specialist. |
amir-qayyum-khan
force-pushed
the
accessibility_fix
branch
from
a77b6a2
to
cfa465b
Compare
|
This is on sandxox2o |
| @@ -48,13 +48,19 @@ function StaffGradedAssignmentXBlock(runtime, element) { | |||
| return; | |||
| } | |||
| } | |||
| $(content).find(".upload").attr("role", "progressbar"); | |||
| $(content).find(".upload").attr("aria-valuemax", "100"); | |||
| $(content).find(".upload").attr("aria-valuemin", "0"); | |||
There was a problem hiding this comment.
A loop would probably be overkill for this, but please store the element before making these calls:
var uploadElement = $(content).find(".upload");
uploadElement.attr(/* params */)
// ...etc...
amir-qayyum-khan
force-pushed
the
accessibility_fix
branch
from
618f49b
to
c818401
Compare
|
@amir-qayyum-khan are there any changes we should consider for this, based on what you heard from edX? |
amir-qayyum-khan
force-pushed
the
accessibility_fix
branch
3 times, most recently
from
c2119c4
to
d659fee
Compare
|
How might I test this? |
|
@jetej You can use screen reading tools like chromeVox (A chrome browser plugin) or command+F5 on mac to verify this PR. |
|
Seems to work ok. I was able to use VoiceOver with both the student and instructor UI. |
|
We'll still need another round of accessibility review after this, but this seems worth merging. |
| <% } %> | ||
| <div class="upload"> | ||
| <div class="upload" aria-describedby="upload_lable" aria-live="polite"> |
There was a problem hiding this comment.
typo: should these be "upload_label"
|
Let's merge after the typo fix. This will all need another round of accessibility review afterwards, but this is an improvement. |
amir-qayyum-khan
force-pushed
the
accessibility_fix
branch
2 times, most recently
from
31673b1
to
b02665d
Compare
|
@pdpinch issue fixed |
amir-qayyum-khan
changed the title
amir-qayyum-khan
force-pushed
the
accessibility_fix
branch
from
68bb1b8
to
003d1d9
Compare
| @@ -100,7 +101,7 @@ | |||
| <% } %> | |||
| </td> | |||
| <td> | |||
| <div class="upload"> | |||
| <div class="upload" aria-live="assertive" role="alert" tabindex="1"> | |||
| {% for name, field in fields %} | ||
| <tr> | ||
| <td>{{name}}</td> | ||
| <td tabindex="1">{{name}}</td> |
amir-qayyum-khan
force-pushed
the
accessibility_fix
branch
2 times, most recently
from
2d731a0
to
a138ad5
Compare
amir-qayyum-khan
force-pushed
the
accessibility_fix
branch
from
433c817
to
664d138
Compare
|
Done @pdpinch
|
|
@amir-qayyum-khan not urgent, but could you rebase this sometime? I'm not sure if it's still relevant, but I'd like to clean this up. |
|
@amir-qayyum-khan do you know if we have an issue for this? |
|
I dont think so @pdpinch |
|
@amir-qayyum-khan can you address the merge conflicts? |
In this PR I fixed accessibility related issues. Added necessary ARIA tags into code.
@carsongee and @pdpinch please review it
Thanks