diff --git a/theme/boost/classes/boostnavbar.php b/theme/boost/classes/boostnavbar.php index 21be26cd8f7..42bc50243eb 100644 --- a/theme/boost/classes/boostnavbar.php +++ b/theme/boost/classes/boostnavbar.php @@ -55,6 +55,9 @@ class boostnavbar implements \renderable { protected function prepare_nodes_for_boost(): void { global $PAGE; + // Defines whether section items with an action should be removed by default. + $removesections = true; + if ($this->page->context->contextlevel == CONTEXT_COURSE) { // Remove any duplicate navbar nodes. $this->remove_duplicate_items(); @@ -90,6 +93,14 @@ class boostnavbar implements \renderable { // Remove 'My courses' if we are in the module context. if ($this->page->context->contextlevel == CONTEXT_MODULE) { $this->remove('mycourses'); + $this->remove('courses'); + // Remove the course category breadcrumb node. + $this->remove($this->page->course->category); + $courseformat = course_get_format($this->page->course)->get_course(); + // Section items can be only removed if a course layout (coursedisplay) is not explicitly set in the + // given course format or the set course layout is not 'One section per page'. + $removesections = !isset($courseformat->coursedisplay) || + $courseformat->coursedisplay != COURSE_DISPLAY_MULTIPAGE; } if (!is_null($this->get_item('root'))) { // We are in site administration. @@ -107,7 +118,7 @@ class boostnavbar implements \renderable { $mycoursesnode->text = get_string('mycourses'); } - $this->remove_no_link_items(); + $this->remove_no_link_items($removesections); // Don't display the navbar if there is only one item. Apparently this is bad UX design. if ($this->item_count() <= 1) { @@ -212,13 +223,16 @@ class boostnavbar implements \renderable { } /** - * Remove items that are categories or have no actions associated with them. + * Remove items that have no actions associated with them and optionally remove items that are sections. * * The only exception is the last item in the list which may not have a link but needs to be displayed. + * + * @param bool $removesections Whether section items should be also removed (only applies when they have an action) */ - protected function remove_no_link_items(): void { + protected function remove_no_link_items(bool $removesections = true): void { foreach ($this->items as $key => $value) { - if (!$value->is_last() && (!$value->has_action() || $value->type == \navigation_node::TYPE_SECTION)) { + if (!$value->is_last() && + (!$value->has_action() || ($value->type == \navigation_node::TYPE_SECTION && $removesections))) { unset($this->items[$key]); } } diff --git a/theme/boost/tests/boostnavbar_test.php b/theme/boost/tests/boostnavbar_test.php index 3d1ecaa9847..86e756b54db 100644 --- a/theme/boost/tests/boostnavbar_test.php +++ b/theme/boost/tests/boostnavbar_test.php @@ -33,12 +33,13 @@ class boostnavbar_test extends \advanced_testcase { */ public function remove_no_link_items_provider(): array { return [ - 'All nodes have links links including leaf node' => [ + 'All nodes have links links including leaf node. Set to remove section nodes.' => [ [ - 'node1' => true, - 'node2' => true, - 'node3' => true, + 'node1' => ['hasaction' => true, 'issection' => false], + 'node2' => ['hasaction' => true, 'issection' => false], + 'node3' => ['hasaction' => true, 'issection' => false], ], + true, [ 'Home' => true, 'Courses' => true, @@ -48,12 +49,13 @@ class boostnavbar_test extends \advanced_testcase { 'node3' => true, ] ], - 'Only some parent nodes have links. Leaf node has a link.' => [ + 'Only some parent nodes have links. Leaf node has a link. Set to remove section nodes.' => [ [ - 'node1' => false, - 'node2' => true, - 'node3' => true, + 'node1' => ['hasaction' => false, 'issection' => false], + 'node2' => ['hasaction' => true, 'issection' => false], + 'node3' => ['hasaction' => true, 'issection' => false], ], + true, [ 'Home' => true, 'Courses' => true, @@ -62,12 +64,13 @@ class boostnavbar_test extends \advanced_testcase { 'node3' => true, ] ], - 'All parent nodes do not have links. Leaf node has a link.' => [ + 'All parent nodes do not have links. Leaf node has a link. Set to remove section nodes.' => [ [ - 'node1' => false, - 'node2' => false, - 'node3' => true, + 'node1' => ['hasaction' => false, 'issection' => false], + 'node2' => ['hasaction' => false, 'issection' => false], + 'node3' => ['hasaction' => true, 'issection' => false], ], + true, [ 'Home' => true, 'Courses' => true, @@ -75,12 +78,13 @@ class boostnavbar_test extends \advanced_testcase { 'node3' => true, ] ], - 'All parent nodes have links. Leaf node does not has a link.' => [ + 'All parent nodes have links. Leaf node does not has a link. Set to remove section nodes.' => [ [ - 'node1' => true, - 'node2' => true, - 'node3' => false, + 'node1' => ['hasaction' => true, 'issection' => false], + 'node2' => ['hasaction' => true, 'issection' => false], + 'node3' => ['hasaction' => false, 'issection' => false], ], + true, [ 'Home' => true, 'Courses' => true, @@ -90,12 +94,13 @@ class boostnavbar_test extends \advanced_testcase { 'node3' => false, ] ], - 'All parent nodes do not have links. Leaf node does not has a link.' => [ + 'All parent nodes do not have links. Leaf node does not has a link. Set to remove section nodes.' => [ [ - 'node1' => false, - 'node2' => false, - 'node3' => false, + 'node1' => ['hasaction' => false, 'issection' => false], + 'node2' => ['hasaction' => false, 'issection' => false], + 'node3' => ['hasaction' => false, 'issection' => false], ], + true, [ 'Home' => true, 'Courses' => true, @@ -103,12 +108,13 @@ class boostnavbar_test extends \advanced_testcase { 'node3' => false, ] ], - 'Some parent nodes do not have links. Leaf node does not has a link.' => [ + 'Some parent nodes do not have links. Leaf node does not has a link. Set to remove section nodes.' => [ [ - 'node1' => true, - 'node2' => false, - 'node3' => false, + 'node1' => ['hasaction' => true, 'issection' => false], + 'node2' => ['hasaction' => false, 'issection' => false], + 'node3' => ['hasaction' => false, 'issection' => false], ], + true, [ 'Home' => true, 'Courses' => true, @@ -116,6 +122,57 @@ class boostnavbar_test extends \advanced_testcase { 'node1' => true, 'node3' => false, ] + ], + 'All nodes have links links including leaf node and section nodes. Set to remove section nodes.' => [ + [ + 'node1' => ['hasaction' => true, 'issection' => false], + 'node2' => ['hasaction' => true, 'issection' => false], + 'sectionnode1' => ['hasaction' => true, 'issection' => true], + 'node3' => ['hasaction' => true, 'issection' => false], + ], + true, + [ + 'Home' => true, + 'Courses' => true, + 'tc_1' => true, + 'node1' => true, + 'node2' => true, + 'node3' => true, + ] + ], + 'All nodes have links links including leaf node and section nodes. Set to not remove section nodes.' => [ + [ + 'node1' => ['hasaction' => true, 'issection' => false], + 'node2' => ['hasaction' => true, 'issection' => false], + 'sectionnode1' => ['hasaction' => true, 'issection' => true], + 'node3' => ['hasaction' => true, 'issection' => false], + ], + false, + [ + 'Home' => true, + 'Courses' => true, + 'tc_1' => true, + 'node1' => true, + 'node2' => true, + 'sectionnode1' => true, + 'node3' => true, + ] + ], + 'Only some parent nodes have links. Section node does not have a link. Set to not remove section nodes.' => [ + [ + 'node1' => ['hasaction' => false, 'issection' => false], + 'node2' => ['hasaction' => true, 'issection' => false], + 'sectionnode1' => ['hasaction' => false, 'issection' => true], + 'node3' => ['hasaction' => true, 'issection' => false], + ], + true, + [ + 'Home' => true, + 'Courses' => true, + 'tc_1' => true, + 'node2' => true, + 'node3' => true, + ] ] ]; } @@ -124,10 +181,11 @@ class boostnavbar_test extends \advanced_testcase { * * @dataProvider remove_no_link_items_provider * @param array $setup + * @param bool $removesectionnodes Whether to remove the section nodes with an associated action. * @param array $expected * @throws \ReflectionException */ - public function test_remove_no_link_items(array $setup, array $expected) { + public function test_remove_no_link_items(array $setup, bool $removesectionnodes, array $expected) { global $PAGE; $this->resetAfterTest(); @@ -139,8 +197,9 @@ class boostnavbar_test extends \advanced_testcase { $page->set_url(new \moodle_url('/course/view.php', array('id' => $course->id))); // A dummy url to use. We don't care where it's pointing to. $url = new \moodle_url('/'); - foreach ($setup as $node => $hasaction) { - $page->navbar->add($node, $hasaction ? $url : null); + foreach ($setup as $key => $value) { + $page->navbar->add($key, $value['hasaction'] ? $url : null, + $value['issection'] ? \navigation_node::TYPE_SECTION : null); } $boostnavbar = $this->getMockBuilder(boostnavbar::class) @@ -156,7 +215,7 @@ class boostnavbar_test extends \advanced_testcase { // Make the call to the function. $rcm = $rc->getMethod('remove_no_link_items'); $rcm->setAccessible(true); - $rcm->invoke($boostnavbar); + $rcm->invoke($boostnavbar, $removesectionnodes); // Get the value for the class variable that the function modifies. $values = $rcp->getValue($boostnavbar);