fix: let the guard option veto a blank pointerdown; buttons that click and drag - #3438
Draft
kumilingus wants to merge 9 commits into
Draft
fix: let the guard option veto a blank pointerdown; buttons that click and drag#3438kumilingus wants to merge 9 commits into
kumilingus wants to merge 9 commits into
Conversation
kumilingus
marked this pull request as draft
July 27, 2026 14:57
kumilingus
force-pushed
the
fix/paper-event-surface
branch
from
July 27, 2026 14:57
ca036f1 to
acf847a
Compare
kumilingus
force-pushed
the
fix/paper-event-surface
branch
from
July 27, 2026 14:59
acf847a to
2449b72
Compare
kumilingus
force-pushed
the
fix/paper-event-surface
branch
2 times, most recently
from
July 27, 2026 15:13
ac297e9 to
be7b923
Compare
samuelgja
approved these changes
Jul 28, 2026
kumilingus
force-pushed
the
fix/paper-event-surface
branch
from
July 28, 2026 19:58
a442f71 to
aeab977
Compare
`pointerdown` consulted `guard()` only when the press hit a cell view, so nothing could suppress a blank interaction. That matters for DOM content rendered into `paper.el` - an overlay, a popup, a toolbar - where a press starts a drag the consumer may not want, and where the paper cannot be reasoned with from outside: its listeners are delegated on `paper.el`, so a `stopPropagation()` from content inside it always arrives after the paper has already reacted. Running the full `guard()` there is not an option: it rejects any target off the paper's event surface, so a ruler, a gutter or a toolbar in `paper.el` would stop opening a blank interaction at all - and the whole gesture with it, since the document-level drag listeners are delegated from `pointerdown`. So `guard()` is split in two: - `guardExplicit()` - decisions made about this very event: the right mouse button, the `guard` option, an `evt.data.guarded` flag. Returns a boolean, or `undefined` when none of them has an opinion. - `guard()` - unchanged: `guardExplicit()` first, then judge the target itself (its tag name, its view, whether it is on the event surface). A press that hit no cell view consults only `guardExplicit()`. It opens a blank interaction as it always has, and `options.guard` can now veto it. Behaviour is otherwise unchanged: `GUARDED_TAG_NAMES` still judges the target, so a `<select>` in an overlay is not treated differently. `guard()` also returns a real boolean now - `evt.data.guarded` is set by the caller and was only ever tested against `undefined`, so a `null` or `0` there used to propagate out of a method typed as returning `boolean`. Also types `Paper.Options['guard']`'s `view` parameter as optional: it has always been called without a view from `pointerclick`, `mouseover` and the others. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`<Paper>` children are portaled into `paper.el` but render outside its SVG. A press on such an overlay, popup or toolbar must not open a blank interaction, and React alone cannot prevent one: React attaches its delegated listeners to the portal container, which IS `paper.el` - the node the paper delegates on, and the paper got there first - so a React `onMouseDown` runs after the paper has already reacted. Native `mousedown` / `touchstart` listeners do work, but they break React's own `onMouseDown` on the overlay content. joint-core leaves that press alone, because a plain `dia.Paper` consumer rendering their own content into `paper.el` has always relied on the blank interaction it starts. This overrides `guardExplicit` in the paper preset to reject it here, where portaling is the documented model and driving the canvas from that content is never what is meant. The override runs after the caller's own `options.guard` and defers to `eventSurface`, so content that *should* drive the canvas can still say so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A `<button>` in a node body or in a magnet could only ever be clicked. joint-core blocks every form control from starting an interaction, so there was no way to drag a node by a button inside it, or to start a link from a button in a magnet - which is the natural gesture when the magnet is a row with a control in it. The block came from one flag answering two questions, `FORM_CONTROL_TAG_NAMES` deciding both "keep the browser's default action" and "block paper interactions". A button needs the first and not the second, so the two are now separate lists: - `FORM_CONTROL_TAG_NAMES` - the paper does not call `preventDefault()`, so the control keeps its native behaviour and stays focusable. - `PREVENT_INTERACTION_TAG_NAMES` - a press does not start an element move or a link. Defaults to the same members, so core behaviour is unchanged. joint-react's paper preset drops `BUTTON` from the second list only. To keep one gesture from being both, `pointerup` withholds the next native `click` once the pointer has travelled past `clickThreshold`. joint-core already withholds its own `pointerclick` at that point; the browser does not, because press and release share a target whenever the node follows the pointer - exactly what happens when an element is dragged by a button inside it. Adds a story with both cases: a button in a node body, and two magnets each with a button. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
kumilingus
force-pushed
the
fix/paper-event-surface
branch
from
July 29, 2026 08:06
aeab977 to
bd60631
Compare
Both tag-name lists were tested against `evt.target` alone, so a control with any markup inside it behaved the opposite way round from a bare one. The press target of `<button><span>Save</span></button>` is the SPAN, which is in neither list, so that button lost its default action (no focus) and did start an element move - exactly backwards from a bare `<button>`. `hasTagNameInPath()` walks from the target up to the cell view instead, so a press anywhere inside a control counts as a press on the control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The story demonstrated a button only. It now covers one control per tag list, in a node body and inside a magnet: - `<button>` - dropped from `PREVENT_INTERACTION_TAG_NAMES` by the paper preset, so it both clicks and drags (moving the element, or starting a link from its magnet) - `<input>` - a form control, so it keeps every gesture that stays inside it and selects text rather than dragging - `<select>` - in `GUARDED_TAG_NAMES`, so the event never reaches the paper at all The button's label sits in a `<span>`, so the story also exercises a press landing inside a control rather than on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…_TAG_NAMES `OPTION` is redundant now that the lists are matched against the whole path: an `<option>` only exists inside a `<select>`, which is listed, so the walk finds it. A `<select multiple>` renders its options inline and does deliver real presses on them - covered by a test. Also removes a `FORM_CONTROL_TAG_NAMES` override that had crept into the joint-react preset. It dropped `BUTTON`, which meant the paper called `preventDefault()` on a button press there, so the button never took focus - the exact regression the two-list split exists to avoid, and the opposite of what the comment beside it claimed. joint-react now inherits the list from joint-core. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`parentElement` is defined on `Node` and returns `null` once the parent is not an element, so the explicit `nodeType` test and the `Node` global reference are both redundant. A non-element target now falls through harmlessly too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #3437 — carries its commit unchanged (credit to @samuelgja) and builds on it.
Split out of this PR: the
eventSurfaceoption now lives in #3441.1.
options.guardcan veto a blank pointerdownpointerdownconsultedguard()only when the press hit a cell view, so nothing could suppress a blank interaction. That matters for DOM content rendered intopaper.el— an overlay, a popup, a toolbar — where the paper cannot be reasoned with from outside: its listeners are delegated onpaper.el, so astopPropagation()from content inside it always arrives after the paper has already reacted.Running the full
guard()there is not an option — it rejects any target off the paper's event surface, so a ruler or gutter inpaper.elwould stop opening a blank interaction at all, and the whole gesture with it, since the document-level drag listeners are delegated frompointerdown.So
guard()is split:guardExplicit(evt, view)— decisions about this event: the right mouse button, theguardoption, anevt.data.guardedflag. Returnsboolean | undefined;undefinedmeans nothing decided. The third state is required becauseguarded: falseis an explicit allow that must beat the target tests.guard(evt, view)— unchanged:guardExplicit()first, then judge the target — tag name, view, event surface.A press that hit no cell view consults only
guardExplicit(), so it opens a blank interaction exactly as before andoptions.guardcan now veto it.GUARDED_TAG_NAMESstill judges the target, so a<select>in an overlay is not treated differently — there is a test pinning that.guard()also returns a real boolean now:evt.data.guardedis caller-set and was only ever tested againstundefined, so anullor0used to propagate out of a method typedboolean.2. joint-react keeps portaled content off the surface
<Paper>children are portaled intopaper.elbut render outside its SVG, and a press on one must not drive the canvas. joint-core has to leave that press alone (a plaindia.Paperconsumer rendering intopaper.elrelies on it), so the strict behaviour lives in the joint-react preset, which overridesguardExplicit. It runs after the caller's ownoptions.guard.3. A button inside a node can both click and drag
A
<button>in a node body or a magnet could only ever be clicked, so there was no way to drag a node by a button inside it, or to start a link from a button in a magnet — the natural gesture when the magnet is a row with a control in it.The block came from one flag answering two questions. Now two lists:
FORM_CONTROL_TAG_NAMESpreventDefault) → native click and focusPREVENT_INTERACTION_TAG_NAMESSame members by default, so core behaviour is unchanged. joint-react's preset drops
BUTTONfrom the second list only —FORM_CONTROL_TAG_NAMESis inherited untouched, so a button keeps its native default action and still takes focus on press. (Verified in Chrome:document.activeElementis the button after a click.)To keep one gesture from being both, the preset's
pointerupwithholds the next nativeclickonce the pointer has travelled pastclickThreshold. joint-core already withholds its ownpointerclickat that point; the browser does not, because press and release share a target whenever the node follows the pointer — exactly what happens when an element is dragged by a button inside it.4. Tag lists are matched against the whole path
Both lists were tested against
evt.targetalone. The press target of<button><span>Save</span></button>is the SPAN, which is in neither list — so a button with an icon or a label span inside it behaved the opposite way round from a bare one: it lost its default action (no focus) and did start an element move. Confirmed ondevbefore fixing.hasTagNameInPath()walks from the target up to the cell view, so a press anywhere inside a control counts as a press on the control. Note this is a behaviour change for anyone who was (accidentally) dragging a node by markup nested inside one of these controls.Path matching also makes
OPTIONredundant — an<option>only exists inside a<select>, which is listed — so it is dropped from both lists. Covered by a test using<select multiple>, whose options render inline and do receive real presses.The lists stay exact
tagNamematches rather than becomingclosest()selectors, so all three tag-name options (including the pre-existingGUARDED_TAG_NAMES) keep one matching semantic, and the arrays do not quietly become CSS selector lists.Story
Examples → Buttons In Magnetsshows one control per list, both in a node body and inside a magnet:<button>— clicks and drags; its label sits in a<span>, so it also exercises the nested-target case<input>— keeps every gesture that stays inside it, selecting text rather than dragging<select>— guarded outright; the event never reaches the paperTyping
Paper.Options['guard']now declaresviewoptional. It has always been called without a view frompointerclick,pointerdblclick,mouseoverand the rest. Custom guards that dereferenceviewwill now fail to compile — that surfaces a latent bug rather than creating one.Tests
guardveto, the<select>bit-exactness case, the two-list split, and a press inside a<button>test:tsand lint cleanNote for reviewers: joint-react's jest resolves
@joint/coretopackages/joint-core/dist/joint.min.js, so that suite only exercises core changes after ayarn dist.🤖 Generated with Claude Code