-
Notifications
You must be signed in to change notification settings - Fork 397
WICKET-6055 Made AjaxLazyLoadPanel non-blocking #151
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
Conversation
When having multiple AjaxLazyLoadPanels on your page, they all block their Wicket request thread until the content is ready to load. This can be problematic when you try to wait for some background job to finish and want to poll for that job to be ready, and only then update the contents. The improvement would be to add a method that gives the developer the option to not update just yet (isReadyForReplacement) and when it returns true, start the replacement. By default this would return true, implementing the current behavior of the AjaxLazyLoadPanel. Furthermore to improve the responsiveness of the ALLP it should add a single timer to the page that can be used by multiple ALLPs to update themselves. The timer would poll each second and the ALLPs would use Wicket's event bus to update themselves. With some reference counting, the timer can remove itself from the page when all ALLPs have updated themselves. This enables refreshing the page as well when outside an AJAX context, or having a user be impatient and pressing F5. Fixes WICKET-6055
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be static to prevent a dangling reference to the surrounding component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would not pose a problem and make the code simpler. With static I have to get the component's page and ran into a order problem of removing the behavior from the page and trying to change the page's meta data. The dangling reference is only a problem when you have memory constraints, but then the lazyloadpanel's reference would not be your biggest problem. Regardless I've changed it to be static.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Logger instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oopsie, this one should've been removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't #onEvent() check for a specific event? If other events are sent through the component hierarchy (e.g. when any other Ajax request is processed), the lazy component might be replaced prematurely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, as isReadyForReplacement() is actually there for preventing replacement prematurely. Why would you wait until some time in the future to do the replacement when you can replace now and the component is ready for replacement?
|
@dashorst What is the status here? Can I help somehow ? |
|
@dashorst Ping! |
|
Any comments about configuring the duration of the polling timer? I'm inclined to just defer that to a specific user request rather than implement it directly. Perhaps someone comes up with a better solution to configure the duration than what we have now. I'd rather wait for the need and a good solution than implement a solution that paints us in the corner. |
|
I've wrapped my ideas about this topic in #240 |
When having multiple AjaxLazyLoadPanels on your page, they all block
their Wicket request thread until the content is ready to load. This
can be problematic when you try to wait for some background job to
finish and want to poll for that job to be ready, and only then update
the contents.
The improvement would be to add a method that gives the developer the
option to not update just yet (isReadyForReplacement) and when it
returns true, start the replacement. By default this would return true,
implementing the current behavior of the AjaxLazyLoadPanel.
Furthermore to improve the responsiveness of the ALLP it should add a
single timer to the page that can be used by multiple ALLPs to update
themselves. The timer would poll each second and the ALLPs would use
Wicket's event bus to update themselves. With some reference counting,
the timer can remove itself from the page when all ALLPs have updated
themselves.
This enables refreshing the page as well when outside an AJAX context,
or having a user be impatient and pressing F5.
Fixes WICKET-6055