From a06b87e4714864ce1184d8baf3d7a19930c0f69c Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 12 Nov 2024 11:50:52 +0000 Subject: [PATCH] MDL-75610 output: Fix title display logic in activity header The display logic for including the title in the activity header was such that the title would only display if both the theme default and the layout option for 'notitle' were undefined or false. It was not possible to a theme to have 'notitle' default to true, but have a layout override that as false to display the title. This change re-writes the is_title_allowed method to encapsulte the new logic, first checking if the current layout has the option set and using that, and if not falling back to the theme default if that is set. If neither is set, the title is displayed. This also tweaks moodle_page::magic_get_layout_options to ensure the theme is initialised before trying to return the layout options. --- lib/classes/output/activity_header.php | 17 ++++++-- lib/pagelib.php | 2 +- lib/tests/output/activity_header_test.php | 51 +++++++++++++++++++++++ 3 files changed, 66 insertions(+), 4 deletions(-) diff --git a/lib/classes/output/activity_header.php b/lib/classes/output/activity_header.php index 01bfd47093192..a8dbb017d7c11 100644 --- a/lib/classes/output/activity_header.php +++ b/lib/classes/output/activity_header.php @@ -56,11 +56,10 @@ class activity_header implements renderable, templatable { public function __construct(moodle_page $page, \stdClass $user) { $this->page = $page; $this->user = $user; - $pageoptions = $this->page->theme->activityheaderconfig ?? []; $layoutoptions = $this->page->layout_options['activityheader'] ?? []; // Do a basic setup for the header based on theme/page options. if ($page->activityrecord) { - if (empty($pageoptions['notitle']) && empty($layoutoptions['notitle'])) { + if ($this->is_title_allowed()) { $this->title = format_string($page->activityrecord->name); } @@ -79,10 +78,22 @@ public function __construct(moodle_page $page, \stdClass $user) { /** * Checks if the theme has specified titles to be displayed. * + * First checks if the current layout has the notitle option set. If it is, uses that option to decide whether the title is + * displayed. If not, then checks whether the theme has the notitle option set and uses that. If neither is set, the title + * is allowed by default. + * * @return bool */ public function is_title_allowed(): bool { - return empty($this->page->theme->activityheaderconfig['notitle']); + $layoutoptions = $this->page->layout_options['activityheader'] ?? []; + $themeoptions = $this->page->theme->activityheaderconfig; + if (isset($layoutoptions['notitle'])) { + return !$layoutoptions['notitle']; + } else if (isset($themeoptions['notitle'])) { + return !$themeoptions['notitle']; + } else { + return true; + } } /** diff --git a/lib/pagelib.php b/lib/pagelib.php index 5b32c4d13e790..2f584a98889d3 100644 --- a/lib/pagelib.php +++ b/lib/pagelib.php @@ -617,7 +617,7 @@ protected function magic_get_pagelayout() { */ protected function magic_get_layout_options() { if (!is_array($this->_layout_options)) { - $this->_layout_options = $this->_theme->pagelayout_options($this->pagelayout); + $this->_layout_options = $this->theme->pagelayout_options($this->pagelayout); } return $this->_layout_options; } diff --git a/lib/tests/output/activity_header_test.php b/lib/tests/output/activity_header_test.php index aa9717c217645..3d43480719980 100644 --- a/lib/tests/output/activity_header_test.php +++ b/lib/tests/output/activity_header_test.php @@ -156,4 +156,55 @@ public function test_get_heading_level(bool $allowtitle, string $title, int $exp $activityheaderstub->set_title($title); $this->assertEquals($expectedheadinglevel, $activityheaderstub->get_heading_level()); } + + /** + * Tests that is_title_allowed returns correctly based on the theme default and current layout options. + * + * The current layout has precedence, if the notitle option is set, otherwise the theme default is used if set. + * + * @param array $themeoptions The activityheader options array set in the theme. + * @param array $layoutoptions The activitityheader options array set in the layout. + * @param bool $allowed The expected return value of is_title_allowed. + * @covers ::is_title_allowed + * @dataProvider get_title_options + * @return void + */ + public function test_is_title_allowed(array $themeoptions, array $layoutoptions, bool $allowed): void { + $themeconfig = $this->getMockBuilder(\theme_config::class) + ->disableOriginalConstructor() + ->getMock(); + $themeconfig->activityheaderconfig = $themeoptions; + $page = $this->getMockBuilder(\moodle_page::class) + ->getMock(); + $page->expects($this->any())->method('__get')->willReturnCallback(fn($name) => match(true) { + $name == 'layout_options' => ['activityheader' => $layoutoptions], + $name == 'theme' => $themeconfig, + default => null + }); + $user = new \stdClass(); + + $activityheader = new activity_header($page, $user); + $this->assertEquals($allowed, $activityheader->is_title_allowed()); + } + + /** + * Return scenarios for test_is_title_allowed. + * + * Test each combination of the 'notitle' option being unset, true and false in each of the theme and layout options. + * + * @return array[] + */ + public static function get_title_options(): array { + return [ + 'Undefined in theme, undefined in layout' => [[], [], true], + 'Undefined in theme, disallowed in layout' => [[], ['notitle' => true], false], + 'Undefined in theme, allowed in layout' => [[], ['notitle' => false], true], + 'Disallowed in theme, undefined in layout' => [['notitle' => true], [], false], + 'Disallowed in theme, disallowed in layout' => [['notitle' => true], ['notitle' => true], false], + 'Disallowed in theme, allowed in layout' => [['notitle' => true], ['notitle' => false], true], + 'Allowed in theme, undefined in layout' => [['notitle' => false], [], true], + 'Allowed in theme, disallowed in layout' => [['notitle' => false], ['notitle' => true], false], + 'Allowed in theme, allowed in layout' => [['notitle' => false], ['notitle' => false], true], + ]; + } }