? ? ? Pending

User tests: Successful: Unsuccessful:

avatar drmenzelit
drmenzelit
19 May 2021

Pull Request for Issue #33918 .

Summary of Changes

This PR removes the Bootstrap collapsible navbar from the template and adds 2 new menu layouts for the collapse function: one layout in core for the default menu and one layout in Cassiopeia for the metismenu (dropdown menu).

These changes allow to decide if the menu should use the responsiveness from Bootstrap or not. Without the collapse function from Bootstrap the user can change the behaviour of the menu as he/she likes using CSS or can use 3rd-party menu modules that bring its own responsiveness.

Testing Instructions

Create a menu on position menu. Change the layout, you can choose between

  • Collapsible Default Menu (core menu with Bootstrap collapse function = burger menu)
  • Default (core menu without collapse)
  • Collapsible Dropdown (Metismenu/Dropdown menu with collapse function)
  • Dropdown (Metismenu without collapse)

Try a 3rd-party menu module.

Actual result BEFORE applying this Pull Request

On mobile the menu displays as burger menu (Navbar Toggler).

Expected result AFTER applying this Pull Request

The menu will show collapsed only if one of the collapsible layouts is selected

image

Menu with layout Collapsible Dropdown

image

Menu with layout Dropdown

image

Note: sample data have to be updated to use collapsible metismenu by default

Co-authored by @dgrammatiko

avatar drmenzelit drmenzelit - open - 19 May 2021
avatar drmenzelit drmenzelit - change - 19 May 2021
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 May 2021
Category Language & Strings Front End Templates (site)
avatar dgrammatiko
dgrammatiko - comment - 19 May 2021

The decision for the menu collapsible behaviour shouldn't be in the template but in the module menu.
This solution is the best showcase of why Joomla ended up having a gazillion toggle switches and being completely disaster on the UX...
Just use a new layout in the mod_menu...

avatar richard67
richard67 - comment - 19 May 2021

Beside this, it should be made consistently also in the error.php file of Cassiopeia.

avatar drmenzelit
drmenzelit - comment - 19 May 2021

Beside this, it should be made consistently also in the error.php file of Cassiopeia.

If this get approved, of course

avatar drmenzelit
drmenzelit - comment - 19 May 2021

Just use a new layout in the mod_menu...

I understand you, but creating a new layout for the bootstrap part combined with the existent core menu and the new metismenu from Cassiopeia is a level over my skills... I would love to know how to do that.

avatar dgrammatiko
dgrammatiko - comment - 19 May 2021

I would love to know how to do that

Maybe something like this: https://github.com/joomla/joomla-cms/compare/4.0-dev...dgrammatiko:—respect-the-architecture-4.0-dev?expand=1

Expands the layouts instead of adding more switches to the template (template doesn't and should never have the responsibility to modify the menu, there's a module for that [modular architecture]).
Screenshot 2021-05-19 at 20 50 56

avatar drmenzelit
drmenzelit - comment - 19 May 2021

Ahhh.. I was thinking too complicated ;-) Your code is much simpler than I thought

avatar drmenzelit
drmenzelit - comment - 19 May 2021

Will you create the PR?

avatar dgrammatiko
dgrammatiko - comment - 19 May 2021

Will you create the PR?

Since you already have this one open you could adjust the code here. There's one thing missing: remove the burger text from the template and add it to the mod_menu

avatar richard67
richard67 - comment - 19 May 2021

Is this burger removal a vegan thing? :-)

avatar dgrammatiko
dgrammatiko - comment - 19 May 2021

Is this burger removal a vegan thing? :-)

I guess the burger removal from any human is totally biodegradatable :D

avatar drmenzelit
drmenzelit - comment - 19 May 2021

Thank you @dgrammatiko ! I will rework the PR tomorrow.

