Skip to content

Commit aeab977

Browse files
kumilingusclaude
andcommitted
feat: let a button inside a node both click and drag
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>
1 parent a1d07ba commit aeab977

7 files changed

Lines changed: 452 additions & 7 deletions

File tree

packages/joint-core/src/dia/Paper.mjs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -597,9 +597,16 @@ export const Paper = View.extend({
597597
_layers: null,
598598

599599
UPDATE_DELAYING_BATCHES: ['translate'],
600-
// If you interact with these elements,
601-
// the default interaction such as `element move` is prevented.
602-
FORM_CONTROL_TAG_NAMES: ['TEXTAREA', 'INPUT', 'BUTTON', 'SELECT', 'OPTION'] ,
600+
// If you interact with these elements, the browser's own default action is kept
601+
// (the paper does not call `preventDefault()`), so a text input can be focused and
602+
// its text selected, a checkbox can be ticked, a button can be pressed.
603+
FORM_CONTROL_TAG_NAMES: ['TEXTAREA', 'INPUT', 'BUTTON', 'SELECT', 'OPTION'],
604+
// If you interact with these elements, the default interaction such as `element move`
605+
// or starting a link from a magnet is prevented, i.e. a press is only ever a click.
606+
// The same members as above by default, but a separate decision: narrow this list to
607+
// let a control both be clicked and start a drag - dropping `BUTTON`, say, makes a
608+
// button inside a magnet draggable to create a link while it stays clickable.
609+
PREVENT_INTERACTION_TAG_NAMES: ['TEXTAREA', 'INPUT', 'BUTTON', 'SELECT', 'OPTION'],
603610
// If you interact with these elements, the events are not propagated to the paper
604611
// i.e. paper events such as `element:pointerdown` are not triggered.
605612
GUARDED_TAG_NAMES: [
@@ -3499,16 +3506,16 @@ export const Paper = View.extend({
34993506

35003507
if (view) {
35013508

3502-
const isTargetFormNode = this.FORM_CONTROL_TAG_NAMES.includes(target.tagName);
3509+
const { tagName } = target;
35033510

3504-
if (this.options.preventDefaultViewAction && !isTargetFormNode) {
3511+
if (this.options.preventDefaultViewAction && !this.FORM_CONTROL_TAG_NAMES.includes(tagName)) {
35053512
// If the target is a form element, we do not want to prevent the default action.
35063513
// For example, we want to be able to select text in a text input or
35073514
// to be able to click on a checkbox.
35083515
evt.preventDefault();
35093516
}
35103517

3511-
if (isTargetFormNode) {
3518+
if (this.PREVENT_INTERACTION_TAG_NAMES.includes(tagName)) {
35123519
// If the target is a form element, we do not want to start dragging the element.
35133520
// For example, we want to be able to select text by dragging the mouse.
35143521
view.preventDefaultInteraction(evt);

packages/joint-core/test/jointjs/paper.js

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1451,6 +1451,46 @@ QUnit.module('paper', function(hooks) {
14511451
'the element moved when dragging from its body');
14521452
});
14531453

1454+
QUnit.test('PREVENT_INTERACTION_TAG_NAMES is separate from FORM_CONTROL_TAG_NAMES', function(assert) {
1455+
1456+
// The two lists start out identical but answer different questions:
1457+
// FORM_CONTROL_TAG_NAMES keeps the browser's default action (no `preventDefault`),
1458+
// PREVENT_INTERACTION_TAG_NAMES blocks the paper's own interactions. Narrowing only
1459+
// the second one is what lets a <button> be clicked AND dragged - to move the
1460+
// element, or to start a link when it sits in a magnet.
1461+
assert.deepEqual(this.paper.PREVENT_INTERACTION_TAG_NAMES, this.paper.FORM_CONTROL_TAG_NAMES,
1462+
'the defaults match, so the split changes nothing on its own');
1463+
assert.notStrictEqual(this.paper.PREVENT_INTERACTION_TAG_NAMES, this.paper.FORM_CONTROL_TAG_NAMES,
1464+
'but they are distinct arrays, so overriding one leaves the other alone');
1465+
1466+
const element = new joint.shapes.standard.Rectangle({
1467+
position: { x: 100, y: 100 },
1468+
size: { width: 100, height: 100 },
1469+
markup: joint.util.svg`
1470+
<foreignObject @selector="fo" width="100" height="100">
1471+
<button @selector="button" type="button">click me</button>
1472+
</foreignObject>
1473+
`
1474+
});
1475+
this.graph.addCell(element);
1476+
1477+
const elementView = this.paper.findViewByModel(element);
1478+
const buttonEl = elementView.findNode('button');
1479+
1480+
// Let a press on a <button> start an interaction, while it keeps its native default.
1481+
this.paper.PREVENT_INTERACTION_TAG_NAMES = ['TEXTAREA', 'INPUT', 'SELECT', 'OPTION'];
1482+
1483+
const positionBefore = element.position();
1484+
const mousedownEvt = simulate.mousedown({ el: buttonEl, clientX: 150, clientY: 150 });
1485+
simulate.mousemove({ el: buttonEl, clientX: 250, clientY: 250 });
1486+
simulate.mouseup({ el: buttonEl, clientX: 250, clientY: 250 });
1487+
1488+
assert.notDeepEqual(element.position(), positionBefore,
1489+
'the element now moves when dragging from a <button>');
1490+
assert.notOk(mousedownEvt.defaultPrevented,
1491+
'the button keeps its default action, so the native click and focus still work');
1492+
});
1493+
14541494
QUnit.test('getContentArea()', function(assert) {
14551495

14561496
assert.checkBboxApproximately(2/* +- */, this.paper.getContentArea(), {

packages/joint-core/types/dia.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1994,6 +1994,7 @@ export class Paper extends mvc.View<Graph> {
19941994

19951995
GUARDED_TAG_NAMES: string[];
19961996
FORM_CONTROL_TAG_NAMES: string[];
1997+
PREVENT_INTERACTION_TAG_NAMES: string[];
19971998

19981999
matrix(): SVGMatrix;
19992000
matrix(ctm: SVGMatrix | Vectorizer.Matrix, data?: any): this;
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
import { useCallback } from 'react';
2+
import { render, waitFor } from '@testing-library/react';
3+
import { GraphProvider, Paper } from '../../components';
4+
import { usePaper } from '../../hooks/use-paper';
5+
import type { PaperView } from '../../mvc/paper';
6+
import { ELEMENT_MODEL_TYPE } from '../../mvc/element-model';
7+
import type { CellRecord } from '../../types/cell.types';
8+
9+
// A button inside a node must be clickable AND draggable: dragging it moves the element
10+
// (or starts a link when it sits in a magnet), while a press-and-release in place is just
11+
// a click. joint-core blocks every form control from starting an interaction, so the
12+
// preset narrows `PREVENT_INTERACTION_TAG_NAMES` to leave `BUTTON` out — and `pointerup`
13+
// withholds the native click once the gesture has moved, since a node dragged by its own
14+
// button ends the gesture back on that button and would otherwise fire `onClick`.
15+
16+
const ELEMENT_ID = 'cell-1';
17+
18+
beforeAll(() => {
19+
// jsdom has no layout, so it ships no `elementFromPoint`; joint-core reaches for it
20+
// while a drag is in flight (`CellView#getEventTarget`). Nothing here depends on the
21+
// node it would return.
22+
document.elementFromPoint = () => null;
23+
});
24+
25+
const initialCells: readonly CellRecord[] = [
26+
{
27+
id: ELEMENT_ID,
28+
type: ELEMENT_MODEL_TYPE,
29+
position: { x: 0, y: 0 },
30+
size: { width: 80, height: 40 },
31+
} as CellRecord,
32+
];
33+
34+
let capturedPaper: PaperView | null = null;
35+
let capturedButton: HTMLButtonElement | null = null;
36+
37+
function Capture() {
38+
capturedPaper = usePaper().paper;
39+
return null;
40+
}
41+
42+
const PAPER_STYLE = { width: 200, height: 200 };
43+
44+
/** Reassigned per test, read at render time by the button's `onClick`. */
45+
let onButtonClick: () => void = () => {};
46+
47+
function NodeWithButton() {
48+
const buttonRef = useCallback((node: HTMLButtonElement | null) => {
49+
capturedButton = node;
50+
}, []);
51+
return (
52+
<>
53+
<rect width={80} height={40} fill="#1c2836" />
54+
<foreignObject width={80} height={40}>
55+
<button type="button" ref={buttonRef} onClick={onButtonClick}>
56+
press
57+
</button>
58+
</foreignObject>
59+
</>
60+
);
61+
}
62+
63+
const renderElement = () => <NodeWithButton />;
64+
65+
/**
66+
* Mount a paper holding one element whose markup contains a `<button>`.
67+
* @param clickSpy - Spy for the button's React `onClick`.
68+
* @returns The paper and the rendered `<button>`.
69+
*/
70+
async function renderNodeWithButton(
71+
clickSpy: () => void
72+
): Promise<{ paper: PaperView; button: HTMLButtonElement }> {
73+
capturedPaper = null;
74+
capturedButton = null;
75+
onButtonClick = clickSpy;
76+
render(
77+
<GraphProvider initialCells={initialCells}>
78+
<Paper style={PAPER_STYLE} renderElement={renderElement}>
79+
<Capture />
80+
</Paper>
81+
</GraphProvider>
82+
);
83+
await waitFor(() => {
84+
expect(capturedPaper).not.toBeNull();
85+
expect(capturedButton).not.toBeNull();
86+
});
87+
const paper = capturedPaper;
88+
const button = capturedButton;
89+
if (!paper || !button) throw new Error('paper or button not mounted');
90+
return { paper, button };
91+
}
92+
93+
function dispatchAt(node: EventTarget, type: string, clientX: number, clientY: number) {
94+
node.dispatchEvent(new MouseEvent(type, { clientX, clientY, bubbles: true, cancelable: true }));
95+
}
96+
97+
/** A real browser fires `click` on the pressed node after `mouseup` — jsdom does not. */
98+
function dispatchClick(node: EventTarget) {
99+
dispatchAt(node, 'click', 0, 0);
100+
}
101+
102+
describe('a button inside a node', () => {
103+
it('moves the element when dragged, and does not click', async () => {
104+
const clickSpy = jest.fn();
105+
const { paper, button } = await renderNodeWithButton(clickSpy);
106+
const element = paper.model.getCell(ELEMENT_ID);
107+
const positionBefore = { ...element.get('position') };
108+
109+
dispatchAt(button, 'mousedown', 10, 10);
110+
// `clickThreshold` counts move events, not pixels, so a drag has to be more than a
111+
// single jump for joint-core to stop treating the gesture as a click.
112+
for (let step = 1; step <= 8; step += 1) {
113+
dispatchAt(document, 'pointermove', 10 + step * 10, 10 + step * 10);
114+
}
115+
dispatchAt(document, 'pointerup', 90, 90);
116+
dispatchClick(button);
117+
118+
expect(element.get('position')).not.toEqual(positionBefore);
119+
expect(clickSpy).not.toHaveBeenCalled();
120+
});
121+
122+
it('clicks when pressed without moving, and does not move the element', async () => {
123+
const clickSpy = jest.fn();
124+
const { paper, button } = await renderNodeWithButton(clickSpy);
125+
const element = paper.model.getCell(ELEMENT_ID);
126+
const positionBefore = { ...element.get('position') };
127+
128+
dispatchAt(button, 'mousedown', 10, 10);
129+
dispatchAt(document, 'pointerup', 10, 10);
130+
dispatchClick(button);
131+
132+
expect(element.get('position')).toEqual(positionBefore);
133+
expect(clickSpy).toHaveBeenCalledTimes(1);
134+
});
135+
});

packages/joint-react/src/presets/paper.ts

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,31 @@ function getPointerId(event: dia.Event): number | null {
4343
return typeof fromOriginal === 'number' ? fromOriginal : null;
4444
}
4545

46+
/**
47+
* Swallow the next native `click`, so a gesture that moved does not also click.
48+
*
49+
* joint-core already withholds its own `pointerclick` once the pointer travels past
50+
* `clickThreshold`, but the browser still fires a DOM `click` whenever press and release
51+
* share a target — which a drag does whenever the node follows the pointer, as when an
52+
* element is moved by a button inside it. Without this, dragging a node by its button
53+
* would run the button's `onClick` on release.
54+
*
55+
* Capture on `document` rather than `paper.el`: React attaches its listeners to the
56+
* portal container, so a listener on `paper.el` could be ordered behind them.
57+
*/
58+
function swallowClick(event: Event): void {
59+
event.stopPropagation();
60+
event.preventDefault();
61+
}
62+
63+
function swallowNextClick(): void {
64+
document.addEventListener('click', swallowClick, { capture: true, once: true });
65+
// A drag released off the pressed node fires no click at all; drop the listener so it
66+
// cannot eat an unrelated one later. Re-adding the same function is a no-op, so
67+
// overlapping gestures cannot stack listeners.
68+
setTimeout(() => document.removeEventListener('click', swallowClick, true), 0);
69+
}
70+
4671
const DEFAULT_CLICK_THRESHOLD = 5;
4772
const DEFAULT_GRID_SIZE = 10;
4873
const DEFAULT_SNAP_RADIUS = 15;
@@ -137,6 +162,8 @@ export const Paper = dia.Paper.extend(
137162

138163
documentEvents: POINTER_DOCUMENT_EVENTS,
139164

165+
FORM_CONTROL_TAG_NAMES: ['TEXTAREA', 'INPUT', 'SELECT', 'OPTION'] ,
166+
140167
// Cell focus tracking. `focusin`/`focusout` bubble (unlike `focus`/`blur`),
141168
// so they delegate to `.joint-cell` and report which cell owns the focused
142169
// node (e.g. a `tabindex` element) via the `cell:focus` / `cell:blur` paper
@@ -258,15 +285,34 @@ export const Paper = dia.Paper.extend(
258285
return isPortaledContent ? true : undefined;
259286
},
260287

288+
/**
289+
* Let a press on a `<button>` start a paper interaction — an element move, or a link
290+
* when the button sits inside a magnet. joint-core blocks every form control here,
291+
* which makes a button click-only; buttons are the one control whose whole purpose is
292+
* a press, so a drag from one is unambiguous. `FORM_CONTROL_TAG_NAMES` is left alone,
293+
* so the button keeps its native default action and stays focusable.
294+
*
295+
* A drag that moves does not also click: `pointerup` swallows the native click.
296+
*/
297+
PREVENT_INTERACTION_TAG_NAMES: ['TEXTAREA', 'INPUT', 'SELECT', 'OPTION'],
298+
261299
/**
262300
* Run the upstream pointerup handler, then release capture. Also runs on
263301
* `pointercancel` (mapped to the same method via the events hash) so
264302
* OS-stolen pointers don't leave listeners attached.
303+
*
304+
* Also withholds the next native `click` when the gesture moved, so a drag started
305+
* from a button does not fire its `onClick` on release.
265306
* @param event - The pointerup or pointercancel event.
266307
*/
267308
pointerup(this: dia.Paper, event: dia.Event) {
268309
const pointerId = getPointerId(event);
269-
const captureTarget = this.eventData(event).captureTarget as Element | undefined;
310+
const { captureTarget, mousemoved = 0 } = this.eventData(event) as {
311+
captureTarget?: Element;
312+
mousemoved?: number;
313+
};
314+
// Read before the super call: it ends the gesture and resets this state.
315+
if (mousemoved > (this.options.clickThreshold ?? 0)) swallowNextClick();
270316
protectedProto.pointerup.call(this, event);
271317
if (!captureTarget || pointerId === null) return;
272318
this.el.classList.remove(DRAGGING_CLASS_NAME);

0 commit comments

Comments
 (0)