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

          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