Account created on 10 June 2022, about 2 years ago
  • Senior Software Developer at ImageX 
#

Merge Requests

More

Recent comments

🇧🇷Brazil joaopauloc.dev

Hi @igorbiki thanks for reporting the issue.
I could reproduce the issue.
I added the unit test for the issue and also i'm adding the patch to fix it.

🇧🇷Brazil joaopauloc.dev

The patch views_load_more-3407928-1.patch worked as expected! Thank you so much @riyas_nr
Check in the image below.

I've created the merge request because the patch didn't work in the first moment and I made a quick fix.
But after spending time reviewing the path I could see that I was using the wrong token(@current_record_count) instead of the @end token.

🇧🇷Brazil joaopauloc.dev

Same issue here, thankfully for the @chejim comment I could fix, this setting that we need to do when we use a different view format could be added on the module page, this documentation will save time from other people.

To reproduce, just create a view and set a format that does not use the .view-content as a wrapper and the issue appears.

Steps to reproduce added on issue summary.

🇧🇷Brazil joaopauloc.dev

Hi folks, i was using the patch and didn't applied after update the module to 2.x version.
I couldn't reproduce this issue anymore on 2.x version.
Taking a look at the code sounds like this issue was fixed by Incorrect integer value for column 'gid'

🇧🇷Brazil joaopauloc.dev

@ckrina the current link on the Tugboat at the moment that I'm accessing is redirecting to another website.

🇧🇷Brazil joaopauloc.dev

It's working for me too.

Try to clear the browser cache since we have CSS files changed.

🇧🇷Brazil joaopauloc.dev

merge requested updated with the latest changes in 1.x branch.

🇧🇷Brazil joaopauloc.dev

Hi folks.
Nice work here, this module will increase a lot the usability of Drupal admin UI.
I found this issue because @rkoller mentioned this one in another issue.
Fixed the typo and works fine. See the image below.

🇧🇷Brazil joaopauloc.dev

joaopauloc.dev made their first commit to this issue’s fork.

🇧🇷Brazil joaopauloc.dev

Hello @dqd.
Exactly, the fields are still required after the submission. On the client side, the required status for both fields is updated and I'm able to submit the form.
But, I got an error message saying that field B is required but shouldn't be as I checked option 1.

I added two patches, one with the test unit with the scenario that I mentioned and the test unit will not pass, and another patch with the fix and you can see the test unit passing.

🇧🇷Brazil joaopauloc.dev

Hello @dqd, my sincere apologies for my last confusing and incomplete comment.
Let me explain better.

I have a website using this module on the 4.0.0-alpha1 version.
Then I tried to upgrade to the next version 4.0.0-alpha2.
After this upgrade one of my content types presented the wrong behavior mentioned in this issue.

I have field A which is a radio box with option 1 and option 2.
Also, I have field B that should be required and visible if option 1 is checked on field A
Then, field C is required and visible if field A has option 2 checked.

The expected behavior stopped working after the upgrade from 4.0.0-alpha1 to 4.0.0-alpha2. So, I tried to update to the latest version 4.0.0-alpha5 but the issue persisted.

After my research, I found this issue. In my last comment when I said I couldn't apply, I meant that I couldn't apply the changes of the merge request 3344587-required-fields-that as patch in the 4.0.0-alpha5 version. I saw other code changes that made the patch fail. So I applied the same change to the 4.0.0-alpha5 version and the behavior to make the field required based on the radio box worked.

Hope I explained better this time. Sorry for the confusion.
Thanks.

🇧🇷Brazil joaopauloc.dev

Got the same issue.
The changes works but I couldn't apply.
Attaching the new patch that I could apply to 4.0.0-alpha5 module version.
Thanks.

🇧🇷Brazil joaopauloc.dev

Also, maybe we need to change the icon when we are dragging which now is different from the icon on the table drag.

🇧🇷Brazil joaopauloc.dev

Hi folks, the last commit was missing the built CSS file.
Now you guys can see the new icon applying the patch again.

But in advance in the image below is the new design.
Personally, I think the old one is better, maybe we also need to change the icon of the cursor which now is different from the icon on the table drag.

🇧🇷Brazil joaopauloc.dev

