Bladeren bron

Fix: stepper close via direct element listeners (not doc delegation)

Previous iterations all routed the close logic through document-level
event delegation — pointermove tracker (8d79f96), focusout (f189ef7),
pointerleave on document (8d79f96), capture-phase click/pointerdown.
None closed the popover reliably in practice. The common thread is
the delegation model: if anything in the document's event path
silently swallows or doesn't fire those events, every delegation-
based close path breaks at once, which matches the observed failure
mode ("slider works but won't close").

Swapped to element-local listeners. pointerenter / pointerleave are
attached DIRECTLY to the two elements that matter:

- the popover (attached once in build());
- the bound input (attached in bindInput() on open(), detached in
  unbindInput() on close() or when rebinding to a different input).

pointerleave on either element starts a 200 ms close timer;
pointerenter on either cancels it. A 300 ms open-grace window makes
the timer reschedule itself instead of dismissing if the pointer
happens to be off both rects in the very first frames after click-
to-open (common: user clicks cell then drifts back toward the
table). These listeners live on the elements themselves — nothing
else on the page can suppress them.

Outside-click close still uses document.addEventListener with
{capture: true}, but now has a 50 ms OPEN_IGNORE_MS guard so the
very click that opened the popup doesn't immediately close it.

Removed the document-level pointermove tracker, the document-level
pointerleave viewport-exit handler, and the focusout handler
entirely — superseded by the element-local listeners.

Kept as-is: slider-to-input sync (elRange `input` event →
boundInput.value + bubbling input/change), scroll/resize anchoring
(window listeners with capture=true + rAF-throttled reposition),
Escape close, ArrowUp/Down keyboard nudge.

CSS untouched — visual style identical.

phpunit: 88 / 208 unchanged. node --check clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
achiappa 2 weken geleden
bovenliggende
commit
e93df6b15d
1 gewijzigde bestanden met toevoegingen van 120 en 117 verwijderingen
  1. 120 117
      public/assets/js/number-stepper.js

+ 120 - 117
public/assets/js/number-stepper.js

@@ -7,20 +7,32 @@
  * sprint-planner.js's debounced save + capacity recompute fire via
  * the `change` event on the bound input.
  *
- * Close triggers (any one fires):
- *   1. Pointer leaves both the input and the popover for >200 ms
- *      (past a 300 ms open-grace window that forgives post-click
- *      cursor wobble).
- *   2. Pointer leaves the viewport.
- *   3. pointerdown lands anywhere outside both input and popover —
- *      registered in the *capture* phase so a descendant
- *      stopPropagation can't trap the popup open.
- *   4. Escape keypress (focus returns to the input).
+ * Close strategy (DIFFERENT from earlier iterations — several prior
+ * attempts relied on document-level `pointermove` / `focusout` /
+ * `pointerleave` delegation and all silently failed in practice).
+ * This version attaches pointer listeners DIRECTLY to the two
+ * elements that matter — the bound input and the popover — in
+ * open(), and detaches them in close(). That way close behaviour
+ * can't be swallowed by anything else on the page.
  *
- * Position tracking: on every scroll (any ancestor, captured at
- * window) and on window resize, a rAF-throttled reposition runs so
- * the popover stays anchored to the input as content moves. If the
- * input's bounding rect goes to 0×0 (removed from DOM) we close.
+ *   - `pointerleave` on input → schedule close (200 ms).
+ *   - `pointerenter` on popover → cancel pending close.
+ *   - `pointerleave` on popover → schedule close.
+ *   - `pointerenter` on input  → cancel pending close.
+ *   - 300 ms open-grace window: if the close timer fires while still
+ *     inside the grace, it reschedules instead of dismissing.
+ *
+ * Outside-click close: capture-phase `pointerdown` on document, with
+ * a 50 ms "just opened" guard so the very click that opened the
+ * popup doesn't count as an outside click.
+ *
+ * Escape closes + returns focus. ArrowUp/Down on the focused input
+ * steps by the input's `step` attribute (replaces the native spinner
+ * shortcut suppressed in Phase 17).
+ *
+ * Scroll anchoring: `window` scroll (capture) + resize listeners
+ * rAF-throttle a reposition, so the popover follows the input as
+ * any scrollable ancestor moves. A 0×0 bounding rect triggers close.
  *
  * Strict-CSP-clean (standard <script src>, no inline handlers). No
  * globals. Vanilla JS — no jQuery.
