🇧🇷Brazil @joaopauloc.dev

Attaching the patch.

Hi folks.
First of all, great module, save-me a lot of time.
I pushed the changes that I suggested.

What do you guys think?

joaopauloc.dev → created an issue.

+1 for RTBC
Worked well for me too.
Core version 10.4.1
PHP version: 8.3
Field Group version: 3.6.0

Hi folks, as @michaellander mentioned I also think this one can benefit many modules or some menu customization.
Following the same idea of ✨ Allow modules to hook into top of content/footer sections of new core navigation Active I've replicated the idea for the footer top section.

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

Hey @plopesc.

I was investigating how the contextual module adds the Edit button and the contextual link on the pages.
The Edit is added by the hook toolbar(ContextualHooks:19), by default this link is hidden, and then a javascript is responsible for showing or keeping it hidden. If I'm not wrong, the behavior checks if there is any contextual link on the page, and if the Edit is toggle to visible, otherwise keeps hidden. The contextual_preprocess function adds the contextual links if I'm not wrong.
Please, anyone, correct me if my understanding is wrong.

Based on that, for the entities we already have a solution, since the user is accessing an entity page the local tasks will appear and we add the Preview editable areas. Since this is a temporary solution, I don't think it's feasible to implement a "complexity" custom logic just to make this work for other pages. We probably need a javascript code to check if there are contextual links on the page and then control the top bar items. We probably will add a custom logic just for a temporary solution.

So, here is my suggestion. Based on the "complexity" that can be to adding the Preview editable area for non-entities pages, and this is a temporary solution. I would like to suggest that we could move forward with the current solution, displaying the feature only for entity pages.

What do you guys think?

Thanks for report this issue @igorbiki.
I pushed the fix. Good catch.
I changed the round stuff to use the correct PHP function.
Thanks.

After |, the value must be a number, not a calculation expression like 0 * 3.

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

Hey @kpaxman, thanks for the report.
I pushed the fix to the issue.
I fixed the help text missing the {{ }} that needs to be added to the expression.
Regarding your example: Total is {{ :number|0 * 3 }}
This expression is not valid, you are using an expression in the default value that is not supported, you can defined any number as default value.
Like: Total is {{ :number|1 }}
Total is {{ :number|0 }}
Total is {{ :number|100 }}

Could please check if the fix works for you?

Thanks

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

Hey @pfrenssen, what do you think about changing the places field to be required only if the waitlist is checked?

Hey @plopesc, thanks for the review.
I'm having a look at the way that the Edit button appears. In the contextual module, they implement the hook toolbar. For the navigation we don't show that toolbar as the other modules do, so the only way that we currently display something similar to the toolbar hook is the top bar item, am I correct? please correct me if I'm wrong.
Since the PageActions plugin displays the Local tasks, how can we implement the edit button without using the local tasks? Should we create another plugin, for example: ToolbarActions, then we could add actions that are similar to the toolbar hook.
Thanks.

Hi @benjifisher and @rkoller, thanks for reviewing this issue.

These are my suggestions for the points Suggest changing the parent, Special treatment when deleting custom menu links.
For the Suggest changing the parent

The link will be opened in another tab. What do you guys think?

Regarding the Special treatment when deleting custom menu links
I changed the text message as you guys suggested.

Hi folks.
I updated the issue summary adding a screenshot with the User interface changes as recommended in comment #37.
Also, when I tested the MR I found the following issue.

Finally, update the modal message with the comments suggested in comment #37. Regarding this message, I would like to suggest another one when there is only one item, for the singular message we could use something like:
Deleting this @singular_label will move its child menu item to the top level:
What do you guys think?

Hey @pfrenssen, I'll push the changes.

I'm attaching the patch with the implementation of @carolpettirossi suggestions.

I am attaching the patch with the first proposal.
There are some scenarios for moving registrants from the waitlist that can be tricky to find a good solution.

For example, there are two users on the waitlist. The first one requested three spaces, and the second registrant requested just one. Then, one registrant is removed from the event that updates one space. What should be the behavior in this case?
Should we wait until more space is available for the first registrant?
Or should we find the first registrant on the waitlist that matches the requested spaces with the currently available spaces?

