Skip to content

fix(memory-graph): allow selecting nodes on touch devices#1262

Merged
ishaanxgupta merged 2 commits into
supermemoryai:mainfrom
abhay-codes07:fix/graph-touch-tap-selection
Jul 22, 2026
Merged

fix(memory-graph): allow selecting nodes on touch devices#1262
ishaanxgupta merged 2 commits into
supermemoryai:mainfrom
abhay-codes07:fix/graph-touch-tap-selection

Conversation

@abhay-codes07

Copy link
Copy Markdown
Contributor

Fixes #1258

What

Selecting a node in the memory graph was impossible on touch devices. Selection rides on the canvas click event, but onTouchStart calls preventDefault() (required to keep the page from scrolling/zooming), which also suppresses the browser-synthesized mouse events — so the click never fires. A single-finger touch went straight into panning, leaving phones and tablets with pan/pinch only: no selection, no popover, no way to open a document from the graph.

Fix

Detect taps from the raw touch lifecycle in InputHandler:

  • remember where a single-finger touch starts
  • cancel the tap once the finger travels more than a 10px slop threshold or a second finger lands (pinch)
  • on release, hit-test the start point through the current viewport transform and forward the result to onNodeClick

Tapping a node selects it (popover opens); tapping empty space clears the selection — matching mouse behaviour. Pan and pinch are untouched.

Testing

  • New unit tests drive the handler through a stub canvas and real ViewportState/SpatialIndex instances: tap on node, tap on empty space, jitter within the slop threshold, pan, pinch, and a tap through a zoomed/panned viewport (src/__tests__/input-handler-touch.test.ts)
  • bun run test in packages/memory-graph — 194 pass
  • bun run check-types — clean

cc @MaheshtheDev

Node selection only worked through the canvas click handler, but
onTouchStart calls preventDefault() to stop page scroll/zoom, which
also suppresses the browser-synthesized click. A single-finger touch
went straight into panning, so on phones and tablets tapping a node
did nothing: no selection, no popover, no way to open a document from
the graph.

Detect taps from the raw touch lifecycle instead: remember where the
single-finger touch started, cancel the tap once the finger moves more
than 10px or a second finger lands, and on release hit-test the start
point through the viewport transform and forward the result to
onNodeClick. Tapping empty space clears the selection, matching mouse
behaviour.

Covered with unit tests driving the handler through a stub canvas:
tap on node, tap on empty space, jitter within the slop threshold,
pan, pinch, and a tap through a zoomed/panned viewport.
Copilot AI review requested due to automatic review settings July 12, 2026 18:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ishaanxgupta

Copy link
Copy Markdown
Contributor

Hi @abhay-codes07,
During a small movement below TAP_MOVE_THRESHOLD, onTouchMove still pans the viewport. Then onTouchEnd hit-tests touchStartX/touchStartY using the already-updated viewport transform. This means the original screen point no longer necessarily maps to the node that was tapped, especially when the graph is zoomed out. A normal few-pixel finger jitter can therefore still cause the tap to miss.

Please either:

  • store the world coordinate/target node at touchstart and use it on release, or
  • hit-test using the latest touch position (lastMouseX/lastMouseY) after the viewport has moved.

A sub-threshold pan during a tap still moves the viewport, so re-projecting
the start screen point through the panned transform on touchend can resolve
the wrong node (or miss entirely) when zoomed out. Capture the world point
under the finger at touchstart, before any panning, and hit-test against that
on release. Adds a regression test that jitters below the tap threshold at low
zoom where the old re-projection lands off the node.
@abhay-codes07

Copy link
Copy Markdown
Contributor Author

Good catch, you are right. The sub-threshold pan in onTouchMove moves the viewport, and onTouchEnd was re-projecting touchStartX/touchStartY through that already-moved transform, so at low zoom a few-pixel jitter shifts the hit point off the node.

Fixed by going with your first option: I capture the world point under the finger at touchstart (before any panning) and hit-test against that on release, instead of re-projecting the start screen point. The stored world coordinate is stable regardless of how the viewport pans during the tap.

Added a regression test that reproduces exactly this: at zoom 0.25 an 8px jitter (below the 10px threshold) pans the viewport, and the old re-projection landed 32 world units off the node (past the ~20 unit hit radius) and missed. It now resolves the node correctly. Pushed in ad70ef8.

@ishaanxgupta
ishaanxgupta merged commit d5f9582 into supermemoryai:main Jul 22, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Memory graph: nodes cannot be selected on touch devices

3 participants