Follow up to #3169973: Clarify "Collapsed State on Active Pages" behavior for tests.
Issue fork collapsiblock-3295655
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #4
bobi-mel commentedI've fixed the issue. Please check it.
Comment #5
darvanenThanks @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:
Comment #6
bobi-mel commentedHi @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)
Comment #7
bobi-mel commentedI've fixed the issue. Please check it.
Comment #8
darvanenThanks @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.
Comment #9
bobi-mel commentedHi @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?
Comment #10
darvanen@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 #3407882: Add gitlab CI finished first so we can just have one set of tests running, shall we focus on that first?
Comment #11
bobi-mel commentedHello! @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
Comment #12
darvanenAh, I see what happened. In #3169973: Clarify "Collapsed State on Active Pages" behavior I did decide that the wording should be
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:
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.
Comment #13
bobi-mel commentedHi @darvanen
I've returned my changes back. All tests passed successfully. Please check it.
Comment #14
darvanenThanks 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.
OR rebase, whatever you find easier.
Comment #15
bobi-mel commentedHi @darvanen
I've rebased the 4.x branch into this branch.
Please check it
Comment #16
darvanenI have made a very small adjustment to grammar in the comments, and asked a question in the MR.
Comment #17
bobi-mel commentedHI @darvanen
I have corrected your comments. Please review again
Comment #18
darvanenThanks 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.
Comment #20
darvanen