Review changes

CHANGES.md

@@ -6,7 +6,7 @@ Changes
 
 ### Unreleased
 
-* 2024-10-25 - Improvement: Limit the max-width of the navbar logo if it is too broad or has a special aspect ratio, resolves #544.
+* 2024-10-25 - Improvement: Add setting to limit the maximum width of the navbar logo if it is too broad or has a special aspect ratio, resolves #544.
 * 2024-10-24 - Release: Change support thread URL in README to a tiny URL.
 * 2024-10-24 - Tests: Try to fix Behat error 'Warning: Undefined array key 1' on Moodle 4.5, resolves #734.
 

README.md

@@ -109,10 +109,6 @@ Here, you can upload a full logo to be used as decoration. This image is especia
 
 Here, you can upload a compact version of the same logo as above, such as an emblem, shield or icon. This image is especially used in the navigation bar at the top of each Moodle page. The image should be clear even at small sizes.
 
-###### Logo Maxwidth
-
-If the logo for the navbar on the top left is too wide or has a special aspect ratio, you can limit the logo's maximum width. Use css definition to limit the max-width.
-
 ##### Favicon
 
 ###### Favicon
@@ -142,6 +138,12 @@ With these settings, you can overwrite the Bootstrap colors which are used withi
 
 ##### Navbar
 
+###### Maximal width of logo in navbar
+
+If the logo for the navbar on the top left is too wide or has a special aspect ratio, you can limit the logo's maximum width. Use css definition to limit the max-width.
+
+###### Navbar color
+
 With this setting, you can change the navbar color from the default light navbar to a dark one or a colored one.
 
 #### Tab "Activity Branding"

lang/en/theme_boost_union.php

@@ -170,9 +170,9 @@
 $string['bootstrapcolordangersetting_desc'] = 'The Bootstrap color for "Danger"';
 // ... Section: Navbar.
 $string['navbarheading'] = 'Navbar';
-// ... Section: Navbar logo max width.
-$string['maxlogowidth'] = 'Maximal width of logo';
-$string['maxlogowidth_desc'] = 'If the logo is too broad or has a special aspect ratio, you can set the maximal width of the logo in the navbar header. Use CSS notation. Possible values can have some digits and end with `px`, `%`, `vw` or the field can be left empty, if you do not want to use this setting.';
+// ... Section: Maximal width of logo in navbar.
+$string['maxlogowidth'] = 'Maximal width of logo in navbar';
+$string['maxlogowidth_desc'] = 'In the navbar, the uploaded compact logo is normally displayed with 100% height and proportional width. However, if the logo is too broad or has another special aspect ratio, you can set the maximal width of the logo in the navbar here. You can enter pixel-based values like 120px, but you can also enter a percentage-based value like 10% or a viewport-width value like 5vw. If you do not enter any value, the logo will be displayed with the default presentation.';
 // ... ... Setting: Navbar color.
 $string['navbarcolorsetting'] = 'Navbar color';
 $string['navbarcolorsetting_desc'] = 'With this setting, you can change the navbar color from the default light navbar to a dark one or a colored one.';

lib.php

