- First commit to issue fork.
- Merge request !24Issue #3295655: Add tests for menu active trail support β (Merged) created by Unnamed author
- last update
about 1 year ago 15 pass - Status changed to Needs review
about 1 year ago 5:41pm 4 March 2024 - Status changed to Needs work
about 1 year ago 11:35pm 6 March 2024 - π¦πΊAustralia darvanen Sydney, Australia
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.
- πΊπ¦Ukraine bobi-mel
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)
- last update
about 1 year ago 15 pass - Status changed to Needs review
about 1 year ago 8:04am 13 March 2024 - Status changed to Needs work
about 1 year ago 9:18pm 13 March 2024 - π¦πΊAustralia darvanen Sydney, Australia
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.
- πΊπ¦Ukraine bobi-mel
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?
- π¦πΊAustralia darvanen Sydney, Australia
@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?
- Status changed to Needs review
about 1 year ago 6:31pm 5 April 2024 - πΊπ¦Ukraine bobi-mel
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 - Status changed to Needs work
12 months ago 1:24am 9 April 2024 - π¦πΊAustralia darvanen Sydney, Australia
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.
- Status changed to Needs review
12 months ago 1:27pm 9 April 2024 - πΊπ¦Ukraine bobi-mel
Hi @darvanen
I've returned my changes back. All tests passed successfully. Please check it. - Status changed to Needs work
12 months ago 10:27am 10 April 2024 - π¦πΊAustralia darvanen Sydney, Australia
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.
- Status changed to Needs review
12 months ago 9:23am 11 April 2024 - πΊπ¦Ukraine bobi-mel
Hi @darvanen
I've rebased the 4.x branch into this branch.
Please check it - Status changed to Needs work
12 months ago 1:28pm 14 April 2024 - π¦πΊAustralia darvanen Sydney, Australia
I have made a very small adjustment to grammar in the comments, and asked a question in the MR.
- Status changed to Needs review
11 months ago 6:12pm 15 May 2024 - πΊπ¦Ukraine bobi-mel
HI @darvanen
I have corrected your comments. Please review again
- Status changed to RTBC
11 months ago 10:05am 16 May 2024 - π¦πΊAustralia darvanen Sydney, Australia
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.
-
darvanen β
committed f78ed7d5 on 4.x authored by
bobi-mel β
Issue #3295655 by bobi-mel, darvanen: Add tests for menu active trail...
-
darvanen β
committed f78ed7d5 on 4.x authored by
bobi-mel β
- Status changed to Fixed
11 months ago 10:05am 16 May 2024 Automatically closed - issue fixed for 2 weeks with no activity.