Moodle
  1. Moodle
  2. MDL-41252

Simple changes to main course page to improve accessibility

    Details

    • Testing Instructions:
      Hide

      in a course, select Topic format and course layout: 'Show all section in one page'. With a screenreader (I use chromevox), on the course view.php page, check that all topic region are announced.

      Change course layout settings for one section per page. Check that all section are still announced.

      Show
      in a course, select Topic format and course layout: 'Show all section in one page'. With a screenreader (I use chromevox), on the course view.php page, check that all topic region are announced. Change course layout settings for one section per page. Check that all section are still announced.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:
      MDL-41252-master
    • Story Points (Obsolete):
      13
    • Sprint:
      FRONTEND Sprint 6

      Description

      We can not navigate through the course page easily. H3 element are currently the way to understand that we are in a new section - if a content contain an h3 it may be confused as a new section. If we add the region aria role, then we could use screenreader shortcut to navigate between section.

      Original report: "The main course page could use a few simple changes that would help identify each topic as a region and label them with the proper label. The changes can be made without changing the page structure or visual appearance."

        Gliffy Diagrams

          Activity

          Hide
          Nate Baxley added a comment -

          I'm attaching a patch file that makes the changes I described.

          Show
          Nate Baxley added a comment - I'm attaching a patch file that makes the changes I described.
          Hide
          Michael de Raadt added a comment -

          Thanks for suggesting this and providing a patch.

          Show
          Michael de Raadt added a comment - Thanks for suggesting this and providing a patch.
          Hide
          moodle.com added a comment -

          this will require identifying the best practice for each area altered, and ensuring the patch follows that best practice. It will also be needed to be backported to be included in master at this point (bugs only, not improvements now that the code freeze is in place)

          Show
          moodle.com added a comment - this will require identifying the best practice for each area altered, and ensuring the patch follows that best practice. It will also be needed to be backported to be included in master at this point (bugs only, not improvements now that the code freeze is in place)
          Hide
          Jérôme Mouneyrac added a comment - - edited

          Hi Nate,
          thanks for your contribution, just as a note instead of a patch I recommend to do a git commit to keep the author credits.

          Show
          Jérôme Mouneyrac added a comment - - edited Hi Nate, thanks for your contribution, just as a note instead of a patch I recommend to do a git commit to keep the author credits.
          Hide
          Jérôme Mouneyrac added a comment -

          Nate I changed the aria region to put them on the li tag it seems to me that they are more the full region. What do you think?

          Show
          Jérôme Mouneyrac added a comment - Nate I changed the aria region to put them on the li tag it seems to me that they are more the full region. What do you think?
          Hide
          Ankit Agarwal added a comment - - edited

          Hi guys,
          The title of this issue doesn't really define the scope of this issue. I would suggest updating it to reflect the changes.
          The only thing I can recommend is using aria-labledby and pointing it to the actual section header instead of using aria-label.

          However if you have a good reason to do it the way you have done, please feel free to go forward.

          Thanks

          Show
          Ankit Agarwal added a comment - - edited Hi guys, The title of this issue doesn't really define the scope of this issue. I would suggest updating it to reflect the changes. The only thing I can recommend is using aria-labledby and pointing it to the actual section header instead of using aria-label. However if you have a good reason to do it the way you have done, please feel free to go forward. Thanks
          Hide
          Jérôme Mouneyrac added a comment - - edited

          Thanks Ankit for reviewing. Reading (http://www.w3.org/TR/wai-aria/roles#region) again I agree that we likely need the aria-labelledby.

          I'll ask the accessibility mailinglist (moodle@listserv.illinois.edu) to see what they think of this issue.

          Show
          Jérôme Mouneyrac added a comment - - edited Thanks Ankit for reviewing. Reading ( http://www.w3.org/TR/wai-aria/roles#region ) again I agree that we likely need the aria-labelledby. I'll ask the accessibility mailinglist (moodle@listserv.illinois.edu) to see what they think of this issue.
          Hide
          Nate Baxley added a comment -

          Jerome, Sorry about the delay getting back to you. As for putting the region on the li: what content will be in the left and right sides? In our theme it's only spacer graphics. The div.content seems to be where the actual content that would be most useful to the user is placed. I'll demur to your judgment as to the real use for the left and ride side divs.

          Show
          Nate Baxley added a comment - Jerome, Sorry about the delay getting back to you. As for putting the region on the li: what content will be in the left and right sides? In our theme it's only spacer graphics. The div.content seems to be where the actual content that would be most useful to the user is placed. I'll demur to your judgment as to the real use for the left and ride side divs.
          Hide
          Jérôme Mouneyrac added a comment -

          Thanks Nate for your answer. The li contains the action icons related to the section, like moving the section around or hidding the section. It made sense to me to have these action icons into the section region.

          Btw are you on the illinois mailing list for accessibility? I sent a message there but I receive no answers.

          Basically what would be good on this issue is to:

          • explain what is the initial problem and explain the step to reproduce it - personally I guessed you had some kind of shortcut to go from region to region and it's why you would want this. I also thought that was a good idea to announce a section as region, instead to just rely on the section been announce by an h3 (could be confusing if a content contain an h3).
          • explain what is the expected solution/behavior. I also can deduce from the patch but it is simpler for people peer-reviewing, testing, just quickly looking at the issue... they don't need to look at the patch. Also it avoids me to make a wrong assumption.

          Anyway I think I'm ok here, I'll fix the missing aria-labelledby, add more info, test instructions and send it to integration. Thanks Nate for the report, the patch and following up

          Show
          Jérôme Mouneyrac added a comment - Thanks Nate for your answer. The li contains the action icons related to the section, like moving the section around or hidding the section. It made sense to me to have these action icons into the section region. Btw are you on the illinois mailing list for accessibility? I sent a message there but I receive no answers. Basically what would be good on this issue is to: explain what is the initial problem and explain the step to reproduce it - personally I guessed you had some kind of shortcut to go from region to region and it's why you would want this. I also thought that was a good idea to announce a section as region, instead to just rely on the section been announce by an h3 (could be confusing if a content contain an h3). explain what is the expected solution/behavior. I also can deduce from the patch but it is simpler for people peer-reviewing, testing, just quickly looking at the issue... they don't need to look at the patch. Also it avoids me to make a wrong assumption. Anyway I think I'm ok here, I'll fix the missing aria-labelledby, add more info, test instructions and send it to integration. Thanks Nate for the report, the patch and following up
          Hide
          Jérôme Mouneyrac added a comment -

          Actually I just noticed but we already have the aria-label attribut (which explain that it already is correctly announced), sending to integration.

          Show
          Jérôme Mouneyrac added a comment - Actually I just noticed but we already have the aria-label attribut (which explain that it already is correctly announced), sending to integration.
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Marina Glancy added a comment -

          Hi Jerome, why don't we backport it to at least 2.5?

          Show
          Marina Glancy added a comment - Hi Jerome, why don't we backport it to at least 2.5?
          Hide
          Jérôme Mouneyrac added a comment -

          I'll backport it to 2.5

          Show
          Jérôme Mouneyrac added a comment - I'll backport it to 2.5
          Hide
          Jérôme Mouneyrac added a comment -

          done

          Show
          Jérôme Mouneyrac added a comment - done
          Hide
          Marina Glancy added a comment -

          Thanks Jerome, integrated in 2.5 and 2.6

          Show
          Marina Glancy added a comment - Thanks Jerome, integrated in 2.5 and 2.6
          Hide
          Mark Nelson added a comment -

          Thanks Jerome, works as expected. As discussed if I switch between the sections quickly then the heading is not read out, but it seems this is an issue with the screen reader and not Moodle.

          Show
          Mark Nelson added a comment - Thanks Jerome, works as expected. As discussed if I switch between the sections quickly then the heading is not read out, but it seems this is an issue with the screen reader and not Moodle.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          "Aequam memento rebus in arduis servare mentem"

          Many thanks for your hard work, this is now part of "Moodle, the LMS". Closing!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - "Aequam memento rebus in arduis servare mentem" Many thanks for your hard work, this is now part of "Moodle, the LMS". Closing! Ciao

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Agile