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.
Hi @darvanen
I have checked this issue and tried to reproduce it. I added the test block group in different sections (Content and Sidebar), also in the block group I added different blocks (menu, custom block).
During the test, I found out the following:
- The Collapsiblock block wrapper was successfully added to the block group ;
- All blocks in the block group were hidden if clicked by the group title;
- Collapsing settings correctly works for each of the blocks in the group;
I think that the Collapsiblock module is fully compatible with the Block Group module and the issue needs to be closed as fixed.
I've fixed the issue. Please check it.
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 added the test
I found the reason why the testLayoutBuilderBlockWillCollapse() test fails.
The test tries to display a node field with a block, but the ig does not provide that the block should display node fields, but only blocks, respectively, in the UI form appear, but in fact it does not affect the display of the field.
I plan to do the following:
- delete the test testLayoutBuilderBlockWillCollapse() (leave only the test for the form)
- create a new file called LayoutBuilderUITest and describe the test there
- create a content type in it
- switch to the default display
- add a block and save the layout
- go to the page and check the current state
- JS click on the tag to change the display
- check if it has changed
I've fixed the issue. Please check it.
bobi-mel → made their first commit to this issue’s fork.
I've separated mage and entity_reference formatters. Please check it.
Hi @ravi kant
I saw these issues when I started working on this issue. However, the eslint issues that are being ignored are related to compiled files. I haven't found a better solution to this problem yet.
Hi
I've removed the neon file and added @phpstan-ignore-next-line
Could you tell me which code you consider unrelated to this task?
I've made the sub-module to implement the plugin
HI! @scott_euser
Thank you for your support!
bobi-mel → made their first commit to this issue’s fork.
Hi @dinazaur @Anybody
I returned the implementation of this method to the current implementation as in the 5.x branch, but removed the isAttached() method as useless
bobi-mel → made their first commit to this issue’s fork.
I've fixed the test. Please check it
Hi @
Comment#2darvanen
We have the following
- all testing of modules will migrate to Drupal GitLab CI
- the process of testing is more flexible
- run tests more fast (it is very useful)
more details here -
https://www.drupal.org/about/core/blog/drupal-cores-gitlab-ci-testing-is... →
@Grevil
I moved the changes to fix the tests to
🐛
Fix broken tests
Fixed
. But without the fixes in photoswipe.setting.yml, the test will fail at the Drupal initiation stage. So to start working on improving the tests, we need to fix the existing issues in the tests
I've fixed broken tests. Place check it.
bobi-mel → made their first commit to this issue’s fork.
I've separated mage and entity_reference formatters. Please check it.
bobi-mel → made their first commit to this issue’s fork.