Skip to content

Commit

Permalink
MDL-75610 output: Fix title display logic in activity header
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
marxjohnson committed Nov 13, 2024
1 parent cb13bba commit a06b87e
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 4 deletions.
17 changes: 14 additions & 3 deletions lib/classes/output/activity_header.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand All @@ -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;
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion lib/pagelib.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
51 changes: 51 additions & 0 deletions lib/tests/output/activity_header_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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],
];
}
}

0 comments on commit a06b87e

Please sign in to comment.