Are we going to keep the drag icons? I think they demonstrate very well the drag proposal and I think we don't have too many different options.
@nod_ do you have any comments about the changes?

🇧🇷Brazil joaopauloc.dev

patch #264(2784233-264-from-262.patch) worked for me in D 10.2.4.
thanks

🇧🇷Brazil joaopauloc.dev

Fixed and already merged into 1.0.0-rc1.
Thanks @mortona2k.

🇧🇷Brazil joaopauloc.dev

I could confirm that the issue was fixed after apply the patch.

Before the patch:

After the patch applied:

Thanks.

🇧🇷Brazil joaopauloc.dev

I did a quick research for drag icons looking for other options but didn't find any different.

Google Material:

Bootstrap icons:

🇧🇷Brazil joaopauloc.dev

Hello @rkoller.

re #99 A: Both details were fixed.

re #99 B: First and second points fixed. now the text Show filtered and Hide filtered is based on the "button" status.

My next steps:

  • Work in more unit tests to cover all new features added by the issue, it's missing some scenarios
  • Work on the remaining tasks by priority
🇧🇷Brazil joaopauloc.dev

This is not the issue that made the test fail @sijumpk.
I commented the some lines on my last commit that broke the feature but I was trying to identify why some tests are failing.
I already tried this option to resize the window locally without the comment work and didn't work.
But I finally found the issue and will commit the fix to the test.

🇧🇷Brazil joaopauloc.dev

Adding two patches as examples of how we could change the tabledrag settings for the core display the vertical icon and for the field_group display the vertical/horizontal icon.

Field group example.

Core.

🇧🇷Brazil joaopauloc.dev

Hey @nod_, the main problem with this approach it's the Manager form display and Manager fields show the vertical/horizontal icon being displayed but the user can only drag on vertically.
I would like to suggest to change the change EntityDisplaysFormBase to work in a different way than the parent works. Then the icon will be properly.
Also, we could create an issue(feature request I think) on the field group module to change the EntityDisplaysFormBase to work as they need.
What do you think?

🇧🇷Brazil joaopauloc.dev

Hello @nod_.

Regarding your comment #36. I was looking for some way to do that.
I couldn't identify a way to make this dynamically for tables that we have h/v dragging. In the case comment on Field form settings, I tried to use other tabledrag settings but wasn't enough.

I checked out the class tabledrag-leaf that makes the drag h/v work added by the form EntityDisplayFormBase. Since this setting is added on the base form, I couldn't identify a way to make this work without any change by the contrib modules like field_group.

I will spend more time this weekend on this issue but I guess without any change on the contribe module will not be possible.

Another possibility could be to keep the form display with the h/v drag icon. As you mentioned in your comment #36 the settings are hierarchical.

🇧🇷Brazil joaopauloc.dev

Thanks for the suggestions @nod_. I didn't know about the indentEnabled setting, much better.
I removed the old setting of the forms.
I also made a list of all the places where we already noticed that the icon should be changed.

Without user configuration.
admin/structure/block
admin/config/content/formats
admin/config/content/formats/manage/basic_html?destination=/admin/config/content/formats
admin/config/media/image-styles/manage/large?destination=/admin/config/media/image-styles
admin/config/regional/language/detection
admin/config/regional/language
admin/config/search/pages
admin/config/user-interface/shortcut/manage/default/customize
admin/structure/taxonomy
admin/people/roles

Needs quick settings.

Field settings default values.
Create a field of the ListItem type add more than one default value and save, you should able to order the default values.

Views
Go to any views and try these.
rearrange fields.
rearrange filter in views.

Workflow
Enable workflow and content moderation modules and go to
/admin/config/workflow/workflows/manage/editorial?destination=/admin/config/workflow/workflows

Regarding the manager form display and manager display I'm working on that.

🇧🇷Brazil joaopauloc.dev

Fixed both pages.
I think we need to add a change record since we changed the table drag plugin to support both icons.
For other modules or even in a core feature to use the vertical icon just need to add the following attribute on the table structure like the example below.

    '#type' => 'table',
    // For filter.admin.js
    '#attributes' => [
      'id' => 'filter-order',
      'data-drag-orientation' => 'drag-y',
    ],

