Draw the novice feedback as a straight line, above the menu - #238
Conversation
The novice move transition appended each incoming position to the upper stroke, so the feedback traced every wobble of the pointer's path from the menu center instead of showing where the pointer is relative to the menu. Replace the stroke with the two-point segment from the menu center to the position instead. Startup and expert keep accumulating, since their stroke is the mark being drawn. The three consumers of the field read the same value, so they follow: the layout projection, the submenu-open transition that folds the upper stroke into the lower one, and the termination context that builds the completed-gesture stroke. The post-gesture trace is a straight polyline again, one segment per menu level. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The renderer creates its canvases once and keeps them, and nothing else sets a paint order, so the layers ended up wherever append order left them. The lower stroke, which records movement made before the menu opened, covered the menu at root depth, and opening a submenu recreated the menu element and pushed both strokes behind it, hiding the origin marker and cutting the line where it crossed the menu's item lines. Re-assert the order after every render: the lower stroke behind the menu, the upper stroke and its marker in front of it. An element only moves when it is out of place. `StrokeCanvas` and `Menu` now expose their element so the renderer can place them. The alternative, giving the three layers explicit z-index values, would turn an internal invariant into styling that consumers can override. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t canvas helper The renderer's stacking note and the menu stylesheet covered the two stroke canvases but left out the completed-gesture feedback, which also belongs in front of the menu. `canvasContext` moves into the shared canvas fixture so the renderer suite's layer check and `queryCanvasContext` unwrap a stubbed context the same way, and `queryCanvasContext` now throws a named error rather than calling through a missing canvas. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
QuentinRoy
left a comment
QuentinRoy
left a comment
There was a problem hiding this comment.
Standards
No documented-standard violations or actionable code smells found.
Spec
One P2 finding: completed-gesture feedback can fall behind a new menu opened before the trace expires. This leaves the stacking contract in #237 partially implemented; details are attached to renderer.ts. No other actionable spec mismatches or scope creep found.
Validation: all 47 tests in machine.test.ts and renderer.test.ts pass. An additional isolated DOM test reproduces the ordering failure: show completed feedback, start another gesture, advance 334 ms, and open its novice menu. The feedback canvas appears before the menu in sibling order, so it paints underneath it.
Standards: 0 findings. Spec: 1 finding; worst severity P2, feedback stacking across consecutive gestures.
| The completed-gesture feedback belongs in front of the menu too, which | ||
| it gets for free: `showFeedback` only ever runs once the menu is gone, | ||
| and appends its own canvas. |
There was a problem hiding this comment.
[P2] Keep surviving feedback above the next menu
Completed feedback lasts 1,000 ms by default. If another gesture opens a menu before that trace expires, the new menu is appended after the feedback canvas. restack() only reorders the upper and lower strokes, so the completed trace paints underneath the new menu. Calling showFeedback() after the previous menu closes does not preserve the order across gestures.
Issue #237 requires "lower stroke behind the menu, upper stroke and completed-gesture feedback in front of it." Include surviving feedback canvases when maintaining that order, and add a DOM regression test that shows feedback before opening another menu. An isolated test with a 334 ms delay reproduces the failure with the default feedback duration.
Novice state stored the upper stroke as an array that only ever held the menu center and the pointer, which the machine had to rebuild on every move and every consumer had to trust was exactly those two points. Store `lastPosition` instead and let `noviceUpperStroke` build the segment for the three places that read it: the layout projection, the submenu open that folds it into the lower stroke, and the completed-gesture stroke. The return type is a pair, so the shape is stated rather than assumed. A menu that has not been moved on yet now reports its segment as its center twice, where the old array held a single point. The line drawn is the same, and the completed-gesture stroke is unchanged, since it already repeated each center where one level's segment met the next. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A completed-gesture trace stays up for a second by default, so a menu opened before it fades was appended after it and covered it. The render that opens that menu now moves any trace still showing back in front of it, after the upper stroke, so a live gesture draws over a fading one. `GestureFeedback` exposes the canvases still showing, the same way the stroke canvases and the menu expose their element. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Dwelling to open the novice menu and then dragging produced two wrong things at once. The line from the menu center followed every wobble of the pointer's path instead of pointing straight at it, so a slow or curved drag left a scribble across the menu. And the stroke was painted underneath the menu, hiding the round marker that shows where the current menu is anchored and cutting the line wherever it crossed an item.
Both are regressions in unreleased code, from separate causes, so they are fixed separately.
The novice
movetransition appended each new position to the upper stroke. Novice state now storeslastPositionand nothing else, andnoviceUpperStrokebuilds the two-point segment from the menu center for the three places that read it: the layout projection, the submenu open that folds it into the lower stroke, and the completed-gesture stroke. The segment is a pair by type, so nothing has to assume it. Startup and expert are untouched and keep accumulating, because there the stroke is the mark being drawn. One consequence worth knowing: a menu nobody has moved on yet reports its segment as its center twice, where the old array held one point. The line drawn is the same and the completed-gesture stroke is byte-identical.The renderer creates its canvases once and keeps them, and neither they nor the menu set a paint order, so the layers ended up wherever append order left them.
rendernow re-asserts the order it needs, lower stroke behind the menu and upper stroke in front of it, moving an element only when it is out of place. Giving the three layersz-indexvalues was the alternative, and was rejected because it turns an internal invariant into styling a consumer can override. The rule is written down in the renderer and inmenu.cssso the next rewrite inherits it.To check by hand, run
yarn demo:dev, hold the pointer still to open the menu, then drag slowly along a curve. The line stays straight and the origin dot stays visible.The tests sit at the two seams that already existed. The machine ones drive it with pointer inputs and assert on the
layoutandfeedbackannouncements rather than its internal state, so the stroke representation stays free to change. The renderer ones render a view into a fresh parent and assert the sibling order under it, including after a submenu replaces the menu element, which is the path the stacking fix could regress.No changeset: the public event set and payloads are untouched, and both defects live in code that has not shipped.
Fixes #237
🤖 Generated with Claude Code