@@ -365,7 +365,7 @@ function theme_boost_union_get_extra_scss($theme) {
     // Setting: Activity icon purpose.
     $content .= theme_boost_union_get_scss_for_activity_icon_purpose($theme);
 
-    // Setting: Navbar and icon styles.
+    // Setting: Navbar styles.
     $content .= theme_boost_union_get_scss_navbar($theme);
 
     // Setting: Mark external links.

locallib.php

@@ -1671,8 +1671,7 @@ function theme_boost_union_get_scss_courseoverview_block($theme) {
 
 
 /**
- * Returns the SCSS code to be used in the navbar. The first usage is a logo icon that is possibly too broad and needs
- * reduced to fit the navbar. See theme_boost_union_maxlogowidth for further details
+ * Returns the SCSS code to be used in the navbar.
  *
  * @param theme_config $theme The theme config object.
  * @return string
@@ -1681,9 +1680,12 @@ function theme_boost_union_get_scss_navbar($theme) {
     // Initialize SCSS snippet.
     $scss = '';
 
-    // Set variables which are read in settings by the logo maxwidth values.
-    if (get_config('theme_boost_union', 'maxlogowidth')) {
-        $scss .= ".navbar-brand img.logo{max-width:".get_config('theme_boost_union', 'maxlogowidth').";height:auto;}\n";
+    // Set styles bases on the maxlogowidth setting.
+    if (!empty(get_config('theme_boost_union', 'maxlogowidth'))) {
+        $scss .= '.navbar-brand, .navbar-brand .logo {
+                max-width: '.get_config('theme_boost_union', 'maxlogowidth').';
+                height: auto;
+        }'.PHP_EOL;
     }
 
     return $scss;

scss/boost_union/post.scss

@@ -7,7 +7,8 @@
  --------------------------------------*/
 
 /* Add an explicit height to the logo in the navbar.
-   This is necessary to render SVG logos properly as their size is calculated different than pixel-based images. */
+   This is necessary to render SVG logos properly as their size is calculated different than pixel-based images.
+   Please note that these values might be overridden by the maxlogowidth setting. */
 .navbar-brand,
 .navbar-brand .logo {
     height: 100%;

settings.php

@@ -128,7 +128,7 @@
 
         // Prepare regular expression for checking if the value is a percent number (from 0% to 100%) or a pixel number
         // (with 2 or 3 digits) or a viewport width number (from 0 to 100). Additionally the field can be left blank.
-        $smallsizeoremptyregex = '/^((\d{1,2}|100)%)|((\d{1,2}|100)vw)|(\d{2,3}px)|(^(?!.*\S))$/';
+        $smallwidthoremptyregex = '/^((\d{1,2}|100)%)|((\d{1,2}|100)vw)|(\d{2,3}px)|(^(?!.*\S))$/';
 
         // Create Look settings page with tabs
         // (and allow users with the theme/boost_union:configure capability to access it).
@@ -400,15 +400,6 @@
         $setting->set_updatedcallback('theme_reset_all_caches');
         $tab->add($setting);
 
-        // Setting: add extra SCSS if logo icon is too broad / wrong aspect ratio.
-        $name = 'theme_boost_union/maxlogowidth';
-        $title = get_string('maxlogowidth', 'theme_boost_union', null, true);
-        $description = get_string('maxlogowidth_desc', 'theme_boost_union', null, true);
-        $default = '';
-        $setting = new admin_setting_configtext($name, $title, $description, $default, $smallsizeoremptyregex, 6);
-        $setting->set_updatedcallback('theme_reset_all_caches');
-        $tab->add($setting);
-
         // Create favicon heading.
         $name = 'theme_boost_union/faviconheading';
         $title = get_string('faviconheading', 'theme_boost_union', null, true);
@@ -530,6 +521,15 @@
         $setting = new admin_setting_heading($name, $title, null);
         $tab->add($setting);
 
+        // Setting: Maximal width of logo in navbar.
+        $name = 'theme_boost_union/maxlogowidth';
+        $title = get_string('maxlogowidth', 'theme_boost_union', null, true);
+        $description = get_string('maxlogowidth_desc', 'theme_boost_union', null, true);
+        $default = '';
+        $setting = new admin_setting_configtext($name, $title, $description, $default, $smallwidthoremptyregex, 6);
+        $setting->set_updatedcallback('theme_reset_all_caches');
+        $tab->add($setting);
+
         // Setting: Navbar color.
         $name = 'theme_boost_union/navbarcolor';
         $title = get_string('navbarcolorsetting', 'theme_boost_union', null, true);

tests/behat/theme_boost_union_looksettings_sitebranding.feature

@@ -117,40 +117,6 @@ Feature: Configuring the theme_boost_union plugin for the "Site branding" tab on
       | max-width  | 10vw      |
       | max-width  | 13%       |
 
-  @javascript
-  Scenario: Setting: Logo max-width - limit logo width readout from theme_config entry (countercheck)
-    When I log in as "admin"
-    And I navigate to "Appearance > Boost Union > Look" in site administration
-    And I click on "Site branding" "link" in the "#adminsettings .nav-tabs" "css_element"
-    And I set the field "Maximal width of logo" to ""
-    And I press "Save changes"
-    And I am on site homepage
-    And the theme cache is purged and the theme is reloaded
-    Then DOM element ".navbar-brand .logo" should have computed style "height" "100%"
-
-  @javascript
-  Scenario: Setting: Logo max-width - limit logo through admin settings - check regex to limit entry (countercheck)
-    When I log in as "admin"
-    And I navigate to "Appearance > Boost Union > Look" in site administration
-    And I click on "Site branding" "link" in the "#adminsettings .nav-tabs" "css_element"
-    And I set the field "Maximal width of logo" to "2px"
-    And I press "Save changes"
-    Then I should not see "Changes saved"
-    And I should see "Some settings were not changed due to an error."
-    And I should see "This value is not valid"
-
-  @javascript @_file_upload
-  Scenario: Setting: Compact logo - Upload a PNG compact logo to the theme and check that it is resized
-    When I log in as "admin"
-    And Behat debugging is disabled
-    And I navigate to "Appearance > Boost Union > Look" in site administration
-    And I click on "Site branding" "link" in the "#adminsettings .nav-tabs" "css_element"
-    And I upload "theme/boost_union/tests/fixtures/moodlelogo.png" file to "Compact logo" filemanager
-    And I press "Save changes"
-    And Behat debugging is enabled
-    And I am on site homepage
-    Then "//nav[contains(@class, 'navbar')]//img[contains(@class, 'logo')][contains(@src, 'pluginfile.php/1/theme_boost_union/logocompact/300x300/')]" "xpath_element" should exist
-
   @javascript @_file_upload
   Scenario: Setting: Compact logo - Upload a SVG compact logo to the theme and check that it is not resized
     When I log in as "admin"
@@ -247,6 +213,28 @@ Feature: Configuring the theme_boost_union plugin for the "Site branding" tab on
       | warning | #0000FF  | rgb(0, 0, 255)   |
       | danger  | #FFFF00  | rgb(255, 255, 0) |
 
+  @javascript @_file_upload
+  Scenario Outline: Setting: Maximal width of logo in navbar - Set the maximum width
+    Given the following config values are set as admin:
+      | config       | value     | plugin            |
+      | maxlogowidth | <setting> | theme_boost_union |
+    And the theme cache is purged and the theme is reloaded
+    When I log in as "admin"
+    And Behat debugging is disabled
+    And I navigate to "Appearance > Boost Union > Look" in site administration
+    And I click on "Site branding" "link" in the "#adminsettings .nav-tabs" "css_element"
+    And I upload "theme/boost_union/tests/fixtures/moodlelogo.png" file to "Compact logo" filemanager
+    And I press "Save changes"
+    And Behat debugging is enabled
+    And I am on site homepage
+    Then DOM element ".navbar-brand" <shouldornot> have computed style "max-width" "<value>"
+    And DOM element ".navbar-brand .logo" <shouldornot> have computed style "max-width" "<value>"
+
+    Examples:
+      | setting | shouldornot | value |
+      | 50px    | should      | 50px  |
+      |         | should not  | 50px  |
+
   Scenario Outline: Setting: Navbar color - Set the navbar color
     Given the following config values are set as admin:
       | config      | value     | plugin            |

version.php

@@ -25,7 +25,7 @@
 defined('MOODLE_INTERNAL') || die();
 
 $plugin->component = 'theme_boost_union';
-$plugin->version = 2024100700;
+$plugin->version = 2024100701;
 $plugin->release = 'v4.5-r1';
 $plugin->requires = 2024100700;
 $plugin->supported = [405, 405];