By default, the table drag will use the horizontal/vertical drag icon.
But we need to update the documentation too. But I don't know how to do that.

🇧🇷Brazil joaopauloc.dev

Hello @rkoller and @ckrina.
I have one question about the new design of the input filter and go-to elements.
We also have the case when the user types something that there is no result and we display a message like

There are no blocks matching the filter conditions.

Should we move this message to the left as the link to go to the element was? like the image below.

🇧🇷Brazil joaopauloc.dev

Good catch @rkoller.
In my opinion, we could follow it as a new follow-up issue and just set it as a related issue. Then we can move forward with this one.
Also, as you mentioned is not related to the changes of this issue. I think the changes in this issue make evident the issue that you found.

🇧🇷Brazil joaopauloc.dev

Hello @rkoller.
I implemented the suggestion mentioned above and I thought both titles fit well. As you can see in the images below.
Move any direction title example.

Change order example.

🇧🇷Brazil joaopauloc.dev

Hello folks, I updated the feature implementing the suggestions above.

A) I moved the link to go to the items found to the right of the input element as in the image above.
B) I implemented the new Show filtered button following the first example on B first image.

Follow the image below as an example of the changes.

Thanks for the feedback and the suggestions.

🇧🇷Brazil joaopauloc.dev

Hello, @rkoller.
I fixed the icon on the allowed values that you mentioned. Any update regarding the icon title?

🇧🇷Brazil joaopauloc.dev

Hello, @rkoller.
I fixed both pages that don't have the icon correct.

If you think that is necessary to update the title of the icon I can work on that.

🇧🇷Brazil joaopauloc.dev

Hello @saschaeggi, thanks.
Good point. I tested here because I did remember the field_group and this was the result.

But in my opinion, this is something that should be fixed on the field_group module.
When this issue is merged I can work on a particular issue in the field_group module. The way that we are adding this option to choose the icon drag will be easy to fix on the field_group.

🇧🇷Brazil joaopauloc.dev

Updated the implementation to show the warning message before and after the content is deleted.
Missing unit tests and fixing probably unit test issues.

🇧🇷Brazil joaopauloc.dev

Hey @dalin, I could confirm that only the menu item of the content was deleted.
Working on the issue...

🇧🇷Brazil joaopauloc.dev

Thank you @dalin for the feedback.

I'll adjust the menu to warn only on the criteria that you mentioned. Also, fix the getQuestion things that you mentioned.

Regarding the menu item also been deleted I'll confirm one more time but I'm almost sure that was deleted too.

I'll work on this issue until this weekend and let you know.
thanks.

🇧🇷Brazil joaopauloc.dev

Tested the new mask option and works well.
Follow the evidence.



🇧🇷Brazil joaopauloc.dev

Hi folks, I have two questions.

First, the warning message will be displayed only for a node that has a menu item as a parent? or should we warn users since the content has any kind of reference in menus? If yes I need to update the implementation.

In Drupal 11 the menu link is also deleted if the content is deleted. We need to update the message, I thought something like

This page has 3 menu children. These menu links will be deleted too if you continue.

Thanks.

🇧🇷Brazil joaopauloc.dev

Good catch @rkoller, i'll update this form too. Do you remember any other place that we should update this icon?

🇧🇷Brazil joaopauloc.dev

This issue was fixed on the current 11-x.
Follow the test below.

Not sure if I'm updating correctly but we don't need to fix the one anymore.
Thanks.

🇧🇷Brazil joaopauloc.dev

Hi all, thanks for testing.
The drag icon for views was fixed.

🇧🇷Brazil joaopauloc.dev

Thanks again @rkoller for the feedback.

#86 - 1. Working on that.

#86 - 2. Make sense, i'm gonna search for a button on drupal that has this style as "pressed" and add this style on the button.

#86 - 3. I'll try to increase the font and add here the style and we can check which size is better.

#86 - 4. Agreed, i think 1 block filtered sounds better than 1 block hidden since we are filtering the blocks. What do you think?

🇧🇷Brazil joaopauloc.dev

Hello, @rkoller.

I agree with your first topic. I updated the implementation to not display all blocks on the region where we drop a block or change the block region by using the select.

The second one about the highlights I didn't change anything, in the next few days I'm gonna check that.

