papegaaij commented on code in PR #1634: URL: https://github.com/apache/wicket/pull/1634#discussion_r4166724258
########## wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/veil/wicket-veil.css: ########## @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +.wicket-veil { + position: fixed; + top: 0; + right: 0; + bottom: 0; + left: 0; + z-index: 10000; + background: transparent; + cursor: wait; +} + +.wicket-veil-host { + position: relative; +} + +.wicket-veil-host > .wicket-veil { Review Comment: In a scrollable host (`overflow: auto/scroll`) the local veil only covers the first screen of content: it is absolutely positioned inside the scroll container, so it scrolls away with the content. With the host scrolled 1000px down, the visible rows stay clickable and the spinner is out of view. I don't see a CSS-only fix while the veil lives inside the host: anything positioned in there scrolls with the content, `position: sticky` would take up layout space, and container query units would need size containment on the host. On the JS side, the veil could follow the visible area: ```js veil.style.top = host.scrollTop + 'px'; veil.style.height = host.clientHeight + 'px'; veil.style.bottom = 'auto'; ``` kept up to date from a `scroll` listener on the host while the veil is up (the wheel still scrolls the host through the veil), or be sized to `host.scrollHeight` so it covers all of the content, with the spinner then needing its own placement. Just an idea, not a requirement: a `popover="manual"` veil lives in the top layer, and with CSS anchor positioning on the host (`top: anchor(top); left: anchor(left); width: anchor-size(width); height: anchor-size(height)`) it covers the host's visible box regardless of scrolling, and leaves the host's `position` alone (see the next comment). The catch for local veils is that the top layer ignores stacking and clipping, so it would paint over a `ModalDialog` or a sticky header in front of the host, and anchor positioning is fairly new in Firefox. For the page veil, though, the top layer is exactly what you want: it is guaranteed to be above everything, which `z-index: 10000` is not. It needs `manual` (an `auto` popover light-dismisses on Esc or an outside click) and resets for the UA popover styles (`inset`, `margin`, `padding`, `border`, `background`, `overflow`). ########## wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/veil/wicket-veil.css: ########## @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +.wicket-veil { + position: fixed; + top: 0; + right: 0; + bottom: 0; + left: 0; + z-index: 10000; + background: transparent; + cursor: wait; +} + +.wicket-veil-host { Review Comment: `.wicket-veil-host` forces `position: relative` on the host for as long as the request runs. A host that is `position: absolute`, `fixed` or `sticky` through a rule of equal or lower specificity drops back into the normal flow on every request and returns when the veil lifts, so the layout jumps on each click. The `relative` is only there to make the host the containing block for the absolutely positioned veil, and `absolute`, `fixed` and `sticky` hosts already are one. Only a static host needs it, and CSS can't select on computed position, so this needs a small JS check next to the class (combined with the `isolation` from the comment below): ```css .wicket-veil-host { isolation: isolate; } .wicket-veil-host-static { position: relative; } .wicket-veil-host > .wicket-veil { position: absolute; z-index: 1000; } ``` ```js host.classList.add(HOST_CLASS); if (getComputedStyle(host).position === 'static') { host.classList.add(STATIC_HOST_CLASS); } ``` in `show()` and `reattach()`, with `hide()` removing both classes. The CSS-only ways to make a containing block without touching `position` all have worse side effects: `transform`/`will-change` also capture `position: fixed` descendants (a `ModalDialog` inside the panel would jump into it), and `contain: layout` stops margins collapsing through the host. ########## wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/veil/wicket-veil.js: ########## @@ -0,0 +1,324 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Veils the page, or a single component, while Ajax requests are in flight. + * + * The veil is transparent and only blocks the mouse. If a request is still running after the + * target's spinner delay, the veil gets the 'wicket-veil-busy' class, which shows a spinner; + * once shown, the spinner stays for at least the target's minimum time, so it does not flicker. + * A request carrying the extra parameter 'wicket_nb' is never veiled. + * + * A local veil can also be raised by the server, for a component it is about to update through a + * WebSocket push: a WebSocket text message {"wicketVeil":"show","id":"<markup id>"} raises it, + * Wicket.Veil.hide(id) - evaluated after the pushed update - or {"wicketVeil":"hide",...} lowers it. + */ +;(function (undefined) { + 'use strict'; + + if (typeof(Wicket.Veil) === "object") { + return; + } + + const NO_VEIL_PARAMETER = 'wicket_nb'; + const VEIL_CLASS = 'wicket-veil'; + const BUSY_CLASS = 'wicket-veil-busy'; + const HOST_CLASS = 'wicket-veil-host'; + const WEBSOCKET_MESSAGE_TOPIC = '/websocket/message'; + const MESSAGE_PREFIX = '{"wicketVeil"'; + + let pageTarget = null; + let localTargets = {}; Review Comment: `localTargets` is a plain object indexed by markup ids, so an id like `toString` or `constructor` resolves to an `Object.prototype` member. `findTarget` returns that function, `acquire()` turns its count into `NaN`, and the request runs with no veil at all. `Wicket.Veil.local("constructor", ...)` also writes `delay`/`minimum` onto `Object`. A `Map` avoids this and removes the `hasOwnProperty` boilerplate from both loops: ```js const localTargets = new Map(); // findTarget const target = node.id && localTargets.get(node.id); if (target) { return target; } function dropStaleTargets() { for (const [id, target] of localTargets) { if (target.count === 0 && !document.getElementById(id)) { localTargets.delete(id); } } } // local() const target = localTargets.get(id); if (target) { configure(target, options); } else { localTargets.set(id, createTarget(id, options)); } // show() and hide() const target = localTargets.get(id); // _reset() for (const target of localTargets.values()) { hide(target); } localTargets.clear(); ``` (The suggestion on `findTarget` above keeps the object lookup so it applies on its own; whichever lands second needs the lookup adjusted.) ########## wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/veil/wicket-veil.js: ########## @@ -0,0 +1,324 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Veils the page, or a single component, while Ajax requests are in flight. + * + * The veil is transparent and only blocks the mouse. If a request is still running after the + * target's spinner delay, the veil gets the 'wicket-veil-busy' class, which shows a spinner; + * once shown, the spinner stays for at least the target's minimum time, so it does not flicker. + * A request carrying the extra parameter 'wicket_nb' is never veiled. + * + * A local veil can also be raised by the server, for a component it is about to update through a + * WebSocket push: a WebSocket text message {"wicketVeil":"show","id":"<markup id>"} raises it, + * Wicket.Veil.hide(id) - evaluated after the pushed update - or {"wicketVeil":"hide",...} lowers it. + */ +;(function (undefined) { + 'use strict'; + + if (typeof(Wicket.Veil) === "object") { + return; + } + + const NO_VEIL_PARAMETER = 'wicket_nb'; + const VEIL_CLASS = 'wicket-veil'; + const BUSY_CLASS = 'wicket-veil-busy'; + const HOST_CLASS = 'wicket-veil-host'; + const WEBSOCKET_MESSAGE_TOPIC = '/websocket/message'; + const MESSAGE_PREFIX = '{"wicketVeil"'; + + let pageTarget = null; + let localTargets = {}; + let subscribed = false; + + function createTarget(id, options) { + return { + id: id, + delay: options.delay, + minimum: options.minimum, + count: 0, + host: null, + veil: null, + shownAt: -1, + spinnerTimer: null, + hideTimer: null + }; + } + + function configure(target, options) { + target.delay = options.delay; + target.minimum = options.minimum; + } + + function isOptedOut(attrs) { + const ep = attrs.ep; + if (Array.isArray(ep)) { + return ep.some(function (parameter) { + return parameter && parameter.name === NO_VEIL_PARAMETER; + }); + } + return !!ep && typeof(ep) === "object" && + Object.prototype.hasOwnProperty.call(ep, NO_VEIL_PARAMETER); + } + + function findTarget(attrs) { + let node = attrs.c ? document.getElementById(attrs.c) : null; + for (; node && node !== document; node = node.parentNode) { + if (node.id && localTargets[node.id]) { + return localTargets[node.id]; + } + } + return pageTarget; + } + + function dropStaleTargets() { + for (const id in localTargets) { + if (Object.prototype.hasOwnProperty.call(localTargets, id) && + localTargets[id].count === 0 && !document.getElementById(id)) { + delete localTargets[id]; + } + } + } + + function hide(target) { + const clock = Wicket.Veil._clock; + clock.clearTimeout(target.spinnerTimer); + clock.clearTimeout(target.hideTimer); + target.spinnerTimer = null; + target.hideTimer = null; + target.shownAt = -1; + if (target.veil && target.veil.parentNode) { + target.veil.parentNode.removeChild(target.veil); + } + if (target.host && target !== pageTarget) { + target.host.classList.remove(HOST_CLASS); + } + target.veil = null; + target.host = null; + } + + function show(target) { + const clock = Wicket.Veil._clock; + if (target.hideTimer !== null) { + if (document.body.contains(target.veil)) { + // the previous request's spinner is still on its minimum time: carry on with it + clock.clearTimeout(target.hideTimer); + target.hideTimer = null; + return; + } + hide(target); + } + + const host = target === pageTarget ? document.body : document.getElementById(target.id); + const veil = document.createElement('div'); + veil.className = VEIL_CLASS; + if (target !== pageTarget) { + host.classList.add(HOST_CLASS); + } + host.appendChild(veil); + target.host = host; + target.veil = veil; + + target.spinnerTimer = clock.setTimeout(function () { + target.spinnerTimer = null; + veil.classList.add(BUSY_CLASS); + target.shownAt = clock.now(); + }, target.delay); + } + + function reattach(target) { Review Comment: When the veiled element is replaced while its count is still above zero, the veil is lost: it is a child of the old element, and `reattach()` only runs from `release()`. Example: the server raises the veil with `getVeilMessage()` and, during the long work, pushes a progress update with `handler.add(panel)`. The push replaces the panel's element, the panel is clickable for the rest of the work, and the spinner timer adds `wicket-veil-busy` to a detached div. The same happens with Ajax when a request on another channel, or a timer, re-renders a local-veil host while that host's own slow request is in flight. Core publishes `/dom/node/added` after every replacement (`Wicket.DOM.replace`, in both engines), so the veil can reattach right away instead of waiting for the release: ```js function onDomNodeAdded() { for (const id in localTargets) { if (Object.prototype.hasOwnProperty.call(localTargets, id)) { reattach(localTargets[id]); } } } // in subscribe() Wicket.Event.subscribe(Wicket.Event.Topic.DOM_NODE_ADDED, onDomNodeAdded); ``` `reattach()` already returns early for a veil that is still attached, or for a target with no veil, so it is safe to call for every target. Looping over all targets also covers a replaced ancestor of the host, not only the host itself. ########## wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/veil/wicket-veil.js: ########## @@ -0,0 +1,324 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Veils the page, or a single component, while Ajax requests are in flight. + * + * The veil is transparent and only blocks the mouse. If a request is still running after the + * target's spinner delay, the veil gets the 'wicket-veil-busy' class, which shows a spinner; + * once shown, the spinner stays for at least the target's minimum time, so it does not flicker. + * A request carrying the extra parameter 'wicket_nb' is never veiled. + * + * A local veil can also be raised by the server, for a component it is about to update through a + * WebSocket push: a WebSocket text message {"wicketVeil":"show","id":"<markup id>"} raises it, + * Wicket.Veil.hide(id) - evaluated after the pushed update - or {"wicketVeil":"hide",...} lowers it. + */ +;(function (undefined) { + 'use strict'; + + if (typeof(Wicket.Veil) === "object") { + return; + } + + const NO_VEIL_PARAMETER = 'wicket_nb'; + const VEIL_CLASS = 'wicket-veil'; + const BUSY_CLASS = 'wicket-veil-busy'; + const HOST_CLASS = 'wicket-veil-host'; + const WEBSOCKET_MESSAGE_TOPIC = '/websocket/message'; + const MESSAGE_PREFIX = '{"wicketVeil"'; + + let pageTarget = null; + let localTargets = {}; + let subscribed = false; + + function createTarget(id, options) { + return { + id: id, + delay: options.delay, + minimum: options.minimum, + count: 0, + host: null, + veil: null, + shownAt: -1, + spinnerTimer: null, + hideTimer: null + }; + } + + function configure(target, options) { + target.delay = options.delay; + target.minimum = options.minimum; + } + + function isOptedOut(attrs) { + const ep = attrs.ep; + if (Array.isArray(ep)) { + return ep.some(function (parameter) { + return parameter && parameter.name === NO_VEIL_PARAMETER; + }); + } + return !!ep && typeof(ep) === "object" && + Object.prototype.hasOwnProperty.call(ep, NO_VEIL_PARAMETER); + } + + function findTarget(attrs) { + let node = attrs.c ? document.getElementById(attrs.c) : null; + for (; node && node !== document; node = node.parentNode) { + if (node.id && localTargets[node.id]) { + return localTargets[node.id]; + } + } + return pageTarget; + } Review Comment: `findTarget` starts from `attrs.c`, but core's `Wicket.Ajax.Call._getTarget` prefers `attrs.event.target` and only falls back to `attrs.c`. With a delegated behavior (`attrs.sel`), `attrs.c` is the container. A click on a row that has its own `LocalVeilBehavior` is then attributed to the container, so the row's veil never shows and the page veil, or an outer veil, takes the request instead. Starting from the event target, as core does, picks the innermost veil around the element that was actually clicked. The `isConnected` check covers throttled calls, where the target may have been replaced by the time the request is sent; `attrs.c` is then resolved again by id. ```suggestion function findTarget(attrs) { let node = attrs.event && attrs.event.target; if (!node || !node.isConnected) { node = typeof(attrs.c) === "string" ? document.getElementById(attrs.c) : null; } for (; node && node !== document; node = node.parentNode) { if (node.id && localTargets[node.id]) { return localTargets[node.id]; } } return pageTarget; } ``` ########## wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/veil/wicket-veil.css: ########## @@ -0,0 +1,61 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +.wicket-veil { + position: fixed; + top: 0; + right: 0; + bottom: 0; + left: 0; + z-index: 10000; + background: transparent; + cursor: wait; +} + +.wicket-veil-host { + position: relative; +} + +.wicket-veil-host > .wicket-veil { + position: absolute; + z-index: 1000; Review Comment: The local veil's `z-index: 1000` is not contained: the host gets `position: relative` but no `z-index`, so it creates no stacking context and the veil competes with the page's own layers. A local veil on a component that has scrolled under a sticky header (say `z-index: 100`), or that sits behind an open `ModalDialog` (`z-index: 1000` in its theme, with the veil later in DOM order), paints the dimmed veil and spinner over the header or the modal. Isolating the host keeps the veil's `z-index` inside it, while the host itself stacks as an ordinary positioned element: ```css .wicket-veil-host { position: relative; isolation: isolate; } ``` The only side effect is during the request: a popup inside the host that reaches outside it can drop below later positioned siblings, while the veil covers the host anyway. ########## wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/veil/wicket-veil.js: ########## @@ -0,0 +1,324 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Veils the page, or a single component, while Ajax requests are in flight. + * + * The veil is transparent and only blocks the mouse. If a request is still running after the + * target's spinner delay, the veil gets the 'wicket-veil-busy' class, which shows a spinner; + * once shown, the spinner stays for at least the target's minimum time, so it does not flicker. + * A request carrying the extra parameter 'wicket_nb' is never veiled. + * + * A local veil can also be raised by the server, for a component it is about to update through a + * WebSocket push: a WebSocket text message {"wicketVeil":"show","id":"<markup id>"} raises it, + * Wicket.Veil.hide(id) - evaluated after the pushed update - or {"wicketVeil":"hide",...} lowers it. + */ +;(function (undefined) { + 'use strict'; + + if (typeof(Wicket.Veil) === "object") { + return; + } + + const NO_VEIL_PARAMETER = 'wicket_nb'; + const VEIL_CLASS = 'wicket-veil'; + const BUSY_CLASS = 'wicket-veil-busy'; + const HOST_CLASS = 'wicket-veil-host'; + const WEBSOCKET_MESSAGE_TOPIC = '/websocket/message'; + const MESSAGE_PREFIX = '{"wicketVeil"'; + + let pageTarget = null; + let localTargets = {}; + let subscribed = false; + + function createTarget(id, options) { + return { + id: id, + delay: options.delay, + minimum: options.minimum, + count: 0, + host: null, + veil: null, + shownAt: -1, + spinnerTimer: null, + hideTimer: null + }; + } + + function configure(target, options) { + target.delay = options.delay; + target.minimum = options.minimum; + } + + function isOptedOut(attrs) { + const ep = attrs.ep; + if (Array.isArray(ep)) { + return ep.some(function (parameter) { + return parameter && parameter.name === NO_VEIL_PARAMETER; + }); + } + return !!ep && typeof(ep) === "object" && + Object.prototype.hasOwnProperty.call(ep, NO_VEIL_PARAMETER); + } + + function findTarget(attrs) { + let node = attrs.c ? document.getElementById(attrs.c) : null; + for (; node && node !== document; node = node.parentNode) { + if (node.id && localTargets[node.id]) { + return localTargets[node.id]; + } + } + return pageTarget; + } + + function dropStaleTargets() { + for (const id in localTargets) { + if (Object.prototype.hasOwnProperty.call(localTargets, id) && + localTargets[id].count === 0 && !document.getElementById(id)) { + delete localTargets[id]; + } + } + } + + function hide(target) { + const clock = Wicket.Veil._clock; + clock.clearTimeout(target.spinnerTimer); + clock.clearTimeout(target.hideTimer); + target.spinnerTimer = null; + target.hideTimer = null; + target.shownAt = -1; + if (target.veil && target.veil.parentNode) { + target.veil.parentNode.removeChild(target.veil); + } + if (target.host && target !== pageTarget) { + target.host.classList.remove(HOST_CLASS); + } + target.veil = null; + target.host = null; + } + + function show(target) { + const clock = Wicket.Veil._clock; + if (target.hideTimer !== null) { + if (document.body.contains(target.veil)) { + // the previous request's spinner is still on its minimum time: carry on with it + clock.clearTimeout(target.hideTimer); + target.hideTimer = null; + return; + } + hide(target); + } + + const host = target === pageTarget ? document.body : document.getElementById(target.id); + const veil = document.createElement('div'); + veil.className = VEIL_CLASS; + if (target !== pageTarget) { + host.classList.add(HOST_CLASS); + } + host.appendChild(veil); + target.host = host; + target.veil = veil; + + target.spinnerTimer = clock.setTimeout(function () { + target.spinnerTimer = null; + veil.classList.add(BUSY_CLASS); + target.shownAt = clock.now(); + }, target.delay); + } + + function reattach(target) { + if (target === pageTarget || !target.veil || document.body.contains(target.veil)) { + return; + } + const host = document.getElementById(target.id); + if (host) { + host.classList.add(HOST_CLASS); + host.appendChild(target.veil); + target.host = host; + } + } + + function release(target) { + const clock = Wicket.Veil._clock; + if (target.shownAt < 0) { + hide(target); + return; + } + const remaining = target.minimum - (clock.now() - target.shownAt); + if (remaining > 0) { + // the update may have replaced the element, and the veil with it + reattach(target); + target.hideTimer = clock.setTimeout(function () { + hide(target); + }, remaining); + } else { + hide(target); + } + } + + function acquire(target) { + target.count++; + if (target.count === 1) { + show(target); + } + } + + function releaseOne(target) { + if (target.count > 0) { + target.count--; + if (target.count === 0) { + release(target); + } + } + } + + function onBeforeSend(jqEvent, attrs) { + if (!attrs || isOptedOut(attrs)) { + return; + } + dropStaleTargets(); + const target = findTarget(attrs); + if (target === null) { + return; + } + attrs.wicketVeil = target; + acquire(target); + } + + function onDone(jqEvent, attrs) { Review Comment: On an Ajax redirect (`setResponsePage(...)` from a slow Ajax handler), `/ajax/call/done` still fires while the browser is navigating, so the veil comes down and the page can be clicked again before the next page arrives: a second click on Save fires a second request, the double submit the guide says the veil prevents. Core deliberately keeps its own activity indicator up in this case (`if (attrs.i && context.isRedirecting !== true)` in the done step of `wicket-ajax-jquery.js`). The difficulty is that `AJAX_CALL_DONE` subscribers only receive `attrs`, not the `context` that carries `isRedirecting`. Two options: - have core expose it, e.g. set a flag on `attrs` where `context.isRedirecting = true` is set, or pass it along with the done event, and skip `releaseOne()` in `onDone` when it is set; - detect it in the veil, e.g. in an `AJAX_CALL_COMPLETE` subscriber via `jqXHR.getResponseHeader('Ajax-Location')`. That only covers the header redirect, not a `<redirect>` element in the response, so I'd prefer the first. ########## wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/veil/wicket-veil.js: ########## @@ -0,0 +1,324 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Veils the page, or a single component, while Ajax requests are in flight. + * + * The veil is transparent and only blocks the mouse. If a request is still running after the + * target's spinner delay, the veil gets the 'wicket-veil-busy' class, which shows a spinner; + * once shown, the spinner stays for at least the target's minimum time, so it does not flicker. + * A request carrying the extra parameter 'wicket_nb' is never veiled. + * + * A local veil can also be raised by the server, for a component it is about to update through a + * WebSocket push: a WebSocket text message {"wicketVeil":"show","id":"<markup id>"} raises it, + * Wicket.Veil.hide(id) - evaluated after the pushed update - or {"wicketVeil":"hide",...} lowers it. + */ +;(function (undefined) { + 'use strict'; + + if (typeof(Wicket.Veil) === "object") { + return; + } + + const NO_VEIL_PARAMETER = 'wicket_nb'; + const VEIL_CLASS = 'wicket-veil'; + const BUSY_CLASS = 'wicket-veil-busy'; + const HOST_CLASS = 'wicket-veil-host'; + const WEBSOCKET_MESSAGE_TOPIC = '/websocket/message'; + const MESSAGE_PREFIX = '{"wicketVeil"'; + + let pageTarget = null; + let localTargets = {}; + let subscribed = false; + + function createTarget(id, options) { + return { + id: id, + delay: options.delay, + minimum: options.minimum, + count: 0, + host: null, + veil: null, + shownAt: -1, + spinnerTimer: null, + hideTimer: null + }; + } + + function configure(target, options) { + target.delay = options.delay; + target.minimum = options.minimum; + } + + function isOptedOut(attrs) { + const ep = attrs.ep; + if (Array.isArray(ep)) { + return ep.some(function (parameter) { + return parameter && parameter.name === NO_VEIL_PARAMETER; + }); + } + return !!ep && typeof(ep) === "object" && + Object.prototype.hasOwnProperty.call(ep, NO_VEIL_PARAMETER); + } + + function findTarget(attrs) { + let node = attrs.c ? document.getElementById(attrs.c) : null; + for (; node && node !== document; node = node.parentNode) { + if (node.id && localTargets[node.id]) { + return localTargets[node.id]; + } + } + return pageTarget; + } + + function dropStaleTargets() { + for (const id in localTargets) { + if (Object.prototype.hasOwnProperty.call(localTargets, id) && + localTargets[id].count === 0 && !document.getElementById(id)) { + delete localTargets[id]; + } + } + } + + function hide(target) { + const clock = Wicket.Veil._clock; + clock.clearTimeout(target.spinnerTimer); + clock.clearTimeout(target.hideTimer); + target.spinnerTimer = null; + target.hideTimer = null; + target.shownAt = -1; + if (target.veil && target.veil.parentNode) { + target.veil.parentNode.removeChild(target.veil); + } + if (target.host && target !== pageTarget) { + target.host.classList.remove(HOST_CLASS); + } + target.veil = null; + target.host = null; + } + + function show(target) { + const clock = Wicket.Veil._clock; + if (target.hideTimer !== null) { + if (document.body.contains(target.veil)) { + // the previous request's spinner is still on its minimum time: carry on with it + clock.clearTimeout(target.hideTimer); + target.hideTimer = null; + return; + } + hide(target); + } + + const host = target === pageTarget ? document.body : document.getElementById(target.id); + const veil = document.createElement('div'); + veil.className = VEIL_CLASS; + if (target !== pageTarget) { + host.classList.add(HOST_CLASS); + } + host.appendChild(veil); + target.host = host; + target.veil = veil; + + target.spinnerTimer = clock.setTimeout(function () { + target.spinnerTimer = null; + veil.classList.add(BUSY_CLASS); + target.shownAt = clock.now(); + }, target.delay); + } + + function reattach(target) { + if (target === pageTarget || !target.veil || document.body.contains(target.veil)) { + return; + } + const host = document.getElementById(target.id); + if (host) { + host.classList.add(HOST_CLASS); + host.appendChild(target.veil); + target.host = host; + } + } + + function release(target) { + const clock = Wicket.Veil._clock; + if (target.shownAt < 0) { + hide(target); + return; + } + const remaining = target.minimum - (clock.now() - target.shownAt); + if (remaining > 0) { + // the update may have replaced the element, and the veil with it + reattach(target); + target.hideTimer = clock.setTimeout(function () { + hide(target); + }, remaining); + } else { + hide(target); + } + } + + function acquire(target) { + target.count++; + if (target.count === 1) { + show(target); + } + } + + function releaseOne(target) { + if (target.count > 0) { + target.count--; + if (target.count === 0) { + release(target); + } + } + } + + function onBeforeSend(jqEvent, attrs) { + if (!attrs || isOptedOut(attrs)) { + return; + } + dropStaleTargets(); + const target = findTarget(attrs); + if (target === null) { + return; + } + attrs.wicketVeil = target; + acquire(target); + } + + function onDone(jqEvent, attrs) { + const target = attrs && attrs.wicketVeil; + if (!target) { + return; + } + delete attrs.wicketVeil; + releaseOne(target); + } + + function onWebSocketMessage(jqEvent, message) { + if (typeof(message) !== "string" || message.indexOf(MESSAGE_PREFIX) !== 0) { + return; + } + let command; + try { + command = JSON.parse(message); + } catch (e) { + return; + } + if (command.wicketVeil === 'show') { + Wicket.Veil.show(command.id); + } else if (command.wicketVeil === 'hide') { + Wicket.Veil.hide(command.id); + } + } + + function subscribe() { + if (subscribed === false) { + subscribed = true; + Wicket.Event.subscribe(Wicket.Event.Topic.AJAX_CALL_BEFORE_SEND, onBeforeSend); + Wicket.Event.subscribe(Wicket.Event.Topic.AJAX_CALL_DONE, onDone); + Wicket.Event.subscribe(WEBSOCKET_MESSAGE_TOPIC, onWebSocketMessage); + } + } + + Wicket.Veil = { + + /** + * Veils the whole page during every Ajax request that no local veil claims. + * + * @param options {Object} - 'delay': milliseconds before the spinner shows, + * 'minimum': milliseconds the spinner stays once shown + */ + page: function (options) { + subscribe(); + if (pageTarget === null) { + pageTarget = createTarget(null, options); + } else { + configure(pageTarget, options); + } + }, + + /** + * Veils only the element with the given id, during the Ajax requests fired by + * components inside it. + * + * @param id {String} - the markup id of the element to veil + * @param options {Object} - as for page() + */ + local: function (id, options) { + subscribe(); + const target = localTargets[id]; + if (target) { + configure(target, options); + } else { + localTargets[id] = createTarget(id, options); + } + }, + + /** + * Raises the local veil registered for the given id, as an Ajax request from inside it + * would, with the same timings. Each call has to be matched by a call to hide(). + * + * @param id {String} - the markup id of a component with a local veil + */ + show: function (id) { + const target = localTargets[id]; + if (target && document.getElementById(id)) { + acquire(target); + } + }, + + /** + * Lowers the local veil raised by show(), respecting the spinner's minimum time. Calls + * without a matching show() are ignored. + * + * @param id {String} - the markup id of a component with a local veil + */ + hide: function (id) { Review Comment: Server `show()`/`hide()` and Ajax requests share one counter, so a `hide()` without a matching `show()` is not ignored as the Javadoc says: it decrements the count held by an Ajax request that is still in flight and lowers that request's veil early. The request's own `onDone` then finds count 0 and does nothing. This happens when the show message never arrived (sent before the WebSocket was open, for instance) but the hide does. Counting the server raises separately keeps the two apart: ```js // createTarget() raised: 0, show: function (id) { const target = localTargets[id]; if (target && document.getElementById(id)) { target.raised++; acquire(target); } }, hide: function (id) { const target = localTargets[id]; if (target && target.raised > 0) { target.raised--; releaseOne(target); } }, ``` ########## wicket-extensions/src/main/java/org/apache/wicket/extensions/ajax/veil/AbstractVeilBehavior.java: ########## @@ -0,0 +1,150 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.extensions.ajax.veil; + +import java.time.Duration; + +import org.apache.wicket.Component; +import org.apache.wicket.behavior.Behavior; +import org.apache.wicket.markup.head.CssHeaderItem; +import org.apache.wicket.markup.head.IHeaderResponse; +import org.apache.wicket.markup.head.JavaScriptHeaderItem; +import org.apache.wicket.markup.head.OnDomReadyHeaderItem; +import org.apache.wicket.request.resource.CssResourceReference; +import org.apache.wicket.request.resource.JavaScriptResourceReference; +import org.apache.wicket.request.resource.ResourceReference; +import org.apache.wicket.resource.CoreLibrariesContributor; +import org.apache.wicket.util.lang.Args; + +/** + * Base class of the behaviors that put a veil over a region of the page while Ajax requests are + * in flight. + * <p> + * The veil appears as soon as a request is sent. It is transparent and swallows mouse clicks, so + * the user cannot fire further requests or change what the pending one is about. Only if the + * request is still running after {@link #getSpinnerDelay()} does the veil get the CSS class + * {@code wicket-veil-busy}, which dims the region and shows a spinner; once shown, the spinner + * stays for at least {@link #getMinimumSpinnerTime()}, so a response arriving just after it + * appeared does not make it flicker. Both timings apply to the page veil and to local veils + * alike, and can be changed per behavior with {@link #setSpinnerDelay(Duration)} and + * {@link #setMinimumSpinnerTime(Duration)}. The veil does not intercept the keyboard. + * <p> + * The look comes from {@code wicket-veil.css} and can be overridden with the classes + * {@code wicket-veil}, {@code wicket-veil-busy} and {@code wicket-veil-host}. + * <p> + * A request is left unveiled when it carries the extra parameter + * {@value PageVeilBehavior#NO_VEIL_PARAMETER}, see {@link PageVeilBehavior#noVeil}. + * + * @see PageVeilBehavior + * @see LocalVeilBehavior + * @since 11.0.0 + */ +public abstract class AbstractVeilBehavior extends Behavior +{ + private static final long serialVersionUID = 1L; + + private static final ResourceReference JS = new JavaScriptResourceReference( + AbstractVeilBehavior.class, "wicket-veil.js"); + + private static final ResourceReference CSS = new CssResourceReference( + AbstractVeilBehavior.class, "wicket-veil.css"); + + private Duration spinnerDelay = Duration.ofMillis(300); + + private Duration minimumSpinnerTime = Duration.ofMillis(500); + + /** + * @return how long a request has to run before the spinner is shown; 300 ms by default + */ + protected Duration getSpinnerDelay() + { + return spinnerDelay; + } + + /** + * Sets how long a request has to run before the spinner is shown. {@link Duration#ZERO} + * shows it as soon as the request is sent. + * + * @param spinnerDelay + * the delay, not negative + * @return this, for chaining + */ + public AbstractVeilBehavior setSpinnerDelay(Duration spinnerDelay) + { + this.spinnerDelay = checkNotNegative(spinnerDelay, "spinnerDelay"); + return this; + } + + /** + * @return how long the spinner stays at least, once it is shown; 500 ms by default + */ + protected Duration getMinimumSpinnerTime() + { + return minimumSpinnerTime; + } + + /** + * Sets how long the spinner stays at least, once it is shown, even when the request is over + * sooner. {@link Duration#ZERO} removes it together with the request. + * + * @param minimumSpinnerTime + * the minimum time, not negative + * @return this, for chaining + */ + public AbstractVeilBehavior setMinimumSpinnerTime(Duration minimumSpinnerTime) + { + this.minimumSpinnerTime = checkNotNegative(minimumSpinnerTime, "minimumSpinnerTime"); + return this; + } + + private static Duration checkNotNegative(Duration duration, String name) + { + Args.notNull(duration, name); + if (duration.isNegative()) + { + throw new IllegalArgumentException(name + " must not be negative: " + duration); + } + return duration; + } + + @Override + public void renderHead(Component component, IHeaderResponse response) + { + super.renderHead(component, response); + + CoreLibrariesContributor.contributeAjax(component.getApplication(), response); + response.render(JavaScriptHeaderItem.forReference(JS)); + response.render(CssHeaderItem.forReference(CSS)); + response.render(OnDomReadyHeaderItem.forScript(getInitScript(component))); + } + + /** + * @param component + * the component this behavior is bound to + * @return the script registering the veil with {@code Wicket.Veil} + */ + protected abstract CharSequence getInitScript(Component component); + + /** + * @return the timings as the options object {@code Wicket.Veil} expects + */ + protected final String getOptions() + { + return String.format("{\"delay\":%d,\"minimum\":%d}", getSpinnerDelay().toMillis(), + getMinimumSpinnerTime().toMillis()); Review Comment: `String.format("%d")` uses the JVM's default locale, which localizes the digits. With a default locale such as `th-TH-u-nu-thai` this renders `{"delay":๓๐๐,...}`, a syntax error, so the domready script fails and no veil is registered. ```suggestion return String.format(Locale.ROOT, "{\"delay\":%d,\"minimum\":%d}", getSpinnerDelay().toMillis(), getMinimumSpinnerTime().toMillis()); ``` (plus `import java.util.Locale;`) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
