Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-42969

Main Menu Block loses ability to move resource or activity up or down

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.6
    • Fix Version/s: 2.6.1
    • Component/s: Blocks
    • Testing Instructions:
      Hide

      Check that within the main menu (only available on front page), you can move the activities around. It should work like in 2.5 (i.e. the front page does not support draganddrop for activities - you should see the non js move icon)

      Show
      Check that within the main menu (only available on front page), you can move the activities around. It should work like in 2.5 (i.e. the front page does not support draganddrop for activities - you should see the non js move icon)
    • Affected Branches:
      MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE
    • Pull Master Branch:
      MDL-42969-master

      Description

      1. Create a resource or activity from the Main Menu Block
      2. Turn on editing mode
      3. The only choice to move the resource is to the right (right arrow). There is no option to move the resource or activity up or down within the block. Clicking on the right arrow does nothing.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              jerome Jérôme Mouneyrac added a comment -

              Sent to peer-review.

              Show
              jerome Jérôme Mouneyrac added a comment - Sent to peer-review.
              Hide
              rwijaya Rossiani Wijaya added a comment -

              Hi Jerome,

              The patch does fix the issue however should it be fixed within the course_get_cm_edit_actions() instead of in block_site_main_menu.php file?

              I also notice the editing option (eg: edit, move, hide, etc) is not available at all if javascript is disabled for the browser.

              [y] Syntax
              [y] Whitespace
              [y] Output
              [-] Language
              [-] Databases
              [y] Testing (instructions and automated tests)
              [-] Security
              [-] Documentation
              [y] Git
              [-] Third party code
              [y] Sanity check

              Show
              rwijaya Rossiani Wijaya added a comment - Hi Jerome, The patch does fix the issue however should it be fixed within the course_get_cm_edit_actions() instead of in block_site_main_menu.php file? I also notice the editing option (eg: edit, move, hide, etc) is not available at all if javascript is disabled for the browser. [y] Syntax [y] Whitespace [y] Output [-] Language [-] Databases [y] Testing (instructions and automated tests) [-] Security [-] Documentation [y] Git [-] Third party code [y] Sanity check
              Hide
              jerome Jérôme Mouneyrac added a comment -

              Thanks Rosie. Just to explain, if you look at course_section_cm() you can see that the dnd icon has been removed from course_get_cm_edit_actions(). You can check the code history, it's actually quite recent. I did try to call course_get_cm_move() but it behaves differently on this specific block following the theme you use (without mentioning that dnd is not support on other front page activities).

              The trick I've done is that I remove the class that the dnd JS is looking for. The patch:

              • matches the existing code logic (the move icon is added outside the function that return the edit actions)
              • is consistent with the front page behavior (no dnd on the front page activities)
              • behaves exactly like in 2.5
                I think it's the best solution to this regression at the current time, before some good review of the dnd JS is done.
              Show
              jerome Jérôme Mouneyrac added a comment - Thanks Rosie. Just to explain, if you look at course_section_cm() you can see that the dnd icon has been removed from course_get_cm_edit_actions(). You can check the code history, it's actually quite recent. I did try to call course_get_cm_move() but it behaves differently on this specific block following the theme you use (without mentioning that dnd is not support on other front page activities). The trick I've done is that I remove the class that the dnd JS is looking for. The patch: matches the existing code logic (the move icon is added outside the function that return the edit actions) is consistent with the front page behavior (no dnd on the front page activities) behaves exactly like in 2.5 I think it's the best solution to this regression at the current time, before some good review of the dnd JS is done.
              Hide
              poltawski Dan Poltawski added a comment -

              I assume this needs to be cherry-picked to 26?

              Show
              poltawski Dan Poltawski added a comment - I assume this needs to be cherry-picked to 26?
              Hide
              jerome Jérôme Mouneyrac added a comment -

              Yes Dan. I read on the chat last week that regression issues were automatically cherry-picked so we didn't need to make a 2.6 version.

              Show
              jerome Jérôme Mouneyrac added a comment - Yes Dan. I read on the chat last week that regression issues were automatically cherry-picked so we didn't need to make a 2.6 version.
              Hide
              poltawski Dan Poltawski added a comment -

              Nope. There is no automatic cherry-picking, please indicate your intentions

              (And starting this week master and 26 will diverge so cherry-picks are not always clean).

              Show
              poltawski Dan Poltawski added a comment - Nope. There is no automatic cherry-picking, please indicate your intentions (And starting this week master and 26 will diverge so cherry-picks are not always clean).
              Hide
              poltawski Dan Poltawski added a comment -

              Integrated to master and 2.6, thanks Jerome

              Show
              poltawski Dan Poltawski added a comment - Integrated to master and 2.6, thanks Jerome
              Hide
              jerome Jérôme Mouneyrac added a comment -

              no problem, thanks Dan.

              Show
              jerome Jérôme Mouneyrac added a comment - no problem, thanks Dan.
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Works as described however I found and reported MDL-43184 while testing this.

              cheers

              Show
              ankit_frenz Ankit Agarwal added a comment - Works as described however I found and reported MDL-43184 while testing this. cheers
              Hide
              poltawski Dan Poltawski added a comment -

              Thanks for your contributions, this change is now upstream!

              “ If debugging is the process of removing software bugs, then programming must be the process of putting them in. ” - Edsger Dijkstra

              Show
              poltawski Dan Poltawski added a comment - Thanks for your contributions, this change is now upstream! “ If debugging is the process of removing software bugs, then programming must be the process of putting them in. ” - Edsger Dijkstra

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    13/Jan/14