The third one. I used the drag again jumping the row filter status, and feels good. I agree with you that dragging a block between the header and the filter status is confusing. So, we can forget about my suggestion on #82.

Last but not least, I fixed the message below the filter input when we didn't have blocks on the filter criteria.

Follow a quick demonstration of the changes.

🇧🇷Brazil joaopauloc.dev

Hello, a quick update of my last comment #81 regarding the #80 2 item.

I could implement the same behavior when we are dragging an item and the next or previous item is a region name. Now the drag item jumps the "filtered count items" message too.

But, I didn't enjoy the usability of this feature, as we already jumped the region header, jumping two rows makes the usability a little weird.
As I mentioned before, I agree that does not make sense for the row that displays the filtered items "be draggable". But now, after I implemented and used the drag feature the experience was not good. This got worse when we needed to move the row to the top or bottom that had not been displayed, was definitely a bad experience. But this is my opinion, I'll appreciate any feedback.

Now, regarding the issue above, I would like to suggest another option to display how many blocks we have filtered or not.
Here is the layout that I thought could be useful to show the blocks hidden or hide all blocks on the region. In this option, I think we kept the current experience of dragging the blocks and also display the filtered information needed.

What do you guys think?

🇧🇷Brazil joaopauloc.dev

Thanks for the quick feedback @rkoller.

#80 1: Totally agree, both methods to move the block must have the same behavior. This was a side effect of the last changes but will be fix on the next commit. Both blocks moved when dropped into the new region will open all blocks filtered if have, this was our last behavior until the changes. I'll wait for new comments if we need to change this behavior as your comment about the negatives and positives regarding this behavior.

#80 2: This change will be a difficult one because the instance of the table drag of this page is handled by another javascript file that is different of the filter page. Drupal table drag does not provide custom events to make this change easy. But I'll try to change this. I agree that the result message filter working as a draggable item is quite strange.

#80 3: Nice suggestions, but I have one question. In my last change, I added this highlight into the rows when the user clicks on the link to go to the element. I think we could add these highlight rows when the user types anything on the filter, no matter if the user clicks on the link or opens all blocks on the region. What do you think?

#80 4: Before those last changes, the height of the fieldset was higher than all other fields that we have this kind of filter, for example, the filter element that we have on the modules page, but now, the height is the same. I think we could follow this height in every filter. However, I noticed that the distance of the filter to the table is larger than the other filters. I'll fix that. Regarding the behavior of clicking many times on the link, I'll fix that too.

One more time, thank you so much for the great feedback and suggestions @rkoller, appreciate that. In the next few days, I'll commit all those changes above and let you know.

Thanks.

🇧🇷Brazil joaopauloc.dev

Hello, @rkoller regarding the issue that you mentioned that is back. I couldn't reproduce it.
Follow the new button behavior.

Also, the new Go to items found. with rows highlighted.

There are a few issues to be fixed but regarding the issues that you mentioned, I think we got some progress.

Thanks.

🇧🇷Brazil joaopauloc.dev

Hello folks, follow some updates.

1: The button on the regions to show/hide all blocks on the region was updated to appear only after any filter was type, also appears only if the region has block filtered.

2: The width of the fieldset input filter getting the entire screen when the user scrolls the page was fixed.

3: The table header sticks below the input filter when the user scrolls the page.

4: The button to hide/show the blocks can also be reached by tab.

5: Finally, when the user clicks on Go to items found. the blocks that match with the filter applied get highlighted by a background color-success, the same behavior when we add a new block on the page.

🇧🇷Brazil joaopauloc.dev

Thanks for the patch @godotislate, nice solution, good job.
I could reproduce the issue.

Running the patch with only the test to see tests failing.

🇧🇷Brazil joaopauloc.dev

Hello folks, I implemented more details about the suggestions/comments that you discussed.

First of all, after the last update you gonna see that the table drag icon added in the previous changes was removed since we have a proper issue to fix that and I also work there Differentiate visually dragging with and without hierarchy Needs work .

Secondly, I implemented the layout proposed by @rkoller on suggestion #62-2. This layout is much better than the first one that I proposed. Take a look at the image below.

