🇺🇦Ukraine @bobi-mel

Account created on 6 December 2022, over 2 years ago
#

Recent comments

🇺🇦Ukraine bobi-mel

HI @fgm

I added CI/CD pipeline and fixed PHPCS issues.

Please check my MR.

Thanks

🇺🇦Ukraine bobi-mel

Yes, sure

I propose you change the default branch to 8.x-1.x

🇺🇦Ukraine bobi-mel

Hi @darvanen

I updated the schema and settings keys and labels

🇺🇦Ukraine bobi-mel

Hi @darvanen

I added the latest changes as we plan. Also, fixed the MR conflict and fixed broken test.

Please check it.

🇺🇦Ukraine bobi-mel

Hi.
I fixed remarks, please check one more time.

🇺🇦Ukraine bobi-mel

Hi @damienmckenna

The Unit tests successfully passed.
Please check the MR.

🇺🇦Ukraine bobi-mel

Hi @damienmckenna

The Unit tests successfully passed.
Please check the MR.

🇺🇦Ukraine bobi-mel

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?

🇺🇦Ukraine bobi-mel

Ok. Let us know if everything works as expected

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

Hi @hesslinger

the patch #270 works well

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

Do you mean a theme doesn't have the white mode at all?

🇺🇦Ukraine bobi-mel

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?

🇺🇦Ukraine bobi-mel

I thought it over and agree with you the Git theme will be good for a test.

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

Hi @darvanen

1) I mean graphic not chart, sorry

2) I will investigate the issue and let you know my ideas to solve it.

🇺🇦Ukraine bobi-mel

Hi @darvanen

I will try to solve this problem.
Please comment on my questions that I left in it

🇺🇦Ukraine bobi-mel

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!

🇺🇦Ukraine bobi-mel

Hi
I added the additional Title field to save the admin label.
Please check it.

🇺🇦Ukraine bobi-mel

HI!
I implemented the same solution for the PHP tests as for the related issue.
Please check it.

🇺🇦Ukraine bobi-mel

Hi @Diwakar07
Thanks for your contribution

🇺🇦Ukraine bobi-mel

Commented on your question

🇺🇦Ukraine bobi-mel

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.

🇺🇦Ukraine bobi-mel

@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

🇺🇦Ukraine bobi-mel

@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"

🇺🇦Ukraine bobi-mel

@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

🇺🇦Ukraine bobi-mel

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.

🇺🇦Ukraine bobi-mel

I've found the reason and fix the bug. Also, added the patch. Please check it.

🇺🇦Ukraine bobi-mel

The patch #2 works well.

🇺🇦Ukraine bobi-mel

Hi @skyredwang
I applied the patch #4. Works well.
Thanks

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

HI @darvanen

I have corrected your comments. Please review again

🇺🇦Ukraine bobi-mel

The patches #2 and #3 can't be applied for D10. I added a new patch and created the MR.

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

@faust74 Thanks for your contribution

🇺🇦Ukraine bobi-mel

@faust74 Thanks for your contribution

🇺🇦Ukraine bobi-mel

@faust74 Thanks for your contribution

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

@KuzyaWkk Looks good! Thanks for the contribution.
Don't forget to set the correct status of the issue - Needs review

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

@pftech has not provided any comments or suggestions.
I think the issue can be closed
@KuzyaWkk - thank you for your contribution

🇺🇦Ukraine bobi-mel

Thanks, everyone!!! I'm happy to join your team

🇺🇦Ukraine bobi-mel

Hi @darvanen

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

🇺🇦Ukraine bobi-mel

Hi @darvanen

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

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

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.

🇺🇦Ukraine bobi-mel

Hi @mparker17
I've created the test for it. I have reproduced the same steps as in your comment.
Please check it.

🇺🇦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

🇺🇦Ukraine bobi-mel

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.

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

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

🇺🇦Ukraine bobi-mel

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

🇺🇦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?

🇺🇦Ukraine bobi-mel

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.

Production build 0.71.5 2024