From a244a773a7d63ce395e4fc241c519b925ea42c57 Mon Sep 17 00:00:00 2001 From: Alexander Bias Date: Fri, 6 Dec 2024 11:20:19 +0100 Subject: [PATCH] Review changes --- CHANGES.md | 1 + classes/smartmenu.php | 9 ++++++- classes/smartmenu_item.php | 25 +++++++++++-------- tests/behat/behat_theme_boost_union.php | 22 ++++++++-------- ...usettings_menuitems_dynamiccourses.feature | 14 +++++------ .../behat_theme_boost_union_generator.php | 10 ++++---- tests/generator/lib.php | 10 ++++---- 7 files changed, 51 insertions(+), 40 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 9e6ca8e5d07..6bc3a321150 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -6,6 +6,7 @@ Changes ### Unreleased +* 2024-12-06 - Tests: Add several Behat optimisations to bring down the test suite run time, resolves #765. * 2024-12-06 - Upstream change: Adopt changes from MDL-83759 ('System notification navbar popover is misplaced in Moodle 4.4 and 4.5') * 2024-12-06 - Upstream change: Adopt changes from MDL-75610 ('Quiz activity name no longer being displayed in quiz landing page when using Safe Exam Browser'), resolves #766. diff --git a/classes/smartmenu.php b/classes/smartmenu.php index 2393e5e3e5d..e33264efb2b 100644 --- a/classes/smartmenu.php +++ b/classes/smartmenu.php @@ -592,11 +592,18 @@ public function build($resetcache=false) { return false; } + // Add marker class to make clear that this is a Boost Union smart menu. $this->menu->classes[] = 'boost-union-smartmenu'; + + // Add custom CSS class. + $this->menu->classes[] = $this->menu->cssclass; + + // Add CSS classes for card menus. $this->menu->classes[] = $this->get_cardform(); // Html class for the card form size, Potrait, Square, landscape. $this->menu->classes[] = $this->get_cardsize(); // HTML class for the card Size, tiny, small, medium, large. $this->menu->classes[] = $this->get_cardwrap(); // HtML class for the card overflow behaviour. - $this->menu->classes[] = $this->menu->cssclass;// Custom class selector for menu. + + // Add CSS classes for more behaviour. $this->menu->classes[] = ($this->menu->moremenubehavior == self::MOREMENU_OUTSIDE) ? "force-menu-out" : ''; // Card type menus doesn't supports inline menus. diff --git a/classes/smartmenu_item.php b/classes/smartmenu_item.php index 9427257db8d..39d50dcb82d 100644 --- a/classes/smartmenu_item.php +++ b/classes/smartmenu_item.php @@ -1042,9 +1042,12 @@ public function build() { return false; } - // Add custom css class. - $class[] = 'boost-union-smartitem'; + // Add marker class to make clear that this is a Boost Union smart menu item. + $class[] = 'boost-union-smartmenuitem'; + + // Add custom CSS class. $class[] = $this->item->cssclass; + // Add classes for hide items in specific viewport. $class[] = $this->item->desktop ? 'd-lg-none' : 'd-lg-inline-flex'; $class[] = $this->item->tablet ? 'd-md-none' : 'd-md-inline-flex'; @@ -1052,11 +1055,14 @@ public function build() { // Add classes for item title placement on card. $class[] = $this->get_textposition_class(); - // Menu item class. + + // Add menu item class. $types = [self::TYPESTATIC => 'static', self::TYPEDYNAMIC => 'dynamic', self::TYPEHEADING => 'heading']; $class[] = 'menu-item-'.($types[$this->item->type] ?? ''); + // Add classes to item data. $this->item->classes = $class; + // Load the location of menu, used to collect menus for locations in menu inline mode. $this->item->location = $this->menu->location; @@ -1330,20 +1336,17 @@ public static function get_types(?int $type = null) { } /** - * Returns the display options for the menu items. + * Return the options for the display setting. * - * @param int|null $option The display option to retrieve. If null, returns all display options. - * @return array|string The array of display options if $option is null, or the display option string if $option is set. - * @throws coding_exception if $option is set but invalid. + * @return array + * @throws \coding_exception */ - public static function get_display_options(?int $option = null) { - $displayoptions = [ + public static function get_display_options() { + return [ self::DISPLAY_SHOWTITLEICON => get_string('smartmenusmenuitemdisplayoptionsshowtitleicon', 'theme_boost_union'), self::DISPLAY_HIDETITLE => get_string('smartmenusmenuitemdisplayoptionshidetitle', 'theme_boost_union'), self::DISPLAY_HIDETITLEMOBILE => get_string('smartmenusmenuitemdisplayoptionshidetitlemobile', 'theme_boost_union'), ]; - - return ($option !== null && isset($displayoptions[$option])) ? $displayoptions[$option] : $displayoptions; } /** diff --git a/tests/behat/behat_theme_boost_union.php b/tests/behat/behat_theme_boost_union.php index 495c8a4145e..5139ec649a4 100644 --- a/tests/behat/behat_theme_boost_union.php +++ b/tests/behat/behat_theme_boost_union.php @@ -15,12 +15,12 @@ // along with Moodle. If not, see . /** - * Standard behat step data providers for theme_boost_union + * Theme Boost Union - Standard behat step data providers. * - * @package theme_boost_union - * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} - * @author Mark Johnson - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @package theme_boost_union + * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class behat_theme_boost_union extends behat_base { /** @@ -203,7 +203,7 @@ protected function resolve_page_instance_url(string $type, string $identifier): ); default: - throw new Exception('Unrecognised quiz page type "' . $type . '."'); + throw new Exception('Unrecognised theme_boost_union page type "' . $type . '."'); } } @@ -232,7 +232,7 @@ public static function get_exact_named_selectors(): array { ), new behat_component_named_selector( 'Smart menu item', - [".//a[contains(@class, 'boost-union-smartitem')][contains(text(), %locator%)]"], + [".//a[contains(@class, 'boost-union-smartmenuitem')][contains(text(), %locator%)]"], ), new behat_component_named_selector( 'Main menu smart menu', @@ -245,7 +245,7 @@ public static function get_exact_named_selectors(): array { 'Main menu smart menu item', [ ".//div[contains(@class, 'primary-navigation')]" . - "//a[contains(@class, 'boost-union-smartitem')][contains(text(), %locator%)]", + "//a[contains(@class, 'boost-union-smartmenuitem')][contains(text(), %locator%)]", ], ), new behat_component_named_selector( @@ -259,7 +259,7 @@ public static function get_exact_named_selectors(): array { 'Menu bar smart menu item', [ ".//nav[contains(@class, 'boost-union-menubar')]" . - "//a[contains(@class, 'boost-union-smartitem')][contains(text(), %locator%)]", + "//a[contains(@class, 'boost-union-smartmenuitem')][contains(text(), %locator%)]", ], ), new behat_component_named_selector( @@ -272,7 +272,7 @@ public static function get_exact_named_selectors(): array { 'User menu smart menu item', [ ".//div[@id = 'usermenu-carousel']//div[contains(@class, 'carousel-item')]" . - "//a[contains(@class, 'boost-union-smartitem')][contains(text(), %locator%)]", + "//a[contains(@class, 'boost-union-smartmenuitem')][contains(text(), %locator%)]", ], ), new behat_component_named_selector( @@ -285,7 +285,7 @@ public static function get_exact_named_selectors(): array { 'Bottom bar smart menu item', [ ".//nav[contains(@class, 'boost-union-bottom-menu')]" . - "//a[contains(@class, 'boost-union-smartitem')][contains(text(), %locator%)]", + "//a[contains(@class, 'boost-union-smartmenuitem')][contains(text(), %locator%)]", ], ), ]; diff --git a/tests/behat/theme_boost_union_smartmenusettings_menuitems_dynamiccourses.feature b/tests/behat/theme_boost_union_smartmenusettings_menuitems_dynamiccourses.feature index eae56aa4002..923ffc5f3a5 100644 --- a/tests/behat/theme_boost_union_smartmenusettings_menuitems_dynamiccourses.feature +++ b/tests/behat/theme_boost_union_smartmenusettings_menuitems_dynamiccourses.feature @@ -121,10 +121,10 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, usi | category_subcats | | When I log in as "student1" Then "Course 01" "theme_boost_union > Smart menu item" should exist in the "List menu" "theme_boost_union > Main menu smart menu" + And "Course 01" "theme_boost_union > Smart menu item" should exist in the "List menu" "theme_boost_union > Menu bar smart menu" + And "Course 01" "theme_boost_union > Smart menu item" should exist in the "List menu" "theme_boost_union > User menu smart menu" + And "Course 01" "theme_boost_union > Smart menu item" should exist in the "List menu" "theme_boost_union > Bottom bar smart menu" And "Course 01a" "theme_boost_union > Smart menu item" exist in the "List menu" "theme_boost_union > Main menu smart menu" - And "Course 01a" "theme_boost_union > Smart menu item" exist in the "List menu" "theme_boost_union > Menu bar smart menu" - And "Course 01a" "theme_boost_union > Smart menu item" exist in the "List menu" "theme_boost_union > User menu smart menu" - And "Course 01a" "theme_boost_union > Smart menu item" exist in the "List menu" "theme_boost_union > Bottom bar smart menu" And "Course 01b" "theme_boost_union > Smart menu item" exist in the "List menu" "theme_boost_union > Main menu smart menu" And "Course 01aa" "theme_boost_union > Smart menu item" exist in the "List menu" "theme_boost_union > Main menu smart menu" @@ -246,7 +246,7 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, usi | title | Dynamic courses | | itemtype | Dynamic courses | | customfields | Test field: | - When I log in as "" + When I log in as "student1" Then "Course 07" "theme_boost_union > Smart menu item" exist in the "List menu" "theme_boost_union > Main menu smart menu" And "Course 07" "theme_boost_union > Smart menu item" exist in the "List menu" "theme_boost_union > Menu bar smart menu" And "Course 07" "theme_boost_union > Smart menu item" exist in the "List menu" "theme_boost_union > User menu smart menu" @@ -256,9 +256,9 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, usi And "Course 04" "theme_boost_union > Smart menu item" exist in the "List menu" "theme_boost_union > Main menu smart menu" Examples: - | value | user | course1 | course2 | course3 | course4 | - | value1 | student1 | should | should | should not | should not | - | value2 | student1 | should not | should not | should | should not | + | value | course1 | course2 | course3 | course4 | + | value1 | should | should | should not | should not | + | value2 | should not | should not | should | should not | @javascript Scenario Outline: Smartmenus: Menu items: Dynamic courses - Sort the course list based on the given setting diff --git a/tests/generator/behat_theme_boost_union_generator.php b/tests/generator/behat_theme_boost_union_generator.php index faba43f59ba..21e9922a65b 100644 --- a/tests/generator/behat_theme_boost_union_generator.php +++ b/tests/generator/behat_theme_boost_union_generator.php @@ -18,12 +18,12 @@ use theme_boost_union\smartmenu_item; /** - * Behat generator for Boost Union + * Theme Boost Union - Behat generator. * - * @package theme_boost_union - * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} - * @author Mark Johnson - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @package theme_boost_union + * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class behat_theme_boost_union_generator extends behat_generator_base { /** diff --git a/tests/generator/lib.php b/tests/generator/lib.php index e1e33051551..be09ea01f4f 100644 --- a/tests/generator/lib.php +++ b/tests/generator/lib.php @@ -18,12 +18,12 @@ use theme_boost_union\smartmenu_item; /** - * Boost Union test data generator + * Theme Boost Union - Test data generator. * - * @package theme_boost_union - * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} - * @author Mark Johnson - * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + * @package theme_boost_union + * @copyright 2024 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class theme_boost_union_generator extends component_generator_base { /**