L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-73645 theme_boost: Keep section breadcumb item in specific layouts

Browse Source 
Avoids removing the section breadcrumb nodes in the course module
context when the course uses the 'One section per page' layout.
Additionaly, it also removes the Courses and Category breadcrumb
nodes in the module contexts to be consistent with the way the
breadcrumbs are displayed in the course context.
This commit is contained in:
Mihail Geshoski
2022-03-02 13:01:03 +08:00
parent 3e24811c00
commit 2a09a3ff76
2 changed files with 105 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files
theme/boost/classes/boostnavbar.php
+18 -4
View File
@@ -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]);
}
}
theme/boost/tests/boostnavbar_test.php
+87 -28
View File
@@ -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);