#109 - Example for easier settings inheritance (only an excerpt)

[danowar2k]

• Example for a Settings class that allows inheritance by changing only a few things (of course it can be improved)
• use those Settings in the settings.php

Basically, one could add every setting to the class and abstract away any theme references. With a little more work (and probably a little less "static") one could extend that Settings class by probably only changing the theme name, if they want.

[danowar2k]

Yeah, I really hate Moodle coding conventions (all lower case variable names etc.)

[abias]

Ah, I missed reading issue #109 which describes the background of this PR. Sorry for that. I understand the purpose of this PR now, however I am still unsure if we should include this change or now. I am looking forward for others' opinions.

In the meantime, I would like to mention that, if the goal of your child theme is to use the settings which are set in the Boost Campus parent theme, you might want to look at https://github.com/moodleuulm/moodle-theme_boost_campus_child where we have built a theme which comes without the need to duplicate all of Boost Campus' settings code.

Cheers, Alex

[danowar2k]

Ah, I missed reading issue #109 which describes the background of this PR. Sorry for that. I understand the purpose of this PR now, however I am still unsure if we should include this change or now. I am looking forward for others' opinions.

In the meantime, I would like to mention that, if the goal of your child theme is to use the settings which are set in the Boost Campus parent theme, you might want to look at https://github.com/moodleuulm/moodle-theme_boost_campus_child where we have built a theme which comes without the need to duplicate all of Boost Campus' settings code.

Cheers, Alex

The method of Boost_Campus_Child (BCC) is that most/all theme settings for BCC are set in the theme BC, right? So to configure the child theme I'd have to configure the parent theme.

I don't know if that's how its normally done, it justs looks odd to me to not have all configuration of a theme in the settings "folder" of the theme itself. Maybe if the settings were shown in the child as well but had a visual indicator that those settings are really the parent's settings.

The point of my PR was that every child theme has its own complete settings set. And the whole lib stuff would be built around the fact that one could just reuse methods of BC without rewriting those because the lib method would just call the settings values for the "current theme" and not just BC values.

Also, it looks a lot more readable in my opinion. Here, let me show an example: We have two themes, BCCBase and BCCCustom. The inheritance goes: BC -> BCCBase -> BCCCustom.

In BCCBase we define some settings functions and in settings.php we just write:

if ($ADMIN->fulltree) {

    // Create settings page with tabs.
    $settings = new theme_boost_admin_settingspage_tabs(
        'themesettingboost_campus_child_base',
        get_string('configtitle', 'theme_boost_campus_child_base', null, true)
    );

    // Add general tab to settings page.
    $settings->add(BoostCampusChildBaseSettings::generalTab());

    // Create advanced settings tab.
    $settings->add(BoostCampusChildBaseSettings::advancedTab());

    // Create course layout tab
    $settings->add(BoostCampusChildBaseSettings::courseLayoutTab());

    // Create footer layout tab
    $settings->add(BoostCampusChildBaseSettings::footerLayoutTab());

    // Create additional layout tab
    $settings->add(BoostCampusChildBaseSettings::additionalLayoutTab());

    // Create design settings tab.
    $settings->add(BoostCampusChildBaseSettings::designTab());

    // Create our settings tab.
    $settings->add(BoostCampusChildBaseSettings::ourTab());

The settings.php of the BCCCustom theme looks the same apart from e.g.

$settings->add(BoostCampusChildCustomSettings::ourTab());
// just add another setting to the tab
$ourTab->add(BoostCampusChildCustomSettings::getHighlightCustomizingSetting());

And a small subclass:

class BoostCampusChildCustomSettings extends BoostCampusChildBaseSettings {

    const THEME_NAME = "boost_campus_child_custom";

    const SETTING_HIGHLIGHT_CUSTOMIZING = "highlightCustomizing";

    const SETTING_HIGHLIGHT_CUSTOMIZING_DEFAULT = false;

    public static function getHighlightCustomizingSetting() {
        $name = static::getSettingName(self::SETTING_HIGHLIGHT_CUSTOMIZING);
        $title = BoostCampusChildCustomL10n::getPluginString(
            BoostCampusChildCustomL10n::SETTING_HIGHLIGHT_CUSTOMIZING_LABEL, null, true
        );
        $description = BoostCampusChildCustomL10n::getPluginString(
            BoostCampusChildCustomL10n::SETTING_HIGHLIGHT_CUSTOMIZING_DESC, null, true
        );
        $setting = new admin_setting_configcheckbox(
            $name, $title, $description, self::SETTING_HIGHLIGHT_CUSTOMIZING_DEFAULT
        );
        return $setting;
    }

    public static function getHighlightCustomizingSettingValue() {
        return static::getSettingValue(self::SETTING_HIGHLIGHT_CUSTOMIZING)
            ?: self::SETTING_HIGHLIGHT_CUSTOMIZING_DEFAULT;
    }

    protected static function getSettingValue(string $settingName) {
        return get_config(BoostCampusChildCustomConstants::getPluginName(), $settingName);
    }

    protected static function getSettingName(string $setting) {
        return BoostCampusChildCustomConstants::getPluginName()."/".$setting;
    }
}

Of course, this could possibly be further condensed. The point of the PR (or a more substantive PR like it) therefore is to help reduce code duplication, enhance readability (I hope) and enable easier reuse, rewriting and expanding of parent theme settings.

(Of course, it would be even better if this was already done in Boost)

[danowar2k]

Oh, just if that's unclear, this PR is not to be merged like it is, just to check if such a change would even be considered, before I do more work on it.

[abias]

Hi @danowar2k ,

many many thanks for working on this PR which was not merged to Boost Campus, unfortunately.

As you now and as we have documented on https://moodle.org/plugins/theme_boost_campus, Boost Campus will not be continued after Moodle 3.x anymore as we will focus on the successor Boost Union which is published on https://moodle.org/plugins/theme_boost_union. For the time being, as long as Moodle 3.x still receives security support, we will only fix critical bugs in Boost Campus if one arises.

Against this background, it does not make real sense to keep this PR open anymore. If you think the spirit of this PR should be ported over to Boost Union, please do not hesitate to create an issue on https://github.com/moodle-an-hochschulen/moodle-theme_boost_union/issues with a draft specification of what you would like to implement to have it discussed.

Cheers, Alex