Regarding the feature to jump to. I removed the link(change filter) that goes to the filter and I followed the suggestion that you guys mentioned

let behave similar to the bulk actions footer bar on /admin/content

. But I have just a concern about the filter block feature. I think the main proposal of this issue, as I understood, is to make a way to find a block on this page easily. Since we started to not hide the regions that do not have visible blocks, my feeling is that it might still pose a slight challenge. What do you think? The jump to functionally makes things easy, I think. That's why I kept the jump to working together with the new filter behavior. I'll wait for any feedback about it. If we want to remove this feature, we could discuss another easier way to find a block on this page.

Just one question about the new button to show/hide blocks on the region, if the filter is empty. How could this button work? Does it make sense to work alone or we can, for example, add a disabled style or even not display it when we are not filtering?

Best.

🇧🇷Brazil joaopauloc.dev

Recently I was working on Allow to JS-filter blocks in regions on the block layout administration page Allow to JS-filter blocks in regions on the block layout administration page Needs work and one of the comments/suggestions was to change this drag icon since in that page we drag only vertically. I added the support there to change this icon based on attribute data.

But I didn't know about this issue, so we decided to remove this feature of and try to do this work here.

I used the drag icon supported by the browsers, but I saw that in the Proposed solution there are some suggestions. I would like to propose that we use the icon displayed by the browsers when we are dragging vertically. So, we can follow a pattern of icons instead of creating a new one, even if the icon proposed is very similar. The icon that I added is not the same as the browser(row-resize), I'll update that soon.

Follow two examples where I change the icon.

and

Thanks.

🇧🇷Brazil joaopauloc.dev

Thanks for the feedback @rkoller, appreciate that.

#62-1-a: Agreed, I'll fix it.

#62-1-b: Agreed, also when I was testing the clear button on other pages, I noticed that the button clears the text on the input, but does not trigger the filter, so the input is cleaned but doesn't clear the results. Maybe we should open an issue to fix that together with button style.

#62-2: I think I didn't understand well, your suggestion to add the eye button and the link to filter in another row only when there are no blocks in that region? or move these two buttons to another row having or not having blocks in that region? if yes, I think is a good idea, I can change that. But if we add the new row below the region with a button only if there are no blocks can be confused I think.

#62-3: Got it, I'll fix it.

#63-A: No, all drag-and-drop table that knows in Drupal admin use the same icon, I got this icon from CSS, when we set the mouse cursor to row-resize. But I think we should remove this style from the changes of this issue and move to this one Differentiate visually dragging with and without hierarchy Needs work , also the @simohell provided some icons to different drag in this issue that we can use.

I'm waiting for new comments after the meeting that you guys will discuss this issue.
Thanks in advance.

🇧🇷Brazil joaopauloc.dev

Hey folks, thanks for the comments.

Regarding the suggestions on #62.

  1. I added the clear widget to clear
  2. In my first approach I didn't hide the regions, but I felt a little confused about finding the block that I just searched for, because there was a little problem with keeping all regions displayed, if the block that I searched displayed on the bottom, the feature to filter the blocks doesn't make much sense, because I still not seeing the block. But okay, I agree that hidden regions cause more confusion to drag blocks or change their position. So, I back the code to display the regions but I added something to help us reach the blocks that we are searching for, Below the input filter I added a link for the user to click and scroll to the first region where a block was found. With this feature, it makes it easier to find a block that we are searching on the bottom page., what do you think about this?
  3. I didn't add the checkbox but would like to suggest that we use a button with an eye icon to control the visibility of the blocks by region that overrides the filter param. Also, besides the eye icon, there is a link for the user to go back to the filter. What do you think?

Regarding the suggestions on #63.
2: I'll work on that.
3: fixed.
4: I could reproduce this issue on a clear d11 side, so I think is not related or is a side effect of this issue.
5) I have to check that.
A) I fixed the table drag icon in this issue, I just forgot about this comment mentioning that there is another issue.
b) I have to check that.

Clear widget and the link below to go to the next region.

The button/eye icon to control the visibility of the blocks by region.

Waiting for comments regarding the changes and I'll work on the unit test for the new changes.
thanks

🇧🇷Brazil joaopauloc.dev

Fixed the unit test that was failing.

Production build 0.69.0 2024