moodle-an-hochschulen / moodle-local_boostnavigation

Moodle plugin which tries to overcome some fixed appearance behaviours of Boost's nav drawer in a clean way
GNU General Public License v3.0
39 stars 28 forks source link

Navigation region requires a label error #40

Closed slewislcm closed 8 months ago

slewislcm commented 5 years ago

Hello, I've installed moodle 3.7+ this week, and when using the boost navigation 3.7 I get a load of errors. When installed it gives no errors, but if I make any changes at all, even 1 tick box to hide dashboard, it gives an error for every menu item. I've tested on a clean moodle install with boost. Also with the boost navigation plugin v3.6 & 3.7.

Navigation region requires a label line 3853 of \lib\navigationlib.php: call to debugging() line 4055 of \lib\navigationlib.php: call to flat_navigation_node->get_collectionlabel() line 443 of \lib\navigationlib.php: call to flat_navigation->add() line 446 of \lib\navigationlib.php: call to navigation_node->build_flat_navigation_list() line 446 of \lib\navigationlib.php: call to navigation_node->build_flat_navigation_list() line 4004 of \lib\navigationlib.php: call to navigation_node->build_flat_navigation_list() line 771 of \lib\pagelib.php: call to flat_navigation->initialise() line 820 of \lib\pagelib.php: call to moodle_page->magic_get_flatnav() line 54 of \theme\boost\layout\columns2.php: call to moodle_page->__get() line 1370 of \lib\outputrenderers.php: call to include() line 1300 of \lib\outputrenderers.php: call to core_renderer->render_page_layout() line 121 of \admin\settings.php: call to core_renderer->header()

Any ideas? thanks,, Simon

audetcameron commented 5 years ago

This Function in lib/navigation.php

    /**
     * Getter, get the label for this flat_navigation node, or it's parent if it doesn't have one.
     *
     * @return string
     */
    public function get_collectionlabel() {
        if (!empty($this->collectionlabel)) {
            return $this->collectionlabel;
        }
        if ($this->parent && ($this->parent instanceof flat_navigation_node || $this->parent instanceof flat_navigation)) {
            return $this->parent->get_collectionlabel();
        }
        debugging('Navigation region requires a label', DEBUG_DEVELOPER);
        return '';
    }

looks like collectionlabel needs to be added to the navigation. Where can we add this in this plugin?

abias commented 5 years ago

Hi @slewislcm and @audetcameron,

I was able to have a look at this problem now.

Since Moodle 3.7, Moodle core adds an aria-label to each nav node collection - which means to the root node collection which is lead by the Dashboard node, the course node collection which is lead by the course name node and the site settings node collection (if you are an admin). It gets this string from the collection's first node's collectionlabel value with the help of the get_collectionlabel() getter function. If we remove the first node from a node collection and the other nodes which Moodle is trying to ask for a collectionlabel with get_collectionlabel(), a debugging warning in thrown.

In the context of this plugin, this only happens for the root node collection as soon as you remove the Dashboard node, but not for the course node collection (as this plugin doesn't have a functionality to remove the course name node) and also not for the bottom nodes added by this plugin (as these aren't a real node collection of its own).

Getting this debugging message with debugging turned on is confusing and annoying, but it is non-breaking. For end-users, it will only affect the aria-label of the root node collection which unfortunately will have the aria-label value "Site settings" as soon as the Dashboard node is removed.

Even more unfortunately, I can't fix this behaviour within the scope of this plugin. The solution would simply be to add a proper collectionlabel to the next remaining node which is asked by Moodle navigation when building the navigation output. But the function set_collectionlabel() is only implemented for flat_navigation_node objects and for navigation_node_collection objects which are both not accessible by this plugin.

So, my only conclusion is plain: "Don't remove the Dashboard node or deal with the debugging messages and the misleading aria-label of the remaining root node collection".

I can add this note to the "local_boostnavigation | removemyhomenode" setting, but I will leave this issue open for some more time in case anybody comes up with a real solution.

Thanks, Alex

danowar2k commented 2 years ago

I looked into this and came to the same result. The only ways this could be adressed I see is:

Is there a Moodle tracker issue for this? Would this even make sense because a 3rd party plugin changes the intended core behaviour?

neeesn commented 2 years ago

This problem is still in 3.11

Navigation region requires a label line 3907 of /lib/navigationlib.php: call to debugging() line 4108 of /lib/navigationlib.php: call to flat_navigation_node->get_collectionlabel() line 443 of /lib/navigationlib.php: call to flat_navigation->add() line 446 of /lib/navigationlib.php: call to navigation_node->build_flat_navigation_list() line 4059 of /lib/navigationlib.php: call to navigation_node->build_flat_navigation_list() line 781 of /lib/pagelib.php: call to flat_navigation->initialise() line 830 of /lib/pagelib.php: call to moodle_page->magic_get_flatnav() line 110 of /theme/myTheme/layout/columns2.php: call to moodle_page->__get() line 1400 of /lib/outputrenderers.php: call to include() line 1330 of /lib/outputrenderers.php: call to core_renderer->render_page_layout() line 121 of /admin/settings.php: call to core_renderer->header()

abias commented 8 months ago

This plugin has finally reached end of life now and will not be upgraded to Moodle 4.x.

I would like to invite you to have a look at our Moodle 4 theme theme_boost_union which supports several aspects of this plugin's feature set and is actively maintained and extended. Thank you for your understanding.

Against this background, I will close this issue.