Another point is, that if there is only one space and we have users on the waitlist requesting more than one space, should we block the registration until the waitlist user is promoted?

Hey @pfrenssen.
I noticed that the promoteFromWaitlist considers only one registrant to be promoted, the next one.
For this new scenario with seats, I wonder if we could consider changing this behavior.

For example: An event that has 10 seats and there are two registrants in the waitlist, the first one requesting 2 seats(Registrant A) and the second one 1 seat(Registrant B). Let's suppose that someone at an event with 5 seats is removed. Currently, only the first registrant with 2 seats is promoted(Registrant A), even with more spaces in the event for the second registrant in the waitlist (Registrant B).
Then, if another user accesses the page and tries to register the user will be registered, and the second user(Registrant B) on the waitlist is still in the waitlist.

So, what do you think if we update the promoteFromWaitlist function to iterate by the registrant while we have space on the event, instead of getting only the first one?

I did some tests in my local adding the following code in the last line of the function promoteFromWaitlist and worked.

$stillAvailable = $this->retrieveAvailability();
if ($stillAvailable > 0 && !empty($first_waitlist)) {
   $this->promoteFromWaitlist();
}

hey @pfrenssen.
I agree with you regarding the route stuff.
I tested your merge request, and it works.
Thanks.

joaopauloc.dev → created an issue.

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

Good catch @finnsky. I forgot the once + context stuff, thanks.
Thanks.

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

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

Hi folks, follow the image below of the preview editable areas.

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

joaopauloc.dev → created an issue.

Hi folks.
First of all, great module!!! good job.
This module saves me a lot of time.

With the fixed applied I was able to add the fields as expected.
Take a look at the image below.

joaopauloc.dev → created an issue.

I didn't push the webform changes. It was a local test. Whatever, I tested it again in a new instance, and worked like a charm.

Thank you @codebymikey. This feature will be available in the next release, 1.0.4. I'm working on other tickets including one of you who opened and we plan to launch everything together.

I couldn't reproduce it, it probably was fixed in the new release.

hey @codebymikey, thanks for your contribution, appreciate that.
but unfortunately, the changes didn't work in webforms.

I changed the field to_divide to use radios instead numbers but the field was not rendered as expected.

Follow the screenshot.
Field display:

Field settings.

joaopauloc.dev → changed the visibility of the branch 3464607-radio-support to hidden.

I tested the patch and the performance has been improved a lot.

Without the patch the script take 51.7ms to run.

With the patch became 1.4ms.

Nice!

Updating the title since the markup doesn't work only with multiple expressions in the markup content.

joaopauloc.dev → created an issue.

Hi @thachakrit.

Sorry for the late response.

How did you configured the cities fields? can you add a screenshot?
I just saw right now that we didn't add some import information in the help text to configure the params, sorry about that.
You need to specify that you are using the param from the webform adding something like the image below.

Also, I'm facing some issues with cache, so try disabling the cache at /admin/config/system/webform_remote_fields for a moment just to check.

Thanks.

Hello @codebymikey, thanks for your contribution.
I'm going to review the module to add the support that you mentioned.
I agree that would be nice to have the support for that.
Thanks.

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

Hi @ty10086, sorry by the any status update, but unfortunately I didn't have time yet.
i'll probably take a look next days.
feel free to text me on the slack, joaopauloc.dev it's my user on slack as well.

Hi @igor, despite the tests pass, I think I need more tests to merge this one into the release.
But let us know if patch fix your issue case.
Next days I'll take a look again and write more detailed unit tests to make sure that everything is working as expected.

Hi @ty10086
At this moment we don't have support to composable fields, but we will check to support this webform field as well.

patch #29 worked for me, Drupal 10.2.6m thanks!

Thanks @igorbiki for the quick update.
I could reproduce the issue. I'll take a look.

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.

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.

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

joaopauloc.dev → created an issue.

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.

lauriii → credited joaopauloc.dev → .

I tested patch #19 and works fine.

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' →

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

It's working for me too.

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

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

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.

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

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.

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.

joaopauloc.dev → created an issue.

Forgot the interdiff.

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.

I reverted the icon to the first one.

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

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.

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?

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

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

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

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

Before the patch:

After the patch applied:

Thanks.