Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add some flag that allows to block users selecting an option while a … #350

Open
wants to merge 2 commits into
base: wicket9.x
Choose a base branch
from

Conversation

ernestosemedy
Copy link
Contributor

@ernestosemedy ernestosemedy commented Oct 13, 2022

add some flag that allows to block users selecting an option while a query is in process

@sebfz1 this PR is based on some changes I did for our project. We will soon test this in our pre-production environment and see if that fixes the issue we are experiencing there. The issue being:

  1. Users select something at client side
  2. Wrong selection is done in server because user selected sate data in client and a request was sent to server side that didn't return items yet.

This PR tries to prevent the user from being able to select something while there is a request to server. I have added this via a flag set to false so that previous behavior is set to be default

@ernestosemedy ernestosemedy force-pushed the feature/reiern70/cancel-select-when-query-is-inprocess branch from e0a9ac7 to f9fe369 Compare October 14, 2022 01:06
Copy link
Collaborator

@martin-g martin-g left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that the new logic mixes Wicket Ajax calls (Wicket.Ajax.ajax(...)) and jQuery ones ($.ajax(...)). Was this the case until now ?

If the logic uses only Wicket Ajax JS APIs then I'd recommend trying to implement the same logic with AjaxChannel.ACTIVE. I.e. use the same channel name of select and fetch calls and they will be auto-cancelled if there is an actively running XHR request in this channel.

super.onConfigure(component);
if (preventSelectWhileQueryIsRunning) {
// we define special functions in for handling select and fetch data
this.setOption("select", "function(request, response) {\n" + getVarName() + ".select(request, response); \n}");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not familiar with WJQUI and jQueryUI JS API but the above line looks suspicious to me. The JS code does not look like it prevents the selection. How does this work exactly ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just delegates into function defined in prototype.

}

private String getVarName() {
return "window.aut_" + getMarkupId();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aut_ is a bit vague. Maybe autoCmpltTxtFld_ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do this.

@ernestosemedy
Copy link
Contributor Author

I see that the new logic mixes Wicket Ajax calls (Wicket.Ajax.ajax(...)) and jQuery ones ($.ajax(...)). Was this the case until now ?

Yes. See explanation bellow.

If the logic uses only Wicket Ajax JS APIs then I'd recommend trying to implement the same logic with AjaxChannel.ACTIVE. I.e. use the same channel name of select and fetch calls and they will be auto-cancelled if there is an actively running XHR request in this channel.

We have 2 operations here:

1- Fetching the data to show in drop down. By default we are passing the URL of behavior that creates the JSON data then

https://github.com/jquery/jquery-ui/blob/main/ui/widgets/autocomplete.js#L381

This is done above in

image

and this does not uses wicket AJAX. Thus I juts followed the same approach

2- The part doing the selection of element. This is done using wicket AJAX generated by Java code. I just translated this into low level JS.

@martin-g
Copy link
Collaborator

If there are both Wicket Ajax and jQuery.ajax() calls then I guess your approach is better!

@mdbergmann
Copy link

I see that the new logic mixes Wicket Ajax calls (Wicket.Ajax.ajax(...)) and jQuery ones ($.ajax(...)). Was this the case until now ?

If the logic uses only Wicket Ajax JS APIs then I'd recommend trying to implement the same logic with AjaxChannel.ACTIVE. I.e. use the same channel name of select and fetch calls and they will be auto-cancelled if there is an actively running XHR request in this channel.

Wondering if this is responsible for the Ajax related issues we've seen since Wicket 9 in wicket-jquery-ui (or Kendo actually).

@ernestosemedy
Copy link
Contributor Author

I see that the new logic mixes Wicket Ajax calls (Wicket.Ajax.ajax(...)) and jQuery ones ($.ajax(...)). Was this the case until now ?
If the logic uses only Wicket Ajax JS APIs then I'd recommend trying to implement the same logic with AjaxChannel.ACTIVE. I.e. use the same channel name of select and fetch calls and they will be auto-cancelled if there is an actively running XHR request in this channel.

Wondering if this is responsible for the Ajax related issues we've seen since Wicket 9 in wicket-jquery-ui (or Kendo actually).

Can you describe them...?

@mdbergmann
Copy link

I see that the new logic mixes Wicket Ajax calls (Wicket.Ajax.ajax(...)) and jQuery ones ($.ajax(...)). Was this the case until now ?
If the logic uses only Wicket Ajax JS APIs then I'd recommend trying to implement the same logic with AjaxChannel.ACTIVE. I.e. use the same channel name of select and fetch calls and they will be auto-cancelled if there is an actively running XHR request in this channel.

Wondering if this is responsible for the Ajax related issues we've seen since Wicket 9 in wicket-jquery-ui (or Kendo actually).

Can you describe them...?

I.e. this: #333

@ernestosemedy
Copy link
Contributor Author

If there are both Wicket Ajax and jQuery.ajax() calls then I guess your approach is better!

Yes. See explanation I gave above. So far component used:

1- JQuey Ajax to fetch data (non blocking): as we were passing URL of behavior producing JSON
2- Wicket AJAX to transfer client side selection to server side.

We will soon deploy some version of the above into our internal "production" environment were people can consistently reproduce the issue thus we will have confirmation if this code improves the situation

@ernestosemedy
Copy link
Contributor Author

I see that the new logic mixes Wicket Ajax calls (Wicket.Ajax.ajax(...)) and jQuery ones ($.ajax(...)). Was this the case until now ?
If the logic uses only Wicket Ajax JS APIs then I'd recommend trying to implement the same logic with AjaxChannel.ACTIVE. I.e. use the same channel name of select and fetch calls and they will be auto-cancelled if there is an actively running XHR request in this channel.

Wondering if this is responsible for the Ajax related issues we've seen since Wicket 9 in wicket-jquery-ui (or Kendo actually).

Can you describe them...?

I.e. this: #333

I do not see an immediate connection but I don't know what the other issue is about. If I have some time I might try to look at it but we are not using Kendo UI and I have 0 experience with that part of the code base

@mdbergmann
Copy link

I see that the new logic mixes Wicket Ajax calls (Wicket.Ajax.ajax(...)) and jQuery ones ($.ajax(...)). Was this the case until now ?
If the logic uses only Wicket Ajax JS APIs then I'd recommend trying to implement the same logic with AjaxChannel.ACTIVE. I.e. use the same channel name of select and fetch calls and they will be auto-cancelled if there is an actively running XHR request in this channel.

Wondering if this is responsible for the Ajax related issues we've seen since Wicket 9 in wicket-jquery-ui (or Kendo actually).

Can you describe them...?

I.e. this: #333

I do not see an immediate connection but I don't know what the other issue is about. If I have some time I might try to look at it but we are not using Kendo UI and I have 0 experience with that part of the code base

Maybe it's completely unrelated. I dunno. Then please forget my interruption.

…e client and server side. This also would allow to not need to keep in memory the choices.
@ernestosemedy ernestosemedy force-pushed the feature/reiern70/cancel-select-when-query-is-inprocess branch from 994e86d to c266cf7 Compare November 8, 2022 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants