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
    • Rank:
      33829

      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

        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: