Conversation
…equest Wicket had no built-in way to stop the user from interacting with a page while an Ajax request was running. An activity indicator shows that something is happening, but a second click on a slow button still sent the request again, and a click elsewhere could act on markup the pending response was about to replace. AjaxDisableComponentListener covers only the component that fired the request, so applications wrote their own veil on top of the global Ajax topics. See GitHub issue #1631. wicket-extensions now has a package org.apache.wicket.extensions.ajax.veil with two behaviors sharing the base class AbstractVeilBehavior: - PageVeilBehavior, added to a page, veils the whole page during every Ajax request fired from it. Adding it to anything but a Page throws IllegalArgumentException. - LocalVeilBehavior, added to any component, veils only that component, and only during requests fired from it or from a component nested in it; such a request does not veil the page. Local veils nest: the innermost one takes the request, each with its own timings. The veil goes up on /ajax/call/beforeSend and comes down on /ajax/call/done, so it also lifts on failure. It is transparent and swallows clicks, so a fast request does not make the page flash. After 300 ms it gets the class wicket-veil-busy, which dims the region and shows a CSS spinner that then stays for at least 500 ms so a response arriving just after it does not make it flicker. Both timings are settable per behavior. Concurrent requests share one veil, and the keyboard is not intercepted. A request opts out with PageVeilBehavior.noVeil(attributes), which adds the extra parameter wicket_nb; the page veil and every local veil leave it alone. Background requests (timers, lazy loading panels) are veiled unless they opt out. A component updated by a WebSocket push is recomputed with no Ajax request the browser could notice, so the server raises its veil: LocalVeilBehavior.getVeilMessage() is a text message to send through the page's WebSocket connection when the work starts, unveil(handler) lowers the veil with the pushed update, and getUnveilMessage() lowers it when the work fails. wicket-extensions does not depend on the WebSocket module; the client listens on /websocket/message. wicket-veil.js subscribes to the global Ajax topics, so it works with both the jQuery-based and the plain JavaScript Ajax engine, and nothing changes in wicket-core. This is new API only: nothing is changed or removed, and an application that does not add one of the behaviors sees no difference, so no migration is needed. Also adds the ajax/veil and websockets/veil examples, a user guide section in the Ajax chapter, Java tests for the behaviors, QUnit tests for Wicket.Veil run against both engines, and Selenium tests driving ajax/veil in headless Chrome. The Selenium tests need Chrome and run only with -Dwicket.selenium=true; JAVASCRIPTTESTING.md says how.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1634 +/- ##
============================================
+ Coverage 61.91% 61.95% +0.03%
- Complexity 11256 11281 +25
============================================
Files 1247 1250 +3
Lines 48375 48429 +54
Branches 6789 6792 +3
============================================
+ Hits 29951 30003 +52
- Misses 15726 15729 +3
+ Partials 2698 2697 -1 🚀 New features to boost your workflow:
|
veil-ajax.mp4veil-websocket.mp4 |
papegaaij
left a comment
There was a problem hiding this comment.
A review of the veil client code, its CSS and the options script. The first three comments are cases where the veil comes down while it should still be blocking; the CSS ones are about local veils in scrolling, positioned or layered hosts.
…irects Review of the veil behaviors (GitHub issue #1631) found cases where the veil did not hold, or disturbed the page it covers: - A host replaced while still veiled, by a pushed progress update or by a request on another channel, lost its veil until the request ended. The veil now listens to /dom/node/added and moves onto the new element at once. - On an Ajax redirect /ajax/call/done still fired, so the veil came down and a second click could send the request again before the next page arrived. Core keeps its activity indicator up in that case, but done subscribers could not tell. Both Ajax engines now pass whether the call is redirecting as an extra argument of /ajax/call/done, and the veil stays up until the page is left. It is lowered when the page is restored from the back-forward cache. - Wicket.Veil.hide() shared one counter with Ajax requests, so a hide whose show never arrived lowered the veil of a running request. Server raises are counted on their own now. - The veil was chosen from attrs.c, which for a delegated behavior is the container, not the element clicked. It now starts from the event target, as core does, and falls back to attrs.c when that element is gone. - Local veils were kept in a plain object, so a markup id such as "constructor" resolved to an Object member. They are kept in a Map. - In a scrolling host the veil scrolled away with the content. It is now kept over the visible part while the host scrolls. - The host was forced to position: relative, so an absolute, fixed or sticky host jumped on every request. Only a static host gets it now, through the new class wicket-veil-host-static. wicket-veil-host isolates the host instead, so the veil no longer paints over a sticky header or a modal dialog in front of it. - The options script was formatted with the default locale, which can localize digits and break the script. It uses Locale.ROOT. An application subscribing to /ajax/call/done sees no difference unless its callback reads the new argument, a boolean that follows attrs. Wicket.Ajax.Call.done() takes it as an optional second parameter.
theVeilSwallowsClicks expected WebElement.click() on a veiled link to throw ElementClickInterceptedException. Current ChromeDriver does not throw: it waits until nothing covers the element any more, then clicks it. The click therefore landed after the veil came down, sent a second request, and the test failed on both engines although the veil does swallow clicks. It only showed when the test was run, which needs -Dwicket.selenium=true. The test now clicks at the link's position with the Actions API, as a user would, and checks that the click had no effect.
The previous commit kept the veil up when the response redirected the browser, and made both Ajax engines pass whether the call is redirecting to /ajax/call/done for that. But a redirect does not always leave the page: a redirect to a download, a mailto: link or a URL answering 204, or a navigation the user cancels at a beforeunload prompt, all keep the page, and the browser fires no event to tell. The veil then never came down, and with a PageVeilBehavior on a base page the whole page stayed blocked until it was reloaded. No timeout can tell these cases from a slow page load. The slow part the veil is there for, the server's work, is covered either way, and the window between the response and the next page is the one every Wicket application already has. The veil comes down at /ajax/call/done for a redirect too, as before, and the change to the Ajax engines, its tests and its documentation are reverted, so the veil needs nothing from core. See GitHub issue #1631.
The review of the veil behaviors found cases where the veil did not hold or disturbed the page it covers, but the examples showed none of them, so the fixes could not be seen or tried out. ajax/veil gets a section for each case an Ajax request can show: one behavior on a list handling the clicks of its rows through a child selector, with only the clicked row veiled; a panel whose markup id is "constructor"; a scrolling box whose veil covers its visible part; an absolutely positioned card that stays in place while veiled; and a panel under a sticky header, whose veil stays behind the header. VeilPageSeleniumTest drives each of them in headless Chrome, with both Ajax engines. websockets/veil pushes a progress update halfway through its long rounds, which redraws the veiled panel while the veil stays on it, and gets a link that sends a slow Ajax request from the panel while the server sends an unveil message no veil message preceded; the panel stays veiled until the request is done. A host replaced by another Ajax request while it is veiled is shown through the WebSocket push only: requests to the same page are serialized by the page lock, so a second Ajax request cannot re-render the host while the first one runs. See GitHub issue #1631.
|
@papegaaij thanks for the review. d531d49 adds the cases from your comments to the examples, so each fix can be tried out:
|
|
@papegaaij Many thanls for your review and aproval. Groetjes uit Panama |
Closes #1631
Adds two behaviors in a new package
org.apache.wicket.extensions.ajax.veil(wicket-extensions) that stop the user from interacting with the page, or with one component, while an Ajax request is running.PageVeilBehavior: added to a page, it veils the whole page during every Ajax request fired from it.LocalVeilBehavior: added to any component, it veils only that component, during requests fired from it or from a component nested in it. Local veils nest, and the innermost one takes the request.The veil goes up on
/ajax/call/beforeSendand comes down on/ajax/call/done. It is transparent at first and swallows clicks. After 300 ms it dims the region and shows a CSS spinner, which then stays for at least 500 ms. Both timings are settable per behavior. A request opts out withPageVeilBehavior.noVeil(attributes).For a component updated by a WebSocket push, the server raises the veil with
LocalVeilBehavior.getVeilMessage()and lowers it withunveil(handler)orgetUnveilMessage(). wicket-extensions does not depend on the WebSocket module.wicket-veil.jsworks with both the jQuery-based and the plain JavaScript Ajax engine. Nothing changes in wicket-core.Migration
New API only, nothing changed or removed, so no migration is needed. An application that does not add one of the behaviors sees no difference.
Also included
ajax/veil(page veil, opting out, nested local veils with their own timings) andwebsockets/veil(a background task that veils a counter panel and pushes its updates).VeilBehaviorTest, QUnit tests forWicket.Veilagainst both engines, andVeilPageSeleniumTestdrivingajax/veilin headless Chrome. The Selenium tests need Chrome and run only with-Dwicket.selenium=true, as described inJAVASCRIPTTESTING.md.