Skip to content

Commit

Permalink
Review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
abias committed Dec 7, 2024
1 parent a00f3fd commit a244a77
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 40 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
9 changes: 8 additions & 1 deletion classes/smartmenu.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 14 additions & 11 deletions classes/smartmenu_item.php
Original file line number Diff line number Diff line change
Expand Up @@ -1042,21 +1042,27 @@ 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';
$class[] = $this->item->mobile ? 'd-none' : 'd-inline-flex';

// 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;

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

/**
Expand Down
22 changes: 11 additions & 11 deletions tests/behat/behat_theme_boost_union.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.

/**
* 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 <[email protected]>
* @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 <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class behat_theme_boost_union extends behat_base {
/**
Expand Down Expand Up @@ -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 . '."');
}
}

Expand Down Expand Up @@ -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',
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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%)]",
],
),
];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,10 @@ Feature: Configuring the theme_boost_union plugin on the "Smart menus" page, usi
| category_subcats | <subcat> |
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" <shouldornot> exist in the "List menu" "theme_boost_union > Main menu smart menu"
And "Course 01a" "theme_boost_union > Smart menu item" <shouldornot> exist in the "List menu" "theme_boost_union > Menu bar smart menu"
And "Course 01a" "theme_boost_union > Smart menu item" <shouldornot> exist in the "List menu" "theme_boost_union > User menu smart menu"
And "Course 01a" "theme_boost_union > Smart menu item" <shouldornot> exist in the "List menu" "theme_boost_union > Bottom bar smart menu"
And "Course 01b" "theme_boost_union > Smart menu item" <shouldornot> exist in the "List menu" "theme_boost_union > Main menu smart menu"
And "Course 01aa" "theme_boost_union > Smart menu item" <shouldornot> exist in the "List menu" "theme_boost_union > Main menu smart menu"

Expand Down Expand Up @@ -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: <value> |
When I log in as "<user>"
When I log in as "student1"
Then "Course 07" "theme_boost_union > Smart menu item" <course1> exist in the "List menu" "theme_boost_union > Main menu smart menu"
And "Course 07" "theme_boost_union > Smart menu item" <course1> exist in the "List menu" "theme_boost_union > Menu bar smart menu"
And "Course 07" "theme_boost_union > Smart menu item" <course1> exist in the "List menu" "theme_boost_union > User menu smart menu"
Expand All @@ -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" <course4> 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
Expand Down
10 changes: 5 additions & 5 deletions tests/generator/behat_theme_boost_union_generator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 <[email protected]>
* @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 <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class behat_theme_boost_union_generator extends behat_generator_base {
/**
Expand Down
10 changes: 5 additions & 5 deletions tests/generator/lib.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 <[email protected]>
* @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 <[email protected]>
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class theme_boost_union_generator extends component_generator_base {
/**
Expand Down

0 comments on commit a244a77

Please sign in to comment.