Add tests for menu active trail support

bobi-mel → made their first commit to this issue’s fork.

I've fixed the issue. Please check it.

Thanks @bobi-mel this is a great start!

However, based on the comments in the test it looks like you're testing the local memory of which block should be open rather than the active menu trail?

The test should go something like:

• Set global settings to "collapsible, collapsed by default"
• Put a menu block on the site
• Go to a page that is in that menu and check that the block is open
• Go to a page this not in that menu and check that the block is closed.

Hi @darvanen

I plan to create a menu with links to nodes.
Changes that I plan to make:
- set by default that the blocks are collapsed in the config form of the feed
- create a content type
- two nodes of this content type
- create a menu
- two fields in the menu (links to these nodes)
- display the menu in the layout block as a block
- change the validation logic
(when the user enters the node, the menu should be expanded, closed on the user's page)

Translated with DeepL.com (free version)

I've fixed the issue. Please check it.

Thanks @bobi-mel.

The fact that the test is supposedly passing with this setting in place gives me great concern for the logic in the test:

$configFormValues['active_pages'] = '0';

That is turning the active menu trail behaviour off.

Have you been running these tests locally and checking the browser output? That might help.

We do need to test behaviour with that setting turned off, but we also need to test with the setting turned on.

Hi @darvanen
I am very sorry that I may have misunderstood the essence of this problem. However, let's finish working on it.
Regarding the value of the option in the settings form 'active_pages' = '0'. I faced a problem that the collapsiblockContentCollapsed class is not removed from the menu in the browser output, since it must be set at the page load stage for the menu to be collapsed. I disabled it because during manual testing this option does not influence on the menu active trail.

Maybe you have any ideas how to do it better?

@bobi-mel no need for an apology! When I'm pointing out things that aren't correct it is to help, not to scold ;)

Yes of course let's complete this, however I think it would be good to get 📌 Add gitlab CI Needs work finished first so we can just have one set of tests running, shall we focus on that first?

Hello! @darvanen

I have checked my test and the current implementation of the module again and tested manually how the different settings work. I found out that when we do not check the "Allow menu blocks to be collapsed on page load if they have active links" checkbox and go to a page with an active link, the menu is open (for the test I created a menu with nodes and added the "/node/1" and "/node/2" links to it. I chose the same type of block behavior as in the test - value 3 - "Collapsible, collapsed by default.").
When you go to the user page or the main page, the menu is always collapsed.
If you check the box "Allow menu blocks to be collapsed on page load if they have active links" in the module settings, the menu block will always be collapsed.
I have reviewed the issue " Clarify "Collapsed State on Active Pages" behavior → " and agree that for correct testing the value of the "active_pages" parameter should be "1".
Therefore, I suggest you accept these changes and open an issue to fix the module logic so that it works as expected

Ah, I see what happened. In #3169973: Clarify "Collapsed State on Active Pages" behavior → I did decide that the wording should be

The primary purpose of this behaviour is to keep a menu block expanded if it contains a link to the current page.

but... when I went to edit the code and wording I found that it had been set up the other way around, hence the different wording on the UI:

Allow menu blocks to be collapsed on page load if they have active links

I don't want to break sites by reversing the polarity of the setting, so let's go with your original test which expected the value to be 0 for always open and 1 for collapsible. Sorry.

Hi @darvanen
I've returned my changes back. All tests passed successfully. Please check it.

Thanks very much @bobi-mel, I'm not seeing any pipelines running on this job. I think we might need to open a fresh fork/MR to benefit from all the recent work on pipelines.

Hi @darvanen

I've rebased the 4.x branch into this branch.
Please check it

I have made a very small adjustment to grammar in the comments, and asked a question in the MR.

HI @darvanen

I have corrected your comments. Please review again

Thanks very much, looks good. Happy to commit this without 'next_major' passing as we have an open issue to check Drupal 11 testing compatibility when the current infrastructure failure clears.

Automatically closed - issue fixed for 2 weeks with no activity.