@@ -28,15 +40,16 @@
 (function () {
     'use strict';
 
-    const OPEN_GRACE_MS  = 300;  // min lifetime of a newly-opened popover
-    const CLOSE_DELAY_MS = 200;  // grace for the cursor to re-enter
+    const OPEN_GRACE_MS  = 300;
+    const CLOSE_DELAY_MS = 200;
+    const OPEN_IGNORE_MS = 50;   // ignore outside-click within this of open
 
-    let pop        = null;
-    let elRange    = null;
-    let boundInput = null;
-    let openedAt   = 0;
-    let closeTimer = null;
-    let rafId      = null;
+    let pop          = null;
+    let elRange      = null;
+    let boundInput   = null;
+    let openedAt     = 0;
+    let closeTimer   = null;
+    let rafId        = null;
 
     function now() {
         return (typeof performance !== 'undefined' && performance.now)
@@ -44,6 +57,10 @@
             : Date.now();
     }
 
+    // --------------------------------------------------------------
+    // Open / close
+    // --------------------------------------------------------------
+
     function build() {
         if (pop) { return; }
         pop = document.createElement('div');
@@ -51,24 +68,32 @@
         pop.hidden = true;
         pop.setAttribute('role', 'dialog');
         pop.setAttribute('aria-label', 'Set value');
-        // `orient="vertical"` is the legacy Firefox attribute; modern
-        // browsers pick up vertical orientation from the writing-mode
-        // CSS in assets/css/input.css. Both are present so the slider
-        // renders vertically everywhere.
         pop.innerHTML = '<input type="range" orient="vertical">';
         document.body.appendChild(pop);
         elRange = pop.querySelector('input[type="range"]');
 
-        // Mirror every slider tick into the bound input. Fire both
-        // `input` (live) and `change` (save + recompute) so
-        // sprint-planner.js picks it up; its 400 ms debounce
-        // coalesces the flurry into one write.
+        // Slider → input sync on every tick.
         elRange.addEventListener('input', function () {
             if (!boundInput) { return; }
             boundInput.value = elRange.value;
             boundInput.dispatchEvent(new Event('input',  { bubbles: true }));
             boundInput.dispatchEvent(new Event('change', { bubbles: true }));
         });
+
+        // Popover-side pointer tracking. Attached once here; input-
+        // side listeners are attached per-open in bindInput() so they
+        // follow whichever input is currently active.
+        pop.addEventListener('pointerenter', cancelCloseTimer);
+        pop.addEventListener('pointerleave', scheduleClose);
+    }
+
+    function bindInput(input) {
+        input.addEventListener('pointerenter', cancelCloseTimer);
+        input.addEventListener('pointerleave', scheduleClose);
+    }
+    function unbindInput(input) {
+        input.removeEventListener('pointerenter', cancelCloseTimer);
+        input.removeEventListener('pointerleave', scheduleClose);
     }
 
     function readNum(input, attr) {
@@ -78,62 +103,26 @@
         return Number.isFinite(n) ? n : NaN;
     }
 
-    function reposition() {
-        if (!pop || pop.hidden || !boundInput) { return; }
-        const r = boundInput.getBoundingClientRect();
-        // Input is detached or fully collapsed — get out.
-        if (r.width === 0 && r.height === 0) { close(); return; }
-
-        const pw     = pop.offsetWidth;
-        const ph     = pop.offsetHeight;
-        const vw     = window.innerWidth;
-        const vh     = window.innerHeight;
-        const GAP    = 6;
-        const MARGIN = 4;
-
-        let left = r.right + GAP;
-        let top  = r.top + (r.height - ph) / 2;
-        if (left + pw > vw - MARGIN) { left = r.left - pw - GAP; }
-        left = Math.max(MARGIN, Math.min(left, vw - pw - MARGIN));
-        top  = Math.max(MARGIN, Math.min(top,  vh - ph - MARGIN));
-
-        pop.style.left = left + 'px';
-        pop.style.top  = top  + 'px';
-    }
-    function scheduleReposition() {
-        if (rafId !== null) { return; }
-        rafId = requestAnimationFrame(function () {
-            rafId = null;
-            reposition();
-        });
-    }
-
-    function isEligible(el) {
-        return !!(el
-            && el.matches
-            && el.matches('input[type="number"]')
-            && !el.disabled
-            && !el.readOnly);
-    }
-
     function open(input) {
         build();
         if (boundInput === input && !pop.hidden) {
             scheduleReposition();
             return;
         }
+        // Moving between inputs: detach from previous before rebinding.
+        if (boundInput && boundInput !== input) {
+            unbindInput(boundInput);
+        }
         boundInput = input;
         openedAt   = now();
         cancelCloseTimer();
+        bindInput(input);
 
         const step = readNum(input, 'step');
         const min  = readNum(input, 'min');
         const max  = readNum(input, 'max');
         const eff  = Number.isFinite(step) && step > 0 ? step : 1;
         const cur  = Number(input.value) || 0;
-        // Slider needs a finite min+max. Fall back sensibly when the
-        // input leaves them open (task-assignment cells: min=0 but no
-        // max) so there's always usable overhead on the slider.
         const sMin = Number.isFinite(min) ? min : 0;
         const sMax = Number.isFinite(max) ? max : Math.max(cur + 5, 10);
 
@@ -153,9 +142,7 @@
         const prev = boundInput;
         boundInput = null;
         if (prev) {
-            // Final change so the last slider position is saved via the
-            // existing debounced pipeline. Harmless no-op if the value
-            // didn't actually change.
+            unbindInput(prev);
             prev.dispatchEvent(new Event('change', { bubbles: true }));
         }
     }
@@ -167,22 +154,59 @@
         if (closeTimer !== null) { return; }
         closeTimer = setTimeout(function () {
             closeTimer = null;
-            // Respect the open-grace window — if the cursor was outside
-            // both rects in the first 300 ms (common: click-to-open
-            // followed by any mouse drift), reschedule instead of
-            // closing instantly.
+            // Don't dismiss during the open-grace window — a click-to-
+            // open whose pointer is wandering in the first 300 ms
+            // shouldn't kill the popup before the user's had a chance
+            // to reach the slider.
             if (now() - openedAt < OPEN_GRACE_MS) { scheduleClose(); return; }
             close();
         }, CLOSE_DELAY_MS);
     }
 
-    function pointInRect(x, y, r) {
-        return x >= r.left && x <= r.right && y >= r.top && y <= r.bottom;
+    // --------------------------------------------------------------
+    // Positioning
+    // --------------------------------------------------------------
+
+    function reposition() {
+        if (!pop || pop.hidden || !boundInput) { return; }
+        const r = boundInput.getBoundingClientRect();
+        if (r.width === 0 && r.height === 0) { close(); return; }
+
+        const pw     = pop.offsetWidth;
+        const ph     = pop.offsetHeight;
+        const vw     = window.innerWidth;
+        const vh     = window.innerHeight;
+        const GAP    = 6;
+        const MARGIN = 4;
+
+        let left = r.right + GAP;
+        let top  = r.top + (r.height - ph) / 2;
+        if (left + pw > vw - MARGIN) { left = r.left - pw - GAP; }
+        left = Math.max(MARGIN, Math.min(left, vw - pw - MARGIN));
+        top  = Math.max(MARGIN, Math.min(top,  vh - ph - MARGIN));
+
+        pop.style.left = left + 'px';
+        pop.style.top  = top  + 'px';
+    }
+    function scheduleReposition() {
+        if (rafId !== null) { return; }
+        rafId = requestAnimationFrame(function () {
+            rafId = null;
+            reposition();
+        });
     }
 
-    // ------------------------------------------------------------------
+    // --------------------------------------------------------------
     // Open triggers
-    // ------------------------------------------------------------------
+    // --------------------------------------------------------------
+
+    function isEligible(el) {
+        return !!(el
+            && el.matches
+            && el.matches('input[type="number"]')
+            && !el.disabled
+            && !el.readOnly);
+    }
 
     document.addEventListener('click', function (ev) {
         if (isEligible(ev.target)) { open(ev.target); }
@@ -191,40 +215,24 @@
         if (isEligible(ev.target)) { open(ev.target); }
     });
 
-    // ------------------------------------------------------------------
-    // Close triggers
-    // ------------------------------------------------------------------
+    // --------------------------------------------------------------
+    // Outside pointerdown → close. Capture phase + open-ignore
+    // window so the opening click doesn't close us.
+    // --------------------------------------------------------------
 
-    // Pointer-position tracker. Uses clientX/Y vs live bounding rects
-    // so a mid-drag reposition (via the scroll listener below) is
-    // picked up on the next move — no stale "thought it was over"
-    // hits.
-    document.addEventListener('pointermove', function (ev) {
-        if (!pop || pop.hidden || !boundInput) { return; }
-        const ir = boundInput.getBoundingClientRect();
-        const pr = pop.getBoundingClientRect();
-        const over = pointInRect(ev.clientX, ev.clientY, ir)
-                  || pointInRect(ev.clientX, ev.clientY, pr);
-        if (over) { cancelCloseTimer(); }
-        else      { scheduleClose(); }
-    });
-
-    // Pointer exits the viewport (mouse into browser chrome, etc.).
-    document.addEventListener('pointerleave', function () {
-        if (pop && !pop.hidden) { scheduleClose(); }
-    });
-
-    // Outside pointerdown — capture phase so a downstream
-    // stopPropagation can't silently leave us hanging.
     document.addEventListener('pointerdown', function (ev) {
         if (!pop || pop.hidden) { return; }
+        if (now() - openedAt < OPEN_IGNORE_MS) { return; }
         const t = ev.target;
         if (t === boundInput || (boundInput && boundInput.contains && boundInput.contains(t))) { return; }
         if (pop.contains(t)) { return; }
         close();
     }, true);
 
-    // Escape closes + returns focus to the input.
+    // --------------------------------------------------------------
+    // Escape closes + returns focus
+    // --------------------------------------------------------------
+
     document.addEventListener('keydown', function (ev) {
         if (!pop || pop.hidden || ev.key !== 'Escape') { return; }
         ev.preventDefault();
@@ -233,21 +241,16 @@
         if (prev) { try { prev.focus(); } catch (_) { /* ignore */ } }
     });
 
-    // ------------------------------------------------------------------
-    // Stay anchored on scroll / resize
-    // ------------------------------------------------------------------
+    // --------------------------------------------------------------
+    // Scroll / resize anchoring
+    // --------------------------------------------------------------
 
-    // Capture phase catches scroll events on any scrollable ancestor
-    // (the Arbeitstage grid's `overflow-x: auto` container, the task
-    // list's overflow div, the main page). rAF-throttled so we never
-    // fight the browser during a rapid wheel burst.
     window.addEventListener('scroll', scheduleReposition, true);
     window.addEventListener('resize', scheduleReposition);
 
-    // ------------------------------------------------------------------
-    // Bonus: keyboard nudge on the focused input (replaces the native
-    // spinner arrows Phase 17 suppressed via CSS).
-    // ------------------------------------------------------------------
+    // --------------------------------------------------------------
+    // Keyboard nudge on the focused input
+    // --------------------------------------------------------------
 
     document.addEventListener('keydown', function (ev) {
         const t = ev.target;