From dd25891bda5c4c51579503385e0f9be52347319d Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 12 Nov 2024 11:50:52 +0000 Subject: [PATCH 1/2] 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 | 53 +++++++++++++++++++++++ 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/lib/classes/output/activity_header.php b/lib/classes/output/activity_header.php index 01bfd470931..a8dbb017d7c 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 @@ class activity_header implements renderable, templatable { /** * 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 5b32c4d13e7..2f584a98889 100644 --- a/lib/pagelib.php +++ b/lib/pagelib.php @@ -617,7 +617,7 @@ class moodle_page { */ 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 aa9717c2176..1092e3bf86c 100644 --- a/lib/tests/output/activity_header_test.php +++ b/lib/tests/output/activity_header_test.php @@ -156,4 +156,57 @@ class activity_header_test extends \advanced_testcase { $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(); + // Mocking the magic_get_layout_options() and magic_get_theme() methods directly doesn't work, + // so mock the whole magic __get() method and just return the test values for those properties. + $page->expects($this->any())->method('__get')->willReturnCallback(fn($name) => match($name) { + 'layout_options' => ['activityheader' => $layoutoptions], + '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], + ]; + } } From 9118a619c6cad559677c72a2ffd641c7e80d4038 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Tue, 12 Nov 2024 11:51:21 +0000 Subject: [PATCH 2/2] MDL-75610 theme: Allow the activity title in Boost theme's secure layout --- .upgradenotes/MDL-75610-2024111313224921.yml | 19 +++++++++++++++++++ .../secure_layout_activity_heading.feature | 1 + theme/boost/config.php | 7 ++++++- 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 .upgradenotes/MDL-75610-2024111313224921.yml diff --git a/.upgradenotes/MDL-75610-2024111313224921.yml b/.upgradenotes/MDL-75610-2024111313224921.yml new file mode 100644 index 00000000000..99ce5698948 --- /dev/null +++ b/.upgradenotes/MDL-75610-2024111313224921.yml @@ -0,0 +1,19 @@ +issueNumber: MDL-75610 +notes: + core: + - message: > + `\core\output\activity_header` now uses the `is_title_allowed()` method + when setting the title in the constructor. + + + This method has been improved to give priority to the 'notitle' option + in the theme config for the current page layout, over the top-level + option in the theme. + + + For example, the Boost theme sets + `$THEME->activityheaderconfig['notitle'] = true;` by default, but in its + `secure` pagelayout, it has `'notitle' = false`. + + This prevents display of the title in all layouts except `secure`. + type: improved diff --git a/mod/quiz/accessrule/seb/tests/behat/secure_layout_activity_heading.feature b/mod/quiz/accessrule/seb/tests/behat/secure_layout_activity_heading.feature index 9c1f1b57e7e..c4e881d1c18 100644 --- a/mod/quiz/accessrule/seb/tests/behat/secure_layout_activity_heading.feature +++ b/mod/quiz/accessrule/seb/tests/behat/secure_layout_activity_heading.feature @@ -35,4 +35,5 @@ Feature: View the activity header when Safe Exam Browser is required Scenario: Quiz description is displayed when Safe Exam Browser is required When I am on the "Test quiz name" "quiz activity" page logged in as student1 Then I should see "Launch Safe Exam Browser" + And "Test quiz name" "heading" should exist And I should see "Test quiz description" diff --git a/theme/boost/config.php b/theme/boost/config.php index 3ee7f1d1f3d..4b1451d0c00 100644 --- a/theme/boost/config.php +++ b/theme/boost/config.php @@ -164,7 +164,12 @@ $THEME->layouts = [ 'secure' => array( 'file' => 'secure.php', 'regions' => array('side-pre'), - 'defaultregion' => 'side-pre' + 'defaultregion' => 'side-pre', + 'options' => [ + 'activityheader' => [ + 'notitle' => false, + ], + ], ) ];