avatar drmenzelit drmenzelit - change - 20 May 2021
Labels Added: ? ?
avatar joomla-cms-bot joomla-cms-bot - change - 20 May 2021
Category Language & Strings Front End Templates (site) Language & Strings Modules Front End Templates (site)
avatar drmenzelit drmenzelit - change - 20 May 2021
The description was changed
avatar drmenzelit drmenzelit - edited - 20 May 2021
avatar drmenzelit drmenzelit - change - 20 May 2021
The description was changed
avatar drmenzelit drmenzelit - edited - 20 May 2021
avatar drmenzelit drmenzelit - change - 20 May 2021
Title
[4.0][Cassiopeia]Add parameter to switch off Bootstrap Navbar Toggler
[4.0][Cassiopeia]Add new layouts to switch off Bootstrap Navbar Toggler
avatar drmenzelit drmenzelit - edited - 20 May 2021
avatar dgrammatiko dgrammatiko - test_item - 20 May 2021 - Tested successfully
avatar dgrammatiko
dgrammatiko - comment - 20 May 2021

I have tested this item successfully on d878714


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33978.

avatar drmenzelit
drmenzelit - comment - 21 May 2021

@rbuelund would you test this PR?

avatar rbuelund
rbuelund - comment - 21 May 2021

I would like to test it, but how - I have no test setup.. I have a beta-8 site.

avatar richard67
richard67 - comment - 21 May 2021

@drmenzelit Do I understand right that the new collapse-layouts are equal to the default layouts without this PR? If so, then blog sample data should be changed for the "Main Menu Blog" using the new collapsible layout here:

https://github.com/joomla/joomla-cms/blob/4.0-dev/plugins/sampledata/blog/blog.php#L1437

avatar richard67
richard67 - comment - 21 May 2021

@drmenzelit Do I understand right that the new collapse-layouts are equal to the default layouts without this PR? If so, then blog sample data should be changed for the "Main Menu Blog" using the new collapsible layout here:

https://github.com/joomla/joomla-cms/blob/4.0-dev/plugins/sampledata/blog/blog.php#L1437

But I'm not sure. @chmst What do you think? Let blog sample data like it is now so it will use the non-collapsible layout after this PR is applied? Or change it so it uses the collapsible layout and so behaves like before?

avatar drmenzelit
drmenzelit - comment - 21 May 2021

@richard67 , @chmst I think, we should change the sample data to use the collapsible layout

avatar drmenzelit
drmenzelit - comment - 21 May 2021

I would like to test it, but how - I have no test setup.. I have a beta-8 site.

You can download a prebuilt package from this PR and install it

image

avatar rbuelund
rbuelund - comment - 21 May 2021

Sorry - I dot not really now what to download and from where... and I am not running on local server

avatar dgrammatiko
dgrammatiko - comment - 21 May 2021

@rbuelund go to https://ci.joomla.org/artifacts/joomla/joomla-cms/4.0-dev/33978/downloads/44050/ and download the file from the second link Joomla_4.0.0-beta8-dev+pr.33978-Development-Full_Package.zip. Then install it as a new site and do your test

avatar rbuelund
rbuelund - comment - 21 May 2021

Hmmm.. not quite ?? If I install CK Maximenu module - then I have no posibility to select the template layout ?? The burger is still there..

selections

avatar dgrammatiko
dgrammatiko - comment - 21 May 2021

Hmmm.. not quite ?? If I install CK Maximenu module - then I have no posibility to select the template layout

For sure you can't use the layouts of X module on Y module. This is expected

avatar richard67
richard67 - comment - 21 May 2021

Hmmm.. not quite ?? If I install CK Maximenu module - then I have no posibility to select the template layout ??

@rbuelund It's a module layout, not a template layout, what this PR provides. So if your 3rd party module doesn't allow to select if it can be collapsed, the it's not a problem with the CMS.

avatar rbuelund
rbuelund - comment - 21 May 2021

Well - this was what I originally wrote as my problem: "If one uses the Cassiopeia template and wants to use another horisontal menu extension as menu instead of Cassiopeias own menu, then Cassiopeias mobile menu icon is still shown." - I wrote explicitly "another extension" - in the thread here: #33918

