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

Navigation block should not show elements which are both empty and have no link

          Details

          • Testing Instructions:
            Hide
            • Open the site frontpage (or most other pages)
              • Confirm that the 'Site pages' link is visible and has children
            • Turn editing on
            • Edit the block settings for the navigation block
            • Change the "Generate navigation for the following" setting to "Categories and courses" and Save changes
              • Confirm that the 'Site pages' link is no longer visible
            Show
            Open the site frontpage (or most other pages) Confirm that the 'Site pages' link is visible and has children Turn editing on Edit the block settings for the navigation block Change the "Generate navigation for the following" setting to "Categories and courses" and Save changes Confirm that the 'Site pages' link is no longer visible
          • Affected Branches:
            MOODLE_22_STABLE
          • Fixed Branches:
            MOODLE_22_STABLE, MOODLE_23_STABLE
          • Pull from Repository:
          • Pull Master Branch:
            MDL-34322-master-1
          • Pull Master Diff URL:

            Description

            It's currently possible an element in the navigation block to have no link, and have children but not display them.
            The result is a node in the navigation tree which has no link and cannot be expanded rendering it a digital paper-weight on our screens.

            In this scenario, we should not display the link as it's potentially confusing - it doesn't do anything.

            To replicate:

            • Open the site frontpage (or most other pages)
            • Turn editing on
            • Edit the block settings for the navigation block
            • Change the "Generate navigation for the following" setting to "Categories and courses" and Save changes
            • Notice that the "Site pages" link can't be expanded any more and don't link to anything

            Proposal: Not show the bleeding thing

              Gliffy Diagrams

                Attachments

                  Activity

                  Ascending order - Click to sort in descending order
                  Hide
                  Permalink
                  dobedobedoh Andrew Nicols added a comment -

                  This patch skips generation of all of the content early on to benefit performance too. There may be another case I haven't considered but I can't think of them right now.

                  This patch should not remove:

                  • JS-only links using the action_link class to provide an action
                  • any action with a string or moodle_url
                  • any node which is expandable
                  Show
                  dobedobedoh Andrew Nicols added a comment - This patch skips generation of all of the content early on to benefit performance too. There may be another case I haven't considered but I can't think of them right now. This patch should not remove: JS-only links using the action_link class to provide an action any action with a string or moodle_url any node which is expandable
                  Hide
                  Permalink
                  salvetore Michael de Raadt added a comment -

                  Thanks for working on that, Andrew.

                  I'll see if we can get that peer reviewed for you soon.

                  Show
                  salvetore Michael de Raadt added a comment - Thanks for working on that, Andrew. I'll see if we can get that peer reviewed for you soon.
                  Hide
                  Permalink
                  samhemelryk Sam Hemelryk added a comment -

                  Looks spot on Andrew, feel free to put up for integration review when you're ready

                  Show
                  samhemelryk Sam Hemelryk added a comment - Looks spot on Andrew, feel free to put up for integration review when you're ready
                  Hide
                  Permalink
                  poltawski Dan Poltawski 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
                  poltawski Dan Poltawski 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
                  Permalink
                  samhemelryk Sam Hemelryk added a comment -

                  Thanks Andrew, I've integrated this now.

                  Show
                  samhemelryk Sam Hemelryk added a comment - Thanks Andrew, I've integrated this now.
                  Hide
                  Permalink
                  andyjdavis Andrew Davis added a comment -

                  Looks good. Passing.

                  Show
                  andyjdavis Andrew Davis added a comment - Looks good. Passing.
                  Hide
                  Permalink
                  nebgor Aparup Banerjee added a comment -

                  yay, it works!

                  This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week.

                  Thank you all for taking the time to get us here.

                  cheers!

                  Show
                  nebgor Aparup Banerjee added a comment - yay, it works! This issue has been put through rigorous processes and finally swam upstream along with some 65 others this week. Thank you all for taking the time to get us here. cheers!

                    People

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

                      Dates

                      • Created:
                        Updated:
                        Resolved:
                        Fix Release Date:
                        10/Sep/12