Details

    • Testing Instructions:
      Hide
      1. Inspect the code of page using blocks, and locate the <a> tags with class 'skip-block' right before each block
      2. Dock some blocks and make sure the <a> tag gets the style property display: none
      3. Undock some blocks but one and make sure the <a> tag is set to be displayed
      4. Refresh the page
      5. Make sure the remaining docked block associated <a> tag is set to be hidden
      6. Undock the block and make sure the <a> tag is set to be visible
      Show
      Inspect the code of page using blocks, and locate the <a> tags with class 'skip-block' right before each block Dock some blocks and make sure the <a> tag gets the style property display: none Undock some blocks but one and make sure the <a> tag is set to be displayed Refresh the page Make sure the remaining docked block associated <a> tag is set to be hidden Undock the block and make sure the <a> tag is set to be visible
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30829-master

      Description

      When a block becomes docked, its skip link is still present in the main page. The link still functions, but adds unnecessary confusion because it implies the block is still there.

      Potential Solution: Remove the skip links

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Nathan Lind added a comment -

            Would this task also address the fact that screen readers don't have a way to read the names and functionality (links) of docked blocks?
            Thanks!
            Nathan

            Show
            Nathan Lind added a comment - Would this task also address the fact that screen readers don't have a way to read the names and functionality (links) of docked blocks? Thanks! Nathan
            Hide
            Frédéric Massart added a comment -

            Assuming that a block can only been docked using JavaScript I also assumed that setting a display to none would be evaluated by the browser used. I also checked for the class of the node looked up to assert that we are hiding the right one.

            Show
            Frédéric Massart added a comment - Assuming that a block can only been docked using JavaScript I also assumed that setting a display to none would be evaluated by the browser used. I also checked for the class of the node looked up to assert that we are hiding the right one.
            Hide
            Rajesh Taneja added a comment -

            Hello Fred,

            Patch looks good, although you might want to consider having a second thought about how this will work without JS. Currently docked blocks are hidden with JS disabled (Not correct, but that's the behaviour), so having this solution will not remove skip links for hidden blocks.

            You might want to check $bc (block_content) in lib/outputrenderers.php - function block where this is added and check for user preference and block state.

            ($bc->collapsible === block_contents::HIDDEN) && 
            (get_user_preferences('docked_block_instance_'.$bc->blockinstanceid, 0))
            

            Show
            Rajesh Taneja added a comment - Hello Fred, Patch looks good, although you might want to consider having a second thought about how this will work without JS. Currently docked blocks are hidden with JS disabled (Not correct, but that's the behaviour), so having this solution will not remove skip links for hidden blocks. You might want to check $bc (block_content) in lib/outputrenderers.php - function block where this is added and check for user preference and block state. ($bc->collapsible === block_contents::HIDDEN) && (get_user_preferences('docked_block_instance_'.$bc->blockinstanceid, 0))
            Hide
            Frédéric Massart added a comment -

            Thanks for your review Raj. I understand what you are saying here, but as the Javascript itself is responsible of docking/undocking, it make more sense to me to handle it on the client side only. Regarding the collapsible thing, I think we should keep it regardless of this attribute. Any other block with an empty content, and therefore pretty much the same height than a collapsed block, will keep the skip link.

            I am pushing this for integration. Cheers!

            Show
            Frédéric Massart added a comment - Thanks for your review Raj. I understand what you are saying here, but as the Javascript itself is responsible of docking/undocking, it make more sense to me to handle it on the client side only. Regarding the collapsible thing, I think we should keep it regardless of this attribute. Any other block with an empty content, and therefore pretty much the same height than a collapsed block, will keep the skip link. I am pushing this for integration. Cheers!
            Hide
            Rajesh Taneja added a comment - - edited

            hmm, just wondering if we add skip links within block then that will be cached by dock script and we don't have to add/remove them.
            FYI:
            https://github.com/rajeshtaneja/moodle/compare/master...wip-mdl-30829

            Show
            Rajesh Taneja added a comment - - edited hmm, just wondering if we add skip links within block then that will be cached by dock script and we don't have to add/remove them. FYI: https://github.com/rajeshtaneja/moodle/compare/master...wip-mdl-30829
            Hide
            Frédéric Massart added a comment - - edited

            Thanks Raj, your solution is from far more elegant than mine. Pushing for integration using your patch. Cheers!

            Edit: Well, at the end I am keeping my patch as the anchor destination should be moved inside the div as well, and then the anchor would not be precisely at the end of the block, especially when the theme uses borders/margins/paddings. I also noted that the drag and drop uses the same logic to locate and move the anchors.

            Show
            Frédéric Massart added a comment - - edited Thanks Raj, your solution is from far more elegant than mine. Pushing for integration using your patch. Cheers! Edit: Well, at the end I am keeping my patch as the anchor destination should be moved inside the div as well, and then the anchor would not be precisely at the end of the block, especially when the theme uses borders/margins/paddings. I also noted that the drag and drop uses the same logic to locate and move the anchors.
            Hide
            Sam Hemelryk added a comment -

            Thanks Fred, this has been integrated now

            Show
            Sam Hemelryk added a comment - Thanks Fred, this has been integrated now
            Hide
            Adrian Greeve added a comment -

            Tested on master and 2.2, 2.3, and master integration branches.
            The style property changes to display none and back again when docking / undocking.
            Test passed.

            Show
            Adrian Greeve added a comment - Tested on master and 2.2, 2.3, and master integration branches. The style property changes to display none and back again when docking / undocking. Test passed.
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio

            This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

            Thanks!

            Show
            Eloy Lafuente (stronk7) added a comment - Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: