Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.3
    • Fix Version/s: 2.1.1
    • Component/s: Accessibility
    • Labels:
      None
    • Testing Instructions:
      Hide

      Using only the keyboard (Tab, Return, etc):
      1. Tab to the Collapse/Expand button next to a block. Verify that the keyboard focus is displayed on the button.
      2. Use the keyboard (presumably Return) to collapse/expand the block. Verify that it collapses/expands correctly.

      Show
      Using only the keyboard (Tab, Return, etc): 1. Tab to the Collapse/Expand button next to a block. Verify that the keyboard focus is displayed on the button. 2. Use the keyboard (presumably Return) to collapse/expand the block. Verify that it collapses/expands correctly.
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-27429-master
    • Rank:
      16844

      Description

      It doesn't seem to be possible to activate the Collapse/Expand buttons next to each block using the keyboard.

        Activity

        Hide
        Aparup Banerjee added a comment -

        this might be resolved with the patch in progress at: MDL-27428

        Show
        Aparup Banerjee added a comment - this might be resolved with the patch in progress at: MDL-27428
        Hide
        John Beedell added a comment -

        I have just tested and can confirm that it is not possible to get to the collapse/expand button in a block using just the keyboard. Currently a tab moves the cursor from a block's 'skip settings' to the blocks's 'Move this to the dock' button, then one more tab takes you to the first item in the block.

        Show
        John Beedell added a comment - I have just tested and can confirm that it is not possible to get to the collapse/expand button in a block using just the keyboard. Currently a tab moves the cursor from a block's 'skip settings' to the blocks's 'Move this to the dock' button, then one more tab takes you to the first item in the block.
        Hide
        John Beedell added a comment -

        Further to my last comment, using the keyboard to 'Move this block to the dock' may not be of any advantage to many disabled users. Once the block is in the dock, the next tab returns you to the top of the page, and there is then no way of accessing the dock and recovering the block just using the keyboard.

        The dock is great for sighted mouse users, but for accessibility I imagine that the collapse/expand feature of blocks would be of more use.

        Could I suggest that the tab order for a block be adjusted to moving from 'Skip settings' to 'Collapse/expand' (rather than the current 'Move this to the dock') then to the first link in the block. This tab order would help solve this bug request and prevent an accessibility nightmare with docked but inaccessible blocks.

        Show
        John Beedell added a comment - Further to my last comment, using the keyboard to 'Move this block to the dock' may not be of any advantage to many disabled users. Once the block is in the dock, the next tab returns you to the top of the page, and there is then no way of accessing the dock and recovering the block just using the keyboard. The dock is great for sighted mouse users, but for accessibility I imagine that the collapse/expand feature of blocks would be of more use. Could I suggest that the tab order for a block be adjusted to moving from 'Skip settings' to 'Collapse/expand' (rather than the current 'Move this to the dock') then to the first link in the block. This tab order would help solve this bug request and prevent an accessibility nightmare with docked but inaccessible blocks.
        Hide
        John Beedell added a comment -

        I have put a fix on https://github.com/Beedell/moodle/commits/wip-MDL-27429-master for review. I have tested this on FF, Chrome and IE7/8.

        Unfortunately the testing on IE7 and 8 showed that there is a separate issue for that browser only with the blocks/dock.js where the enter key used on the hide/show icon docks the last block on the page (with and without this fix in place).

        Do I need to be concerned that this fix is over two commits?

        Show
        John Beedell added a comment - I have put a fix on https://github.com/Beedell/moodle/commits/wip-MDL-27429-master for review. I have tested this on FF, Chrome and IE7/8. Unfortunately the testing on IE7 and 8 showed that there is a separate issue for that browser only with the blocks/dock.js where the enter key used on the hide/show icon docks the last block on the page (with and without this fix in place). Do I need to be concerned that this fix is over two commits?
        Hide
        Sam Marshall added a comment -

        Hi John, the code looks good to me, but yes it should be in a single commit (in this case - there are sometimes justifications for separate commits where each one is a standalone improvement, especially for larger changes).

        Please use 'git rebase -i' to glomp them into a single commit. (You may need to get the EDITOR environment variable right for this to work, i.e. set it to a suitable texteditor such as notepad2.) If advice online about git rebase -i is confusing, then I can help doing it...

        After that, rebase onto latest master and post the compare url here please.

        Show
        Sam Marshall added a comment - Hi John, the code looks good to me, but yes it should be in a single commit (in this case - there are sometimes justifications for separate commits where each one is a standalone improvement, especially for larger changes). Please use 'git rebase -i' to glomp them into a single commit. (You may need to get the EDITOR environment variable right for this to work, i.e. set it to a suitable texteditor such as notepad2.) If advice online about git rebase -i is confusing, then I can help doing it... After that, rebase onto latest master and post the compare url here please.
        Hide
        John Beedell added a comment -
        Show
        John Beedell added a comment - Branch updated on github. The compare url is https://github.com/Beedell/moodle/compare/master...wip-MDL-27429-master .
        Hide
        Sam Marshall added a comment -

        Sorry John, I went and spotted a minor flaw in your code now. It looks like the code (which was written in a rather confusing way in the first place) is doing this when it sets up the events:

        object.doSomething().on(...)

        This is relying on the 'chaining' behaviour of the doSomething() method (it's actually 'set' but just for example).

        So your change to do

        object.doSomething().on(...)
        object.doSomething().on(...)

        means that it is actually doing 'doSomething' twice, as well as doing the two different 'on' commands.

        I think the second (added) line should just be

        object.on()

        Do you agree? If so, please change those two lines (for show and hide) and rebase yet again (sorry). You should probably also recheck briefly just to check that doesn't break it, but I don't think it will.

        Show
        Sam Marshall added a comment - Sorry John, I went and spotted a minor flaw in your code now. It looks like the code (which was written in a rather confusing way in the first place) is doing this when it sets up the events: object.doSomething().on(...) This is relying on the 'chaining' behaviour of the doSomething() method (it's actually 'set' but just for example). So your change to do object.doSomething().on(...) object.doSomething().on(...) means that it is actually doing 'doSomething' twice, as well as doing the two different 'on' commands. I think the second (added) line should just be object.on() Do you agree? If so, please change those two lines (for show and hide) and rebase yet again (sorry). You should probably also recheck briefly just to check that doesn't break it, but I don't think it will.
        Hide
        Sam Marshall added a comment -

        To clarify, when I said 'rebase again', to make changes from this point you can just go 'git commit --amend' then push --force it again, without having to actually use the rebase command (it's effectively the same thing).

        Show
        Sam Marshall added a comment - To clarify, when I said 'rebase again', to make changes from this point you can just go 'git commit --amend' then push --force it again, without having to actually use the rebase command (it's effectively the same thing).
        Hide
        John Beedell added a comment -

        Thank you, that does make sense and is done now.

        Show
        John Beedell added a comment - Thank you, that does make sense and is done now.
        Hide
        Sam Marshall added a comment -

        John, when I look at the compare URL you gave above:
        https://github.com/Beedell/moodle/compare/master...wip-MDL-27429-master
        I still see two commits and not one. Could you rebase them together? Or am I missing something...

        Show
        Sam Marshall added a comment - John, when I look at the compare URL you gave above: https://github.com/Beedell/moodle/compare/master...wip-MDL-27429-master I still see two commits and not one. Could you rebase them together? Or am I missing something...
        Hide
        John Beedell added a comment -

        Oh no - two sillies on my part.
        1) I did not sign up as a watcher, so did not get an email about this last issue.
        2) I thought I had done this squash, but obviously had not.
        So my apologies as I had kind of put this to bed without giving it a final check.
        Anyway it is now ready for progressing now...Thank you.

        Show
        John Beedell added a comment - Oh no - two sillies on my part. 1) I did not sign up as a watcher, so did not get an email about this last issue. 2) I thought I had done this squash, but obviously had not. So my apologies as I had kind of put this to bed without giving it a final check. Anyway it is now ready for progressing now...Thank you.
        Hide
        Sam Marshall added a comment -

        Thanks John.

        Although we missed the 2.1 release, as this is an accessibility fix that our expert tester said was high priority, we think it should still be included in the 2.1 branch.

        Show
        Sam Marshall added a comment - Thanks John. Although we missed the 2.1 release, as this is an accessibility fix that our expert tester said was high priority, we think it should still be included in the 2.1 branch.
        Hide
        Sam Marshall added a comment -

        oops got branch names wrong

        Show
        Sam Marshall added a comment - oops got branch names wrong
        Hide
        Eloy Lafuente (stronk7) 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
        Eloy Lafuente (stronk7) 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
        Sam Marshall added a comment -

        Note: We had already rebased this to latest weekly (even though we did it before Eloy's comment), so it should be correct.

        Show
        Sam Marshall added a comment - Note: We had already rebased this to latest weekly (even though we did it before Eloy's comment), so it should be correct.
        Hide
        Sam Hemelryk added a comment -

        Thanks guys - this has been integrated now.

        Cheers
        Sam

        Show
        Sam Hemelryk added a comment - Thanks guys - this has been integrated now. Cheers Sam
        Hide
        Andrew Davis added a comment -

        looks good.

        Show
        Andrew Davis added a comment - looks good.
        Hide
        Sam Hemelryk added a comment -

        Congratulations - this fix has just been released in the weeklies.

        Show
        Sam Hemelryk added a comment - Congratulations - this fix has just been released in the weeklies.
        Hide
        Petr Škoda added a comment -

        fixing version and author to get proper stats

        Show
        Petr Škoda added a comment - fixing version and author to get proper stats

          People

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

            Dates

            • Created:
              Updated:
              Resolved: