Skip to content

Commit

Permalink
Review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
abias committed Jan 31, 2023
1 parent 18ae157 commit 9215d03
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 50 deletions.
2 changes: 1 addition & 1 deletion CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ Changes

### Unreleased

* 2023-01-30 - Feature: Allow the admin to set CSS rules for the Moodle Mobile App, solves #195.
* 2023-01-28 - Improvement: Do not resize SVG logo files during serving, helps to solve #160.
* 2023-01-26 - Feature: Add dedicated logo settings to Boost Union, solves #211.
* 2023-01-22 - Feature: Allow the admin to change the H5P content bank width, solves #201.

### v4.0-r11

* 2023-01-24 - Feature: Add Setting (textfield) to upload css rules for the layout in the mobile App, solves #195.
* 2023-01-21 - Improvement: Add note about grandchild themes to the README file, solves #122.
* 2023-01-16 - Improvement: Remove Boost Union's own fallback CSS file for now, relates to #89.

Expand Down
8 changes: 5 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,13 @@ With this setting you can upload custom fonts to the theme. The advantage of upl

Moodle core ships with FontAwesome 4 icons which are fine, but FontAwesome has evolved since then. If you want to use more recent FontAwesome icons, you can do this with this setting. As soon as you choose another version than FontAwesome 4, additional settings will appear where you can upload more recent FontAwesome versions.

#### Tab "Mobile"
#### Tab "Mobile app"

##### Supplementary Mobile Settings
##### Mobile appearance

With this setting you can upload css rules for the mobile layout. This setting overwrites the filepath in the setting mobilecssurl.
###### Additional CSS for Mobile app

With this setting, you can write custom CSS code to customise your mobile app interface. The CSS code will be only added to the Mobile app depiction of this Moodle instance and will not be shown in the webbrowser version.

### Settings page "Feel"

Expand Down
17 changes: 10 additions & 7 deletions lang/en/theme_boost_union.php
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,16 @@
$string['fontawesomecheck-fa6free-newstuff-description'] = 'Newer FontAwesome versions ship with additional icons compared to the FontAwesome 4 iconset. If you see a virus icon on the left hand side, your FontAwesome 6 version is properly showing new icons which are new in FontAwesome 6.';
$string['fontawesomecheck-fa6free-filter-title'] = 'FontAwesome filter';
$string['fontawesomecheck-fa6free-filter-description'] = 'As you have the FontAwesome filter plugin installed, you should be sure that the filter handles the new FontAwesome 6 icons correctly as well. If you see a users icon on the left hand side, the filter is working properly with the FontAwesome 6 version icons.';
// Settings: Mobile tab.
$string['mobiletab'] = 'Mobile';
// ... Section: Additional mobile css.
$string['mobilecssheading'] = 'Supplementary Mobile Settings';
$string['mobilecss'] = 'Additional CSS';
$string['mobilecss_desc'] = 'CSS-Rules will be only applied in the depiction of the Moodle instance in the Moodle App. This setting overwrites the setting mobilecssurl in Mobile App -> Mobile Appearance.';
$string['mobilecss_overwrite'] = 'Currently the setting is set to the file {$a} and the setting will be overwritten.';
// Settings: Mobile app tab.
$string['mobiletab'] = 'Mobile app';
// ... Section: Mobile appearance.
$string['mobileappearanceheading'] = 'Mobile appearance';
// ... ... Setting: Additional CSS for Mobile app.
$string['mobilecss'] = 'Additional CSS for Mobile app';
$string['mobilecss_desc'] = 'With this setting, you can write custom CSS code to customise your mobile app interface. The CSS code will be only added to the Mobile app depiction of this Moodle instance and will not be shown in the webbrowser version. Read more about this feature in the <a href="https://moodledev.io/general/app/customisation/remote-themes#how-do-remote-themes-work">Moodle dev docs</a>.';
$string['mobilecss_set'] = 'As soon as you add any CSS code to this setting and save the setting, the <a href="{$a->url}">Moodle core setting <em>mobilecssurl</em></a> will be automatically set to a URL of the Boost Union theme.';
$string['mobilecss_overwrite'] = 'As soon as you add any CSS code to this setting and save the setting, the <a href="{$a->url}">Moodle core setting <em>mobilecssurl</em></a> will be automatically overwritten with a URL of the Boost Union theme. Currently this setting is set to <a href="{$a->value}">{$a->value}</a>.';
$string['mobilecss_donotchange'] = 'This step is necessary to ship the CSS code to the Mobile app. Do not change the URL there unless you really want to remove the CSS code from the Mobile app again.';

// Settings: Feel page.
$string['configtitlefeel'] = 'Feel';
Expand Down
23 changes: 15 additions & 8 deletions locallib.php
Original file line number Diff line number Diff line change
Expand Up @@ -1287,19 +1287,26 @@ function theme_boost_union_get_course_header_image_url() {
}

/**
* Helper function which adds the CSS file from the Look->mobile setting to the Moodle page.
* It's meant to be called when changing the setting only.
* Helper function which sets the URL to the CSS file as soon as the theme's mobilescss setting has any CSS code.
* It's meant to be called as callback when changing the admin setting only.
* *
* @throws coding_exception
* @throws dml_exception
* @throws moodle_exception
*/
function theme_boost_union_add_mobile_css_url() {
global $CFG;
function theme_boost_union_set_mobilecss_url() {
// Check if the admin has set any CSS code for the Mobile app.
$csscode = get_config('theme_boost_union', 'mobilescss');
if (!empty($csscode)) {
// Build the Mobile app CSS file URL.
$mobilescssurl = new moodle_url('/theme/boost_union/mobile/styles.php');

// Build the flavour CSS file URL.
$mobilecssurl = new moodle_url('/theme/boost_union/mobile/styles.php');
// Set the $CFG->mobilecssurl setting.
set_config('mobilecssurl', $mobilescssurl->out());

// Not sure why, but $CFG->mobilecssurl is not accepted.
set_config('mobilecssurl', $mobilecssurl->out());
// Otherwise.
} else {
// Clear the $CFG->mobilecssurl setting.
set_config('mobilecssurl', '');
}
}
26 changes: 17 additions & 9 deletions mobile/styles.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* Theme Boost Union - Mobile styles serving. Logic copied from flavours/style.php.
* Theme Boost Union - Mobile App styles serving.
*
* @package theme_boost_union
* @copyright 2023 Nina Herrmann <[email protected]>
Expand All @@ -40,18 +40,26 @@
require_once($CFG->dirroot.'/lib/csslib.php');
require_once($CFG->dirroot.'/lib/configonlylib.php');

// Initialize CSS code.
$css = '';
// Initialize SCSS code.
$scss = '';

// Get the css fro the setting.
// Get the raw SCSS from the admin setting,
// throw an exception if get_config throws an exception which happens only if something is really wrong.
try {
$configmobilecss = get_config('theme_boost_union', 'mobilecss');
// Note: In the current state of implementation, this setting only allows the usage of custom CSS, not SCSS.
// There is a follow-up issue on Github to add SCSS support.
// However, to ease this future improvement, the setting has already been called 'mobilescss'.
$configmobilescss = get_config('theme_boost_union', 'mobilescss');

// Catch the exception.
} catch (\Exception $e) {
// Should not happen but in case...
// Just die, there is no use to output any error message, it would even be counter-productive if the browser
// tries to interpret it as CSS code.
die;
}

// Always add the css-code in case it is empty - maybe it is supposed to be deleted.
$css .= $configmobilecss;
// Always add the CSS code even if it is empty.
$scss .= $configmobilescss;

// Send out the resulting CSS code. The theme revision will be set as etag to support the browser caching.
css_send_cached_css_content($css, theme_get_revision());
css_send_cached_css_content($scss, theme_get_revision());
48 changes: 35 additions & 13 deletions settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -825,35 +825,57 @@
$setting = new admin_setting_description($name, $title, $description);
$tab->add($setting);
}

// Add tab to settings page.
$page->add($tab);

// Create mobile settings tab.

// Create mobile app tab.
$tab = new admin_settingpage('theme_boost_union_look_mobile',
get_string('mobiletab', 'theme_boost_union', null, true));
get_string('mobiletab', 'theme_boost_union', null, true));

// Create mobilecss heading.
$name = 'theme_boost_union/mobilecssheading';
$title = get_string('mobilecssheading', 'theme_boost_union', null, true);
// Create Mobile appearance heading.
$name = 'theme_boost_union/mobileappearanceheading';
$title = get_string('mobileappearanceheading', 'theme_boost_union', null, true);
$setting = new admin_setting_heading($name, $title, null);
$tab->add($setting);

// Create a textfield for Raw CSS for mobile.
$name = 'theme_boost_union/mobilecss';
// Setting: Additional CSS for Mobile app.
$name = 'theme_boost_union/mobilescss';
$title = get_string('mobilecss', 'theme_boost_union', null, true);
$description = get_string('mobilecss_desc', 'theme_boost_union', null, true);
// In case another URL is set (in the mobilecssurl setting), we add a hint to the description.
if (isset($CFG->mobilecssurl) && strpos($CFG->mobilecssurl, '/boost_union/mobile/styles.php') == false
&& !empty($CFG->mobilecssurl)) {
$description .= html_writer::div(get_string('mobilecss_overwrite', 'theme_boost_union',
$CFG->mobilecssurl), 'alert alert-danger');
$mobilecssurl = new moodle_url('/admin/settings.php', array('section' => 'mobileappearance'));
// If another Mobile App CSS URL is set already (in the $CFG->mobilecssurl setting), we add a warning to the description.
if (isset($CFG->mobilecssurl) && !empty($CFG->mobilecssurl) &&
strpos($CFG->mobilecssurl, '/boost_union/mobile/styles.php') == false) {
$mobilescssnotification = new \core\output\notification(
get_string('mobilecss_overwrite', 'theme_boost_union',
array('url' => $mobilecssurl->out(), 'value' => $CFG->mobilecssurl)).' '.
get_string('mobilecss_donotchange', 'theme_boost_union'),
\core\output\notification::NOTIFY_WARNING);
$mobilescssnotification->set_show_closebutton(false);
$description .= $OUTPUT->render($mobilescssnotification);

// Otherwise, we just add a note to the description.
} else {
$mobilescssnotification = new \core\output\notification(
get_string('mobilecss_set', 'theme_boost_union',
array('url' => $mobilecssurl->out())).' '.
get_string('mobilecss_donotchange', 'theme_boost_union'),
\core\output\notification::NOTIFY_INFO);
$mobilescssnotification->set_show_closebutton(false);
$description .= $OUTPUT->render($mobilescssnotification);
}
// Using admin_setting_scsscode is not 100% right here as this setting does not support SCSS.
// However, is shouldn't harm if the CSS code is parsed by the setting.
$setting = new admin_setting_scsscode($name, $title, $description, '', PARAM_RAW);
$setting->set_updatedcallback('theme_boost_union_add_mobile_css_url');
$setting->set_updatedcallback('theme_boost_union_set_mobilecss_url');
$tab->add($setting);

// Add tab to settings page.
$page->add($tab);


// Add settings page to the admin settings category.
$ADMIN->add('theme_boost_union', $page);

Expand Down
54 changes: 46 additions & 8 deletions tests/behat/theme_boost_union_looksettings_mobile.feature
Original file line number Diff line number Diff line change
@@ -1,20 +1,58 @@
@theme @theme_boost_union @theme_boost_union_looksettings @theme_boost_union_looksettings_mobile
Feature: Configuring the theme_boost_union plugin for the "mobile" tab on the "Look" page
Feature: Configuring the theme_boost_union plugin for the "Mobile app" tab on the "Look" page
In order to use the features
As admin
I need to be able to configure the theme Boost Union plugin

@javascript
Scenario: mobilecss: - Insert content in editor for additional css which is only displayed in the App.
Background:
Given the following config values are set as admin:
| config | value |
| enablemobilewebservice | yes |
| config | value |
| enablemobilewebservice | yes |

Scenario: Setting: Additional CSS for Mobile app - Insert CSS code and test that the mobilecssurl URL is set correctly.
When I log in as "admin"
And I navigate to "Appearance > Boost Union > Look" in site administration
And I click on "Mobile app" "link" in the "#adminsettings .nav-tabs" "css_element"
And I set the field "Additional CSS for Mobile app" to multiline:
"""
a.test { font-size: 16px; }
"""
And I press "Save changes"
And I navigate to "General > Mobile app > Mobile appearance" in site administration
Then "//div[@id='admin-mobilecssurl']//input[contains(@value, 'theme/boost_union/mobile/styles.php')]" "xpath_element" should exist

Scenario: Setting: Additional CSS for Mobile app - Insert CSS code and test that the mobilecssurl URL is overwritten correctly.
Given the following config values are set as admin:
| config | value |
| mobilecssurl | https://mymoodle/mycss.css |
When I log in as "admin"
And I navigate to "Appearance > Boost Union > Look" in site administration
And I click on "Mobile" "link" in the "#adminsettings .nav-tabs" "css_element"
And I set the field "Additional CSS" to ".atest {font-size: 16px;}"
And I click on "Mobile app" "link" in the "#adminsettings .nav-tabs" "css_element"
And I set the field "Additional CSS for Mobile app" to multiline:
"""
a.test { font-size: 16px; }
"""
And I press "Save changes"
And I navigate to "General > Mobile app > Mobile appearance" in site administration
Then "//div[@id='admin-mobilecssurl']//input[contains(@value, 'theme/boost_union/mobile/styles.php')]" "xpath_element" should exist
And I should not see "mycss.css" in the "#id_s__mobilecssurl" "css_element"

Scenario: Setting: Additional CSS for Mobile app - Remove CSS code and test that the mobilecssurl URL is cleared correctly.
Given the following config values are set as admin:
| config | value |
| mobilecssurl | https://mymoodle/mycss.css |
And the following config values are set as admin:
| config | value | plugin |
| mobilescss | a.test { font-size: 16px; } | theme_boost_union |
When I log in as "admin"
And I navigate to "Appearance > Boost Union > Look" in site administration
And I click on "Mobile app" "link" in the "#adminsettings .nav-tabs" "css_element"
And I set the field "Additional CSS for Mobile app" to multiline:
"""
"""
And I press "Save changes"
And I navigate to "General > Mobile app > Mobile appearance" in site administration
Then "//div[@id='admin-mobilecssurl']//input[contains(@value, 'theme/boost_union/mobile/styles.php')]" "xpath_element" should not exist

# Manual Testing, is css really used in App.
# Unfortunately, this can't be tested with Behat yet as Mobile App testing is not added to this plugin yet.
# Scenario: Setting: Additional CSS for Mobile app - Verify that the CSS code has an effect in the Mobile app.
2 changes: 1 addition & 1 deletion version.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
defined('MOODLE_INTERNAL') || die();

$plugin->component = 'theme_boost_union';
$plugin->version = 2022080922;
$plugin->version = 2022080923;
$plugin->release = 'v4.0-r11';
$plugin->requires = 2022041900;
$plugin->supported = [400, 400];
Expand Down

0 comments on commit 9215d03

Please sign in to comment.