Hi @ggh
Please create the MR for this issue
@see https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... →
HI @fgm
I added CI/CD pipeline and fixed PHPCS issues.
Please check my MR.
Thanks
Yes, sure
I propose you change the default branch to 8.x-1.x
bobi-mel → created an issue.
bobi-mel → made their first commit to this issue’s fork.
Hi @darvanen
I updated the schema and settings keys and labels
Hi @darvanen
I added the latest changes as we plan. Also, fixed the MR conflict and fixed broken test.
Please check it.
Hi.
I fixed remarks, please check one more time.
Hi @damienmckenna
The Unit tests successfully passed.
Please check the MR.
Hi @damienmckenna
The Unit tests successfully passed.
Please check the MR.
Hi @darvanen
Your idea sounds great.
I've investigated the current implementation of arrows. They are added as a background for the field. I think the implementation of your suggestion will require big changes to the current logic and it will be a major update, and I'm not sure if we can apply these changes to sites that already use this module without breaking them.
So I propose to do the following.
1) Implement my suggestions in comment #28;
2) Make a new release of the module;
3) Within a month, check if there are any new suggestions for graphics from the developers;
4) Create an issue for the new version 5 of the module;
5) Create a problem for the graphics with a detailed description of how the graphic design of the module should look like for version 5 (several types of arrows, adding custom colors for arrows, compatibility with prefers-color-scheme);
6) Add this issue as a parent issue;
7) Create other issues that will expand the functionality of the module and follow the latest drupal trends.
What do you think of this idea?
Ok. Let us know if everything works as expected
Thanks for everyone
bobi-mel → made their first commit to this issue’s fork.
Hi @darvanen
I tried to emulate the cases by using the themes that are dark. I used Seven Dark and Dark Awesome. Unfortunately, these themes don't have a specific class that is added after the switched theme color mode, but have the 'js' class that are present in the HTML tag for authenticated and anonymous users.
Also, I am not sure that the site uses a contrib theme as a site theme. In the custom theme, a developer can add this specific class that can be used as a trigger to switch color mode for arrows.
Based on the above I propose the following:
1) rename the new checkbox label from 'Enable Dark mode' to 'Enable collapse arrow color switcher'
2) rename the field for the class that changes the color from "Theme dark mode class' to "Color mode trigger class'
3) Add an additional description for the 'Theme dark mode class'. If your theme is dark to set the white color for the arrow add the class that is present in the Body or HTML tag for authenticated and anonymous users or create it.
Let me know what you think about it.
Probably you have any suggestion
Works well for Drupal 10.3.6
Hi @hesslinger
the patch #270 works well
We have separate tests for the Image and the Media Reference. While working on the issue I fixed the test for the Media Reference after splitting the formatted
I guess they are enough
Do you mean a theme doesn't have the white mode at all?
bobi-mel → created an issue.
Thanks for support. It really helped.
See MR - https://git.drupalcode.org/project/collapsiblock/-/merge_requests/37
Hi @darvanen
I have finished working on this task, but I faced the problem of creating and running a pipeline for the last commit for the PHPUnit tests to run with the Gin theme as the testing theme. I had assumed that it was a problem with the GitLab pipeline and hoped that closing it and creating a new merge request would solve it.
Unfortunately, this did not solve the problem.
Can you tell me how to start the pipeline for the last commit since I haven't found the cause of the problem yet?
bobi-mel → created an issue.
I fixed the issue. Please check it.
I thought it over and agree with you the Git theme will be good for a test.
HI
I fixed PHPCH, created test coverage for the Protected Pages View Filter, and fixed the bug for the Select All option that I found while working on the test.
Please check it
Hi @darvanen
I tried to use the Gin theme → , but it is only an admin theme and can't be used for front-end part of the site.
I propose:
1) Add a new 'Dark mode' element to the module config form;
2) Also, Add text field 'Theme class for dark mode', because different themes add different custom classes the Body or HTML tag.
4) Use existing behavior if added theme dark mode class and enabled dark mode for module set the class 'collapsiblock-dark-mode' to the block title;
5) Create two SVG images for each type of arrow: dark and white;
6) By default use dark.
7) Use these arrows - https://www.svgrepo.com/svg/393200/triangle-down-filled as more modern.
Let me know what you think about it
Hi @darvanen
1) I mean graphic not chart, sorry
2) I will investigate the issue and let you know my ideas to solve it.
Hi @darvanen
I will try to solve this problem.
Please comment on my questions that I left in it
Hi @darvanen
I will try to solve this problem. However, in order not to waste time, I would like to clarify some details:
1) If possible, please provide an example of the future chart;
2) What behavior do you expect the chart to have?
3) The Acquia slate theme is outdated, what theme will we use to test the results?
4) Add important points to consider while working on the task;
5) Your suggestions for the final result of the assignment.
Thank you!
Hi
I added the additional Title field to save the admin label.
Please check it.
HI!
I implemented the same solution for the PHP tests as for the related issue.
Please check it.
Hi!
Fixed the test, please check it!
Hi @Diwakar07
Thanks for your contribution
Commented on your question
Hi @darvanen @jurgenhaas
I discussed with my colleagues the problem of uniqueness for each block cookie and we came to the conclusion that the only reliable way to guarantee the uniqueness of cookies in Layout Builder is to add a UUID to the block ID.
I added these changes and also added changes for tests to search for a block by class.
Check it out.
@darvanen
After adding weight to the block ID, the testLayoutBuilderBlockWillCollapse test was broken. I tried to quickly retrieve the block from the entity node, but I haven't found a good solution yet.
When running tests, the weight of the test block is always "2".
Please clarify if it will be good if we define the Xpath for Title and Content in the test as follows.
$collapsiblockTestBlockTitleXpath = $this->assertSession()->buildXPathQuery('//*[@id=:blockId]//h2', [
':blockId' => '#collapse-blocknode' . $nid . 'body . 2',
]);
$collapsiblockTestBlockContentXpath = $this->assertSession()->buildXPathQuery('//*[@id=:blockId]//div[2]/p', [
':blockId' => 'collapse-blocknode' . $nid . 'body' . '2-content',
]);
Perhaps you have better ideas than mine, I will be grateful for your advice
@jurgenhaas
I added blocks in two ways:
1) The first is to have the block added to each new node that is created. I added it as follows:
- Followed this link - /admin/structure/types/manage/article/display/default/layout
- Clicked on the Add block button and added the selected block
2) The second way is to edit the Layout for a specific node:
- I followed the link - /node/5/layout
- Click on the Add block button and add the block
Regarding the value 0 or 1 after ":" in the cookie.
The value "0" will be set for the key of the block whose state has changed and is now set to be minimized.
The value "1" has analogous logic only for the block whose current state is "expanded" after changes.
Accordingly, if you changed the display of blocks and set the current state for all "minimized", then all keys in the cookies will be with the value "0"
@darvanen
I tried to add the same block to different sections of the same node and their behavior does not affect each other, it works as expected
Hi @darvanen @jurgenhaas
I tried to reproduce the potential problems described in #49.
I got the following results:
1) How to add the same block to different sections, they will have different weights and each will keep their behavior
2) I added a block that will be added to each node when it is created. When changing it from collapsed to expanded, its state is saved for each node.
3) I tried to create a page structure by manually adding the same blocks in the same sequence to two different nodes, for example: a Related Content block below it is a Who's Online block. Their state is saved and displayed on each of them in the same way.
If the structure is different, the behavior is also different.
4) @jurgenhaas try deleting cookies in your browser manually and check again, I have a hunch that this is the problem.
I think that adding weight solves the problem with the uniqueness of the cookie keys.
I've found the reason and fix the bug. Also, added the patch. Please check it.
bobi-mel → created an issue.
bobi-mel → created an issue.
The patch #2 works well.
Hi @skyredwang
I applied the patch #4. Works well.
Thanks
HI @darvanen
I studied this issue and was able to reproduce it by adding two identical blocks to the LayoutBuilder Layout. The EventSubscriber that was added to implement the module logic functionality for LayoutBuilder does not guarantee the uniqueness of the ID for the blocks. I propose to add to the logic of our EventSubscriber to the ID of the block its weight without a lower dash as a separator to guarantee the uniqueness of the key for the cookie.
@see https://git.drupalcode.org/project/collapsiblock/-/merge_requests/19/dif...
change the code to the following $id = Html::getId('collapsiblock-wrapper-' . $build['#configuration']['id'] . $build["#weight"]);
The cookie will look like this: {%22who7%22:1%2C%22who8%22:0}
Let me know what you think of my idea and we can add it
HI @darvanen
I have corrected your comments. Please review again
The patches #2 and #3 can't be applied for D10. I added a new patch and created the MR.
bobi-mel → made their first commit to this issue’s fork.
bobi-mel → created an issue.
@faust74 Thanks for your contribution
@faust74 Thanks for your contribution
@faust74 Thanks for your contribution
bobi-mel → made their first commit to this issue’s fork.
@KuzyaWkk Looks good! Thanks for the contribution.
Don't forget to set the correct status of the issue - Needs review
bobi-mel → made their first commit to this issue’s fork.
@pftech has not provided any comments or suggestions.
I think the issue can be closed
@KuzyaWkk - thank you for your contribution
Thanks, everyone!!! I'm happy to join your team
Hi @darvanen
I've rebased the 4.x branch into this branch.
Please check it
Hi @darvanen
I've rebased the 4.x branch into this branch.
Please check it
One additional issue was fixed by me
Add a FunctionalJavascript test for the jQuery.fn.selectAll functionality ✨ Add a FunctionalJavascript test for the jQuery.fn.selectAll functionality Fixed
Hi @darvanen
I've returned my changes back. All tests passed successfully. Please check it.
@mparker17
I've fixed PHPCS notices
I've fixed the issue Add a FunctionalJavascript test for the jQuery.fn.selectAll functionality ✨ Add a FunctionalJavascript test for the jQuery.fn.selectAll functionality Fixed
Please check it.
Hi @mparker17
I've created the test for it. I have reproduced the same steps as in your comment.
Please check it.
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
Hello @darvanen
I have returned the test I deleted and made some changes to it. I changed the previous method of adding a field as a block because it did not add the wrapper for the block provided by the module.
Please check my changes.
Hello
@scott_euser.
I have reviewed this issue and the @viren18febS commit.
I found that the issue was created for the 2.0 branch of the module. Therefore, I created a separate branch for the 2.0 version of the module, made the fixes as per your recommendation and created a merge request. Please check again if everything is done correctly
bobi-mel → made their first commit to this issue’s fork.
Hi
@Raveen Thakur
The method accessCheck() has the default value TRUE
@see web/core/lib/Drupal/Core/Entity/Query/QueryInterface.php
If you didn't have any parameters access checking will enabled
@TR
I've chacked once again my site and the Voting API module. I using the 8.x-3.0-beta4 version. In this version the getEntityResults() method doesn't have call the accessCheck() method. Also, I've checked the dev version of the module. The accessCheck() really exists in it. It something strange why these changes didn't include the latest release of the module. The patch is solving the problem to the next release
I've fixed the issue. Check it
bobi-mel → created an issue.
I've fixed the issue
bobi-mel → made their first commit to this issue’s fork.
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 → created an issue.
Hi @darvanen
I have checked this issue and tried to reproduce it. I displayed fasets using the facets_block. To test I was added several fasets to the block.
All content in the block body are hiding or displaying as expected.
I think that the Collapsiblock module is fully compatible with the Facets Block module and the issue needs to be closed as fixed.