Paint the line on both edges of a pointer drag - #22
Conversation
.lf-dragging is half of unrecordedGesture(), so a widget wearing it moves core's z row — and the pointer path wrote the class from two places that paint nothing. The keyboard path already paints on both edges (#grab, #release); the drag did not, and the window there is a whole gesture rather than a frame: the focus paint lands on the mousedown and fallbackTolerance fires onStart after it, so on a quiet board nothing repaints until the drop. The line therefore offered undo for as long as the reader held the card, over a press the dispatcher was already refusing. onEnd is the same gap backwards: a card dropped where it was picked up takes the class off and returns before #send, leaving the offer withheld from a press that is live. keys()'s docblock stated the paint duty for a widget's own rows; it now states it for state a widget writes that moves a row it never declared.
leaf-agent
left a comment
There was a problem hiding this comment.
Self-review. The fix reads correct on both edges: onStart paints after the class lands, and onEnd paints before the branch that returns without a send — and where a send does follow, sendAction records it synchronously before its first await, so the coalesced frame sees sending non-empty and z stays withheld, as the comment claims. The test is the shape tests/CLAUDE.md asks for under A state the page passes through is not a state to poll for — consume the step, then read once — rather than an expect that would report whichever later paint the page happened to make.
One observation, on where the fix sits rather than on whether it works.
|
The assertion reads the accept pill after the undo and finds Nothing in this diff reaches it: the two added So this branch stays red until #21 lands. Re-running the failed job to see whether it clears on its own; if the maintainer takes #21 first, a rebase would settle it for good rather than by luck. |
|
Green on the third attempt of The second one is not a |
The paint duty for .lf-dragging was stated in prose on keys()'s docblock, which leaves it to whichever module remembers — the shape CLAUDE.md names under "The widget list is never closed", and the same one the data-lf-state mark was moved out of. No declaration is needed here: the class is core's own vocabulary, with core its only reader, so core can own the writer. dragging(el, on) toggles it and paints, and lf-board's four sites say the reader's hand is on it rather than writing the class and remembering the paint.
# Conflicts: # plugins/leaf/skills/leaf/bundled/widgets/lf-board.js
leaf-agent
left a comment
There was a problem hiding this comment.
Self-review of the increment since the last one (5920c34, plus the merge of main at 9205da8). No new findings in the code: dragging(el, on) is the shape the earlier thread argued for, .lf-dragging now has exactly one writer, and the four call sites in lf-board.js are the only ones. The net diff of 002d502..9205da8 over the two runtime files is exactly that change, and the merge took main's #grab refactor (const index = cards.indexOf(card)) rather than reinstating the branch's form — so nothing semantic was resolved in either direction.
What is worth flagging is a signal gap rather than a defect: ci is green on 5920c34 but has never registered on 9205da8, the merge commit that is now the head. Since main picked up several runtime refactors between the two (Refactor state coordination to use semantic facets, Hold the page still while it is arriving), the merged tree the branch would land as had no gate on it. I ran the covering subset against 9205da8 itself on this runner rather than leave that unstated.
What I ran, and the bug-backs
Checked out 9205da8 detached (the review checkout's refs/pull/22/merge was stale — it merged 5920c34 into an older base):
uv run pytest tests/test_render.py -q -n0 --run-nightly -k "pointer_drag or dragging or live_drag"
2 passed, 483 deselected in 9.11s
uv run pytest tests/test_render.py -q -n0 --run-nightly -k "board or grab or card or undo or keyline or z_"
37 passed, 448 deselected in 57.33s
Both edges bug-backed separately, because one assertion carrying both would not have shown it:
- Dropping
paintKeys()fromdragginginleaf.js— the drag-edge assertion fails,undostill on the line under a live drag. - Swapping
onEnd'sdragging(this, false)for a barethis.classList.remove("lf-dragging")inlf-board.js, so the class comes off but nothing paints — the drop-edge assertion fails (the drop that sent nothing left the line refusing a press that is live), while the drag-edge one still passes.
So test_a_pointer_drag_stops_the_line_offering_the_press_it_refuses is not resting on one paint standing in for the other. The quiet-window premise its docstring rests on also holds in the code: poll() only reaches renderPanel() (and so paintHere) when eventSeq > lastEventSeq, and renderStatus/renderVersions/renderOthers call neither — so a routine poll landing inside the drag cannot repaint the line and mask a missing edge.
The full suite is still the maintainer's to run; this is the subset that covers the change, not --run-nightly over everything.
# Conflicts: # plugins/leaf/skills/leaf/assets/leaf.js # tests/test_render.py
leaf-agent
left a comment
There was a problem hiding this comment.
Self-review of the new base. The increment since the last review is the merge of main at #21 and nothing else: git diff 85631b3..5044bb6 over the two runtime files is exactly this branch's change, so the merge neither dropped anything from main nor reinstated the branch's form of anything main had refactored. No new findings in the code.
What the merge moves is the ground under the fix rather than the fix. #21 replaced sending with outbox and added watchProjectionDrag, a .lf-dragging observer that paints on the drag's far edge — and that second one is worth checking rather than assuming, because it is a paint on the drop edge that did not exist when this test was bug-backed. A test that no longer fails without the fix would be green for the wrong reason, and green is exactly what it is. It doesn't reach here: the observer is armed only from reconcileState, which returns at its own .lf-dragging gate during a drag, so on the quiet board this test drives it is never installed. I re-ran both bug-backs on the merged tree to confirm that rather than resting on the reading.
The other term survives the rename intact. unaccountedGesture() is undoing || outbox.length > 0 || .lf-dragging, and the outbox push is synchronous inside the new Promise executor, ahead of drainOutbox's first await — so the frame onEnd's paint coalesces to still lands after #send has stated what is in flight, which is what the PR body claims of the drop that does send.
ci is green on 5044bb6 itself, which closes the signal gap the last review flagged — that review ran the covering subset by hand because ci had never registered on the merge head. It has now, on a base that includes #21.
I also updated the description: it named unrecordedGesture() and applyActions(), neither of which exists on the current base, so a reader grepping the merged tree for either would have found nothing.
The two bug-backs, on the merged tree
Against the review checkout (refs/pull/22/merge, 85631b3 + 5044bb6), which is current here rather than stale:
uv run pytest tests/test_render.py -q -n0 --run-nightly -k "pointer_drag or dragging"
1 passed, 510 deselected in 4.07s
Each edge removed on its own, since one assertion carrying both would not show it:
paintKeys()dropped fromdragginginleaf.js— fails on the drag edge, the line still offeringundounder a live drag.onEnd'sdragging(this, false)swapped for a barethis.classList.remove("lf-dragging"), so the class comes off with nothing painting — fails on the drop edge:AssertionError: the drop that sent nothing left the line refusing a press that is live.
So the test still proves both edges on the post-#21 base, and the new drag-end paint is not standing in for either. Tree restored after each.
The quiet-window premise the docstring rests on also still holds under #21's poll: the syncAsks calls added around it are in presentPage/startPage, page startup rather than the poll loop, and syncAsks otherwise reaches the page through the lf-actions event, which watchProjectionDrag dispatches only after the drag has ended.
…ter (#39) Two test reads are holding `ci` red on `main`, one per commit, and neither can show a green `test` while the other stands — so they are here together rather than in two PRs that would each stay red. They revert independently; nothing in one depends on the other. ## The standing mark's pointer `test_the_page_marks_the_comment_the_reader_is_standing_in` — `Page.wait_for_function: Timeout 30000ms exceeded` waiting for `lf-mark-hover` after the pointer is put on the standing mark. First seen on [run 32779874922](https://github.com/max-sixty/leaf/actions/runs/32779874922); the run that opened this session, [32778837094](https://github.com/max-sixty/leaf/actions/runs/32778837094), failed instead on the note-set axis, which [90ba2a7](90ba2a7) had already fixed four minutes after that run was queued. The walk that puts the reader in a comment opens the panel, and the document slides into its new width over the fifth of a second that follows (`syncLayout`, the slide `panel_settled` was written for). The point handed to `page.mouse.move` is taken on that flight, against a column that is still moving, so the pointer goes where the mark was and the slide carries the mark out from under it. The runtime is right to drop the hover — `pageShifted` recomputes `markAt(pointer.x, pointer.y)` after the reflow, and by then the pointer is over ordinary prose. Measured at the read, on Linux: ``` at the move: body animations ["margin-right"], margin-right 35.3px, point x=358 600ms later: body animations [], margin-right 420px, mark spans x=172.7..236.4 ``` Nothing retries, because a highlight is not an element — so it surfaces 30s later at the hover wait, reading as though the page had stopped lighting what the pointer is on. `panel_settled(page)` before the point is read; it consumes the transition through `getAnimations()`, so it returns on the frame the slide ends rather than encoding the stylesheet's duration. It goes at the geometry read rather than up at the walk, because the three readings in between are about the arrival's lift and are green as they stand. ## The key line's rows `test_a_pointer_drag_stops_the_line_offering_the_press_it_refuses` — `assert 'undo' in '⏎ / space\ngrab the card\nesc\nlet go\n?\nmore'`. New with [141a430](141a430) (#22), which was verified against a base that predates [ee4d444](ee4d444). `renderLine` now paints two chips — the first row and the way out — and hides the rest behind More, whatever the page is offering. So `inner_text` no longer answers what a row's `when` decided: on a board page the widget's own `⏎ / space grab the card` and `esc let go` take both seats, and `z undo` is invisible whether it is live or not. The read after the drop asked for a chip that page can never paint, and the read mid-drag passed on that same permanent absence — proving nothing about the edge it was written for. What `when` decides is whether the row is in the line at all: ``` mid-drag: ⏎/space press it · c comment on the page · d/u half a page · esc let go · g go to · v versions · i design mode · ? more after drop: ⏎/space grab the card · c comment on the page · d/u half a page · z undo · esc let go · g go to · v versions · i design mode · ? more ``` `_painted_line` returns those rows instead of the visible text. A repaint that never came still leaves the rows the previous state put there, so the reading stays once-only and both edges keep their bug-backs. ## Testing Each fix reproduced at its own failure before the change and green after, at `-n0 --run-nightly` on Linux, then again together on the rebase onto current `main` (d3151de): `8 passed` over the two tests and the key-line tests around them. Bug-backs, since a wait added to a timing failure and a widened read are both shapes that can green a test by removing what it proves: | edit | result | | --- | --- | | `paintHover(id)` → `paintHover(null)` in `leaf.js` | standing-mark test fails at the same hover wait, with `panel_settled` in place | | drop `paintKeys()` from `dragging` | `the line offered a press the dispatcher refuses for the length of a drag` | | `onEnd`'s `dragging(this, false)` → bare `classList.remove` | `the drop that sent nothing left the line refusing a press that is live` | `uv run pytest tests -n2` — 494 passed, 6 skipped. `pre-commit` clean on the changed file. The full `--run-nightly` module is left to this PR's own `ci`. --- Automated fix for [failed run](https://github.com/max-sixty/leaf/actions/runs/32778837094) --------- Co-authored-by: leaf-agent <318509791+leaf-agent@users.noreply.github.com> Co-authored-by: Maximilian Roos <m@maxroos.com>
Problem
.lf-draggingis half ofunaccountedGesture(), so a widget wearing it moves core'szrow — andlf-boardwrote the class from four places, of which only the two on the keyboard path painted.#grabadded it and calledpaintKeys()on the next line, under the comment that says why;#releaseremoved it and calledpaintKeys(). The pointer path did neither: Sortable'sonStartended on a bareclassList.add, andonEndopened on the matchingremoveand could leave at itsfrom === to && oldIndex === newIndexearly return before#sendwas ever reached.The window on the first edge is a whole gesture rather than a frame, and nothing else covers it: the focus paint lands on the mousedown and
fallbackTolerance: 4firesonStartafter it,reconcileState()returns at its own.lf-dragginggate so a poll's replay is not a paint, andsyncAsks()is not in the poll loop. So on a quiet board the line went on offeringundofor as long as the reader held the card, over a press the dispatcher was already refusing.onEndis the same gap read backwards: a card dropped where it was picked up takes the class off with nothing following it, so the offer stayed withheld from a press that was live.Solution
The paint moves to the class, in core:
dragging(el, on)toggles.lf-draggingand callspaintKeys(), andlf-board's four sites becomedragging(this, true/false)— the two that already painted along with the two that didn't.Core owning the writer is what closes the gap for the widget after this one. The class is core's own vocabulary and core is its only reader (
unaccountedGesture, and the replay gate inreconcileState), so a duty stated in prose — "whoever moves the state paints" — would leave the twelfth widget to learn it from a docblock or not at all, with a paint that never happened as the symptom: invisible to the file lint, which doesn't read modules, and toversion check --render, which has no wrong artifact to compare against. That is the shapeCLAUDE.mdnames under The widget list is never closed ("Before gating an obligation every adopter must remember, ask whether the declaration already states enough for the layer to do it once"), and the resolution thedata-lf-statemark already took. Here it needs no declaration at all, because the class is already the layer's.The paint coalesces to a frame, which is what lets one call stand for everything else the same gesture moved: the widget's own rows in
#grab, where the grab is a press on an already-focused grip and no focus event fires, and — on the drop that does send — a frame landing after#sendhas stated what is in flight, so it still reads as a gesture the log has not taken.onEndkeeps the call ahead of its branches, because the branch that returns early is the one with no send behind it.Testing
test_a_pointer_drag_stops_the_line_offering_the_press_it_refusesstages a keyboard move so there is a gesture to take back, then pointer-drags that same card by the grip the move left focused — so the mousedown lands on an already-focused control and fires no focusin, leaving the paint under test as the only one that could clear the offer. It reads the line once at each edge, and each read fails on its own edge with that edge's paint removed.Two ways this test passed while proving nothing, and what fixed each
The read has to be once, not
expect. Written asexpect(...).not_to_contain_text("undo"), the mid-drag assertion passed with the fix removed — and so did the assertion after the drop. A probe against unmodifiedmainshows why: mid-drag the class is on and the line still saysz undo, for the whole 0.9s the probe watched, so the defect is real — butexpectre-asks for five seconds, and a version poll two seconds out repaints the line whatever the drag did. That istests/CLAUDE.md's "a state the page passes through is not a state to poll for", read the other way round: the page passes through the asserted state on its way back from a paint nobody asked for._painted_lineconsumes the framepaintHerecoalesces to and reads once.A box measured across the FLIP is a box the card has left. The keyboard move animates the card into its new column, and
bounding_box()taken across that read x=737 where the settled answer was x=1092. The press lands on the grip at that instant and the pointer is somewhere else by the mousemove after it, so Sortable never starts the drag — and the failure arrives aswait_for_selector("lf-board.lf-dragging")timing out on a page nobody dragged, thirty seconds later and nowhere near the cause. The test waits for the card's own animations to empty first.Bug-back. Each edge on its own at
dbb520f, when the paint sat inlf-board:And with the paint now in one place, dropping
paintKeys()fromdraggingtakes out the same assertion (1 failed, 24 passedover-k "board or dragging or pointer_drag").The suite, run the way CI runs it on this Linux runner:
869 passed, 6 skipped in 590.59s.pre-commitis clean on both changed files. One run in between failedtest_a_withdrawn_decision_is_still_withdrawn_after_a_reload, which passes alone and passed on the unmodified branch head under the same load — a contention failure on a page this branch does not touch.Found reviewing #21, which closes the same shape of gap for
sending— the other term of the same predicate. This one reverts independently of that, so it is on its own branch offmain; the two touchleaf.jsin different places.Merged with
mainat #21, which renamedunrecordedGesturetounaccountedGestureand moved the.lf-draggingreplay gate intoreconcileState— the names above are the post-#21 ones, and the mechanism is unchanged under them.