Moodle
  1. Moodle
  2. MDL-35816 Accessibility Review issues (Deque)
  3. MDL-35876

Move here descriptions need to be more verbose when moving blocks on the view profile page

    Details

    • Testing Instructions:
      Hide
      1. With javascript off turn on editing blocks
      2. move a block
      3. inspect the dropzone for the block you are moving
      4. ensure it contains text describing the location it will be moved to - that is, before or after another block.
      5. Enable the mymobile theme
      6. Make sure there are no errors on the screen.
      Show
      With javascript off turn on editing blocks move a block inspect the dropzone for the block you are moving ensure it contains text describing the location it will be moved to - that is, before or after another block. Enable the mymobile theme Make sure there are no errors on the screen.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-35876-master
    • Rank:
      44641

      Description

      Issue
      Keyboard access - Once in "Customization" mode, it is difficult to move the blocks around, using only the keyboard because the move here description for each area is not unique to a user. A non sighted user with a screen reader cannot tell where they are moving the block to on the screen.

      Standard Level
      WCAG 2 2.1.1 (A) http://www.w3.org/WAI/WCAG20/quickref/#qr-keyboard-operation-keyboard-operable

      Impact

      Example Link
      http://demo.moodle.net/user/profile.php?id=0&sesskey=ZdAicfllpV&bui_moveid=293

      Test Steps

      1. Log in as teacher/admin
      2. Navigate to a course
      3. Turn off JavaScript in your browser
      4. Click on a block move icon; the page should reload
      5. Inspect content around a target location

        Activity

        Hide
        Michael de Raadt added a comment -

        I wasn't able to replicate this problem. I was able to move blocks on the profile page using only the keyboard.

        This is not the case for course pages, though. There, with JavaScript on, you cannot move blocks or activities/resources using the keyboard alone.

        Show
        Michael de Raadt added a comment - I wasn't able to replicate this problem. I was able to move blocks on the profile page using only the keyboard. This is not the case for course pages, though. There, with JavaScript on, you cannot move blocks or activities/resources using the keyboard alone.
        Hide
        Jason Hardin added a comment -

        I see what you mean. I was able to do this on the demo page. I was at least able to move it to the navigation block to the center column i was not able to move the block to below the settings block. I could select the dotted area but the block would not move.

        Based on what NVDA read I would say that each of the Move block here descriptions need to be more descriptive. Such as Move Navigation block below settings block or Move navigation block above settings block. At minimum I would need to know that i am moving it above or below and what area.

        So the description in this ticket definitely needs to be changed or split into 2 tickets one for the JavaScript issue in course and one for the move to text being more verbose.

        Show
        Jason Hardin added a comment - I see what you mean. I was able to do this on the demo page. I was at least able to move it to the navigation block to the center column i was not able to move the block to below the settings block. I could select the dotted area but the block would not move. Based on what NVDA read I would say that each of the Move block here descriptions need to be more descriptive. Such as Move Navigation block below settings block or Move navigation block above settings block. At minimum I would need to know that i am moving it above or below and what area. So the description in this ticket definitely needs to be changed or split into 2 tickets one for the JavaScript issue in course and one for the move to text being more verbose.
        Hide
        Michael de Raadt added a comment -

        Hi, Jason.

        I agree. Could you edit this issue and reword it to ask for more verbose descriptions of targets and add a second issue relating to moving blocks in course pages.

        Show
        Michael de Raadt added a comment - Hi, Jason. I agree. Could you edit this issue and reword it to ask for more verbose descriptions of targets and add a second issue relating to moving blocks in course pages.
        Hide
        Jason Fowler added a comment -

        Any idea what the text should be like Jason? I have my own ideas but would like some input if possible, so I don't go off in the wrong direction.

        Show
        Jason Fowler added a comment - Any idea what the text should be like Jason? I have my own ideas but would like some input if possible, so I don't go off in the wrong direction.
        Hide
        Jason Hardin added a comment -

        My idea for the wording is to use above or below with the block name that the block would be placed in which column and above or below which blocks. So if I have a column with calendar, latest news and people I would see 4 options.
        1. Move to sidepre column above the calendar block
        2. Move to sidepre column above the latest news block
        3. Move to sidepre column above the people block
        4. Move to sidepre column below the people block

        For a really descriptive phraseIi would combined the above and below in 2 and 3 to read
        2. Move to sidepre column above the latest news and below the calendar blocks.
        3. Move to sidepre column above the people and below the latest news blocks.

        The second part may be overkill, but would convey the most accurate spacial reference for the block. You may also want to add the name of the block being moved to make sure the user remembers which block they are moving as they tab through the move areas.

        The other thing that you may want to happen is lock focus on only the move areas and provide the user with a way to cancel out of the move action. Containing focus would allow a user to click move block then tab through each of the move areas and select the one to move to. This would speed up their ability to move the block without having to tab through the links in the other blocks.

        Show
        Jason Hardin added a comment - My idea for the wording is to use above or below with the block name that the block would be placed in which column and above or below which blocks. So if I have a column with calendar, latest news and people I would see 4 options. 1. Move to sidepre column above the calendar block 2. Move to sidepre column above the latest news block 3. Move to sidepre column above the people block 4. Move to sidepre column below the people block For a really descriptive phraseIi would combined the above and below in 2 and 3 to read 2. Move to sidepre column above the latest news and below the calendar blocks. 3. Move to sidepre column above the people and below the latest news blocks. The second part may be overkill, but would convey the most accurate spacial reference for the block. You may also want to add the name of the block being moved to make sure the user remembers which block they are moving as they tab through the move areas. The other thing that you may want to happen is lock focus on only the move areas and provide the user with a way to cancel out of the move action. Containing focus would allow a user to click move block then tab through each of the move areas and select the one to move to. This would speed up their ability to move the block without having to tab through the links in the other blocks.
        Hide
        Jason Fowler added a comment -

        Locking focus is a huge undertaking, and would probably require rebuilding the source order, or at least some fancy z-index and modal control javascript, far beyond the scope of this issue.

        Will look into adding the first part for now. Even this may be a little ambitious ... can't tell off the top of my head though.

        Show
        Jason Fowler added a comment - Locking focus is a huge undertaking, and would probably require rebuilding the source order, or at least some fancy z-index and modal control javascript, far beyond the scope of this issue. Will look into adding the first part for now. Even this may be a little ambitious ... can't tell off the top of my head though.
        Hide
        Jason Fowler added a comment -

        having spoken about this yesterday in Scrum, I was asked if the amount of work this would require is actually worth the pay off right now. I'll be doing a little investigation into the amount of work it would require to get this done, and deciding on that later today.

        Show
        Jason Fowler added a comment - having spoken about this yesterday in Scrum, I was asked if the amount of work this would require is actually worth the pay off right now. I'll be doing a little investigation into the amount of work it would require to get this done, and deciding on that later today.
        Hide
        Jason Fowler added a comment -

        turns out master/lib/ajax/blocks.php demonstrates most of what is needed - I will need to translate it to the none AJAX interface, if it isn't already there, but it could be done.

        Show
        Jason Fowler added a comment - turns out master/lib/ajax/blocks.php demonstrates most of what is needed - I will need to translate it to the none AJAX interface, if it isn't already there, but it could be done.
        Hide
        Jason Fowler added a comment -

        Moving to Sprint 30 as sprint 29 has ended

        Show
        Jason Fowler added a comment - Moving to Sprint 30 as sprint 29 has ended
        Hide
        Jason Fowler added a comment -

        Master only patch due to the huge changes in the strings needed to accomplish the task.

        Show
        Jason Fowler added a comment - Master only patch due to the huge changes in the strings needed to accomplish the task.
        Hide
        Ruslan Kabalin added a comment -

        Thanks for the patch Jason.

        [Y] Syntax
        [Y] Output
        [Y] Whitespace
        [Y] Language
        [-] Databases
        [-] Testing
        [-] Security
        [Y] Documentation
        [N] Git
        [Y] Sanity check

        All good! Just a minor thing, it is better to keep first line of commit message (commit subject line) less than 72 characters, just easier to read. Further details can go below, separated by the blank like (commit body). Can you please re-write it. Also there are two commented dev artifacts in the code, remove them unless they are absolutely necessary.

        Thanks!

        Show
        Ruslan Kabalin added a comment - Thanks for the patch Jason. [Y] Syntax [Y] Output [Y] Whitespace [Y] Language [-] Databases [-] Testing [-] Security [Y] Documentation [N] Git [Y] Sanity check All good! Just a minor thing, it is better to keep first line of commit message (commit subject line) less than 72 characters, just easier to read. Further details can go below, separated by the blank like (commit body). Can you please re-write it. Also there are two commented dev artifacts in the code, remove them unless they are absolutely necessary. Thanks!
        Hide
        Jason Fowler added a comment -

        Thanks Ruslan, changed the commit message to be less wordy, and removed the left over dev stuff. Pushing for integration now.

        Show
        Jason Fowler added a comment - Thanks Ruslan, changed the commit message to be less wordy, and removed the left over dev stuff. Pushing for integration now.
        Hide
        Damyon Wiese added a comment -

        Hi Jason,

        This needs testing instructions if it's going to be integrated...

        Show
        Damyon Wiese added a comment - Hi Jason, This needs testing instructions if it's going to be integrated...
        Hide
        Damyon Wiese added a comment -

        Jason - can you provide the testing instructions and resubmit this for next week?

        Thanks!

        Show
        Damyon Wiese added a comment - Jason - can you provide the testing instructions and resubmit this for next week? Thanks!
        Hide
        CiBoT added a comment -

        Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

        Show
        CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
        Hide
        Damyon Wiese added a comment -

        Hi Jason,

        First - apologies for not looking closer at this issue last week to give you some earlier feedback (my fault was trying to get through too many issues).

        Minor issues:
        Need to remove the text param from the phpdoc for the block_move_target constructor in lib/outputcomponents.php

        More important:
        You have not documented the new parameters to block_move_target (and it is not clear what they are for without knowing this issue).

        You should not build strings by concatination if possible - you should have separate strings for moveblockafterblock and moveblockbeforeblock.

        You have changed an API - needs comments in upgrade.txt (function "block_move_target" changed in renderer).

        Most important:
        You have broken mymobile! (Because you changed the API above)

        Thanks - these should all be easy fixes.

        Show
        Damyon Wiese added a comment - Hi Jason, First - apologies for not looking closer at this issue last week to give you some earlier feedback (my fault was trying to get through too many issues). Minor issues: Need to remove the text param from the phpdoc for the block_move_target constructor in lib/outputcomponents.php More important: You have not documented the new parameters to block_move_target (and it is not clear what they are for without knowing this issue). You should not build strings by concatination if possible - you should have separate strings for moveblockafterblock and moveblockbeforeblock. You have changed an API - needs comments in upgrade.txt (function "block_move_target" changed in renderer). Most important: You have broken mymobile! (Because you changed the API above) Thanks - these should all be easy fixes.
        Hide
        Jason Fowler added a comment -

        Thanks for the feedback Damyon. Will endeavour to get them sorted ASAP.

        Show
        Jason Fowler added a comment - Thanks for the feedback Damyon. Will endeavour to get them sorted ASAP.
        Hide
        Jason Fowler added a comment -

        All fixed now Damyon. please continue with the integration of this issue.

        Show
        Jason Fowler added a comment - All fixed now Damyon. please continue with the integration of this issue.
        Hide
        Damyon Wiese added a comment -

        Thanks Jason,

        I've integrated this for master only now.

        I had to add a commit to fix whitespace, indenting and make the update.txt comment more verbose.

        Show
        Damyon Wiese added a comment - Thanks Jason, I've integrated this for master only now. I had to add a commit to fix whitespace, indenting and make the update.txt comment more verbose.
        Hide
        Adrian Greeve added a comment -

        Tested on the master integration branch.
        No problems found.
        Test passed.

        Show
        Adrian Greeve added a comment - Tested on the master integration branch. No problems found. Test passed.
        Hide
        Damyon Wiese added a comment -

        Thanks for your hard work. This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

        Show
        Damyon Wiese added a comment - Thanks for your hard work. This issue has been integrated upstream and is now available via git (and in some hours, via mirrors and downloads).

          People

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

            Dates

            • Created:
              Updated:
              Resolved: