Repository navigation
feat(cursor): add SVG pointer and transient click locator demo - #51
shreyaskarnik merged 6 commits into
Conversation
pseudo-cursor.mp4 |
|
@shreyaskarnik I add skill updates, demo and details tom make pseudo mouse cursor "matching" the project standard |
shreyaskarnik
left a comment
shreyaskarnik
left a comment
There was a problem hiding this comment.
Thank you @OleksandrKucherenko — it's great to see the prototype from #37 turned into a real feature. A few design choices here are clearly right: sending every path point through real page.mouse.move() events (so cursorHighlight and trackCursor keep working on top of it), carrying mode through the existing ARGO_CURSOR_HIGHLIGHT bridge instead of adding a new one, and crediting human-cursor and invisible_playwright in the header.
A few things need to change before this can merge. I checked each one against the branch, not just the diff.
Must change
1. Please drop videos/pseudo-cursor.mp4 from the PR. Video files are kept out of git in this repo (.gitignore covers videos/*.mp4; this one would have been force-added). Every clone carries the file forever, and an earlier branch here was rejected by GitHub because a large video sat in its history. Removing it in a new commit is enough, since the PR is squash-merged. That also means updating the two README references (README.md:190 and :204). Linking a video hosted outside the repo, such as a GitHub attachment, works well here.
2. cursor.click() clicks at coordinates that may no longer be the target (src/human-cursor.ts:177 and :194–199). moveTo reads boundingBox() once. Up to ~1.1s of travel and a 350ms dwell follow, then mouse.down()/mouse.up() fire at that stored point. That skips Playwright's actionability checks. If the layout shifts while the pointer travels (a late image, a toast, an animation), the click lands on whatever is there now, and a demo recording is exactly where a misclick costs the most. A small change keeps the visual travel and restores the checks: after the glide, finish with target.click({ position: { x: box.width * point.x, y: box.height * point.y }, delay: holdMs }). If nothing moved, Playwright lands on the same pixel, so there's no visible jump. If something did move, it waits for the element or retargets.
Should change
3. The z-index change affects existing users, not just the new mode (src/cursor.ts:92, :116, :164). The ring and ripples go from 99998/99997 to 2147483646/2147483645, which puts them above headline cards, lower-thirds and image cards. Anyone already using video.cursorHighlight: true will see their recordings change. If that's intended, a sentence in the README is enough. If not, the higher layer could be limited to mode: 'click'.
4. Control used as a modifier triggers the locator circle (src/cursor.ts:144). Every Control keyup fires locate(), so page.keyboard.press('Control+A') or Control+C in a demo leaves a stray circle at the last mouse position. Checking that no other key was pressed while Control was held would fix it. Separately, only Control is handled, so Meta on macOS does nothing, which may or may not be intended.
Worth a look, not blocking
positionchanges only whenmoveTofinishes (:190). A plainpage.click(), apage.mouse.*call or amoveTothat throws partway leaves it stale, and the next glide then starts with a jump from the old point.- In click mode,
clickRipple: falsealso turns off click feedback. The docs don't mention that.
One note so you don't spend time on it: an automated pass flagged the switch from npx playwright to resolving @playwright/test/cli from Argo's own location as a regression for global installs. I reproduced that setup, and main fails identically there, while loading the config and before recording starts. So it isn't a regression from this PR, and the Windows motivation for the change holds up.
Thanks again. The glide looks great in the recording, and I'm looking forward to the next revision.
cursor.click() now finishes with locator.click({ position, delay }) so
Playwright's actionability checks still run after the glide: if the layout
shifted while the pointer traveled, Playwright waits for or retargets the
element instead of clicking the stale absolute point.
The tracked position no longer updates only when a glide finishes. Real
mouse moves are recorded as they happen (per glide step and through the
patched page.mouse.move), and the SVG glyph — which follows every real
mousemove, including the ones Playwright issues inside locator.click() —
reconciles the tracked position before each glide and after each click.
A glide that throws midway now resumes from the last reached point, and
dispose() unwraps its tracker.
… circles The ring and click ripple drop back to their historical z-index layers (99998/99997, under overlays) in continuous mode, so recordings made before mode:'click' existed render unchanged. The above-overlays layer now belongs exclusively to the click-mode locator circles, which stay below the SVG pointer and above headline cards, lower thirds, and image cards. Tests pin both contracts: exact legacy layers in continuous mode, and locator circles above overlays with the pointer above them in click mode.
…pple is false clickRipple governs the continuous mode's expanding ripple; in click mode the locator circle IS the click feedback, so it now fires regardless of the flag. Documented in the README and the skill reference.
Drop videos/pseudo-cursor.mp4 from the PR — recorded videos stay out of git (the file remains on disk via .gitignore) — and link the GitHub-attachment recording from the README instead. A repo-hygiene guard keeps the demo recording untracked and the README linking externally. Docs now state the behaviors pinned by the tests: clicks press through Playwright's actionability-checked locator.click, Control/Meta locate only on a bare release (chords are ordinary modifiers), clickRipple governs the continuous ripple only, and the above-overlays layer belongs to the click-mode locator circles while the continuous ring keeps its historical layer beneath overlays.
|
All points addressed, each with a failing-test-first cycle (11 new tests; suite at 790 green). (1) The mp4 is out of the PR; the README links the attachment, and a hygiene guard keeps the demo recording untracked. (2) click() now finishes through locator.click({ position, delay }), so actionability checks retarget on layout shift. (3) The continuous ring/ripple are back on 99998/99997 — the above-overlays layer is reserved for click-mode circles (pinned by an e2e). (4) Control/Meta only locate on a bare release; chords like Control+A are ignored, and Meta covers macOS. Also fixed the two non-blocking notes: tracked position now follows every real mouse move (including ones inside locator.click(), reconciled via the SVG glyph), and clickRipple: false no longer disables click-mode click feedback. |
shreyaskarnik
left a comment
shreyaskarnik
left a comment
There was a problem hiding this comment.
Thank you @OleksandrKucherenko, this is excellent work. You went from a prototype in #37 to a finished feature, and addressed every point from the review, including the two I'd marked as non-blocking.
I checked each fix against the code rather than the summary:
- Recording removed.
pseudo-cursor.mp4is out of the tree and the README links the attachment instead. The hygiene test is a nice touch: it guards this recording without tripping over the older showcase videos still onmain. - Clicks respect actionability. Pressing through
target.click({ position, delay })keeps the glide and restores Playwright's checks, so a layout shift during travel no longer lands the click on the wrong element. - Layers. Continuous mode is back on 99998/99997, and only click-mode circles sit above overlays. Existing
cursorHighlightusers won't see any change. - Chords. Only a bare Control or Meta release triggers the circle now, and adding Meta for macOS covers the case I raised.
- Position tracking. Re-syncing from the pointer's on-page position after
locator.click()handles the moves that never pass throughpage.mouse.
The tests do real work. I reverted the click change, the chord guard and the layer split one at a time, and each revert failed the tests you added. Merged locally with current main (which now includes the hyperframes integration), it passed all 891 tests, and CI is green.
One note for the future, not something to change now: tracking works by wrapping page.mouse.move on the page object. Your dispose logic and comments handle several cursors on one page well. If a future Playwright release made page.mouse read-only, this would be the place it breaks, and the e2e suite you wrote would catch it.
Approving and merging. Thanks again, and for the patience through the review round!
|
@shreyaskarnik PR updated |
4 participants
Adds a visible SVG pointer to recorded demos, with brief location feedback instead of a circle following every movement. With
cursorHighlight: { mode: 'click' }, a circle contracts toward the pointer on appearance, click, or Control release, then disappears after 700 ms. Repeated triggers restart one circle; existing continuous-ring behavior remains the default.Exports
createHumanCursor()with reproducible curved movement, pre-click dwell, real Playwright mouse events, navigation restoration, and explicit hide/dispose methods. The SVG and highlight use the same click hotspot and compose above other overlays without intercepting input. Adapts the pointer prototype shared in #37, with attribution in the source.Includes a self-contained HTML playground, recording configuration, scenes manifest, and a 47-second recorded demo. The demo shows three simultaneous overlays, pointer-only travel, click feedback, Control-key location feedback, navigation, styling, and cleanup. No app server or TTS engine is needed.
Also fixes the Windows recording launcher by invoking the installed Playwright CLI through Node instead of executing the
npx.cmdshim, and retains subprocess error details when stdout/stderr are empty.Validation:
npm run buildnode bin/argo.js validate pseudo-cursornode bin/argo.js pipeline pseudo-cursorrun on Windows; inspected exported frames for circle-free movement and transient locator feedback (1280 x 720, 30 fps)git diff --checkCloses #37.