avatar richard67
richard67 - comment - 21 May 2021

Well - this was what I originally wrote as my problem: "If one uses the Cassiopeia template and wants to use another horisontal menu extension as menu instead of Cassiopeias own menu, then Cassiopeias mobile menu icon is still shown." - I wrote explicitly "another extension" - in the thread here: #33918

@rbuelund If that other menu extension will not provide a menu toggle, no such toggle will be shown, so I don't really understand your problem now.

avatar rbuelund
rbuelund - comment - 21 May 2021

The toggle is indeed still shown with this solution
toggle

avatar rbuelund
rbuelund - comment - 21 May 2021

Ahh sorry - my wrong. No, it works.

avatar richard67
richard67 - comment - 21 May 2021

@rbuelund Finally 😄 Thanks. If that was a successful test, could you mark your test result in the issue tracker? Just go to here https://issues.joomla.org/tracker/joomla-cms/33978 , use the "Test this" button, select the right test result and submit. Thanks in advance.

avatar rbuelund rbuelund - test_item - 21 May 2021 - Tested successfully
avatar rbuelund
rbuelund - comment - 21 May 2021

I have tested this item successfully on d878714

This works as it should :-)


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33978.

avatar richard67
richard67 - comment - 21 May 2021

@dgrammatiko Finally, after this PR goes the right way now, maybe you should revert your "thumb down" for the description?

avatar rbuelund
rbuelund - comment - 21 May 2021

Done


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33978.

avatar richard67 richard67 - change - 21 May 2021
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 21 May 2021

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33978.

avatar richard67
richard67 - comment - 21 May 2021

@drmenzelit @chmst This PR will need a follow-up PR for making the Blog Sample Data use the collapsible dropdown layout, see my comment above #33978 (comment) .

Who wants to make it?

avatar drmenzelit
drmenzelit - comment - 21 May 2021

@chmst is the expert for sample data :-)

avatar chmst chmst - close - 21 May 2021
avatar chmst chmst - merge - 21 May 2021
avatar chmst chmst - change - 21 May 2021
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2021-05-21 18:34:20
Closed_By chmst
Labels Added: ?
avatar chmst
chmst - comment - 21 May 2021

Thank you! The blog sample data change will follow right now.

avatar richard67
richard67 - comment - 21 May 2021

Does anybody know if there is some documentation to be updated with the changes from this PR?

avatar richard67
richard67 - comment - 22 May 2021

For Blog sample data update see PR #34086 . Please test.

avatar chmst
chmst - comment - 23 May 2021

If there are more than one collapsible menus on a site, all are toggled simultaneoulsy (on cassiopiea).

avatar dgrammatiko
dgrammatiko - comment - 23 May 2021

If there are more than one collapsible menus on a site, all are toggled simultaneoulsy (on cassiopiea).

The code implies that the collapsible layout will be used only once per page because it has an ID that needs to be unique <div class="collapse navbar-collapse" id="navbar">. If you want to allow multiple instances of the module in the page you need to change the id to id="navbar-<?php echo $module->id; ?>" and also all the aria attributes referring to that id. Something like

<nav class="navbar navbar-expand-md">
	<button class="navbar-toggler navbar-toggler-right" type="button" data-bs-toggle="collapse" data-bs-target="#navbar-<?php echo $module->id; ?>" aria-controls="navbar-<?php echo $module->id; ?>" aria-expanded="false" aria-label="<?php echo Text::_('MOD_MENU_TOGGLE'); ?>">
		<span class="icon-menu" aria-hidden="true"></span>
	</button>
	<div class="collapse navbar-collapse" id="navbar-<?php echo $module->id; ?>">
		<?php require __DIR__ . '/default.php'; ?>
	</div>
</nav>
avatar chmst
chmst - comment - 23 May 2021

Thank you! In general, sites will have only one collapsible menu. So am not sure yet if it is a nice feature.

Add a Comment

Login with GitHub to post a comment