Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/react-on-cells-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@joint/react": minor
---

useOnCellsChange - add the hook, with the `selectMeasuredState`, `selectIsMeasured` and `selectElementsSizes` selectors
`useOnElementsMeasured` is deprecated in favor of it.
5 changes: 5 additions & 0 deletions .changeset/react-store-callbacks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@joint/react": patch
---

<GraphProvider /> - fix a store callback that throws or changes the graph breaking later updates
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ import type * as ElementModelModule from '../../../mvc/element-model';
import type * as LinkModelModule from '../../../mvc/link-model';
import type * as UseGraphStoreModule from '../../../hooks/use-graph-store';
import type * as UseCellIdsModule from '../../../hooks/use-cell-ids';
import type * as UseOnElementsMeasuredModule from '../../../hooks/use-on-elements-measured';
import type * as UseOnCellsChangeModule from '../../../hooks/use-on-cells-change';
import type * as SelectorsModule from '../../../selectors';
import type * as GraphProviderModule from '../graph-provider';
import type * as PaperModule from '../../paper/paper';

Expand Down Expand Up @@ -88,8 +89,11 @@ const linkModelModule: typeof LinkModelModule = require('../../../mvc/link-model
const { useGraphStore }: typeof UseGraphStoreModule = require('../../../hooks/use-graph-store');
const { useCellIds }: typeof UseCellIdsModule = require('../../../hooks/use-cell-ids');
const {
useOnElementsMeasured,
}: typeof UseOnElementsMeasuredModule = require('../../../hooks/use-on-elements-measured');
useOnCellsChange,
}: typeof UseOnCellsChangeModule = require('../../../hooks/use-on-cells-change');
const {
selectMeasuredState,
}: typeof SelectorsModule = require('../../../selectors');

const graphProviderV1: typeof GraphProviderModule = require('../graph-provider');
const paperV1: typeof PaperModule = require('../../paper/paper');
Expand Down Expand Up @@ -191,8 +195,8 @@ function Probe() {
mountSequence += 1;
return mountSequence;
});
useOnElementsMeasured(({ isInitial }) => {
measuredCalls.push(isInitial);
useOnCellsChange(selectMeasuredState, (version, previousMeasuredState) => {
if (version) measuredCalls.push(!previousMeasuredState);
});
const ids = useCellIds();
return h(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React, { memo, useLayoutEffect, useRef } from 'react';
import { useImperativeApi } from '../../hooks/use-imperative-api';
import { GraphStoreContext } from '../../context';
import { GraphStore } from '../../store';
import type { AutoSizeOrigin } from '../../store/graph-store';
import type { AutoSizeOrigin } from '../../store/measurement';
import type { OnIncrementalCellsChange } from '../../store/graph-projection';
import type { ElementJSONInit, LinkJSONInit, CellInput } from '../../types/cell.types';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import { CellIdContext, GraphStoreContext, PaperStoreContext } from '../../../..
import { ELEMENT_MODEL_TYPE } from '../../../../mvc/element-model';
import type { CellRecord, CellId } from '../../../../types/cell.types';

const RenderEmpty: ComponentType<Record<string, unknown>> = () => <span data-testid="render-empty" />;
const RenderEmpty: ComponentType<Record<string, unknown>> = () => (
<span data-testid="render-empty" />
);

const CELLS: readonly CellRecord[] = [
{
Expand All @@ -27,7 +29,11 @@ const CELLS: readonly CellRecord[] = [
* can re-mount SVG / HTML element items with `portalElement={null}` for the
* defensive guard branches.
*/
function StoreCapture({ onCapture }: { readonly onCapture: (graph: unknown, paper: unknown) => void }) {
function StoreCapture({
onCapture,
}: {
readonly onCapture: (graph: unknown, paper: unknown) => void;
}) {
const graphStore = useContext(GraphStoreContext);
const paperStore = useContext(PaperStoreContext);
if (graphStore && paperStore) onCapture(graphStore, paperStore);
Expand Down Expand Up @@ -63,11 +69,7 @@ describe('paper-element-item exports', () => {
value={capturedPaper as React.ContextType<typeof PaperStoreContext>}
>
<CellIdContext.Provider value={'one' as CellId}>
<SVGElementItem
renderElement={RenderEmpty}
portalElement={null}
areElementsMeasured
/>
<SVGElementItem renderElement={RenderEmpty} portalElement={null} />
</CellIdContext.Provider>
</PaperStoreContext.Provider>
</GraphStoreContext.Provider>
Expand Down Expand Up @@ -101,11 +103,7 @@ describe('paper-element-item exports', () => {
value={capturedPaper as React.ContextType<typeof PaperStoreContext>}
>
<CellIdContext.Provider value={'one' as CellId}>
<HTMLElementItem
renderElement={RenderEmpty}
portalElement={null}
areElementsMeasured
/>
<HTMLElementItem renderElement={RenderEmpty} portalElement={null} />
</CellIdContext.Provider>
</PaperStoreContext.Provider>
</GraphStoreContext.Provider>
Expand Down Expand Up @@ -143,18 +141,16 @@ describe('paper-element-item exports', () => {
value={capturedPaper as React.ContextType<typeof PaperStoreContext>}
>
<CellIdContext.Provider value={'missing-cell-id' as CellId}>
<HTMLElementItem
renderElement={RenderEmpty}
portalElement={portalTarget}
areElementsMeasured
/>
<HTMLElementItem renderElement={RenderEmpty} portalElement={portalTarget} />
</CellIdContext.Provider>
</PaperStoreContext.Provider>
</GraphStoreContext.Provider>
);

// Placeholder wrapper should still be created with id and zero geometry.
const wrapper = portalTarget.querySelector('div[model-id="missing-cell-id"]') as HTMLDivElement | null;
const wrapper = portalTarget.querySelector(
'div[model-id="missing-cell-id"]'
) as HTMLDivElement | null;
expect(wrapper).toBeTruthy();
expect(wrapper?.style.width).toBe('0px');
expect(wrapper?.style.height).toBe('0px');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,18 @@ export interface ElementItemProps {
readonly renderElement: ComponentType<Record<string, unknown>>;
/** The DOM element to portal into. */
readonly portalElement: SVGElement | HTMLElement | null;
/** Whether all auto-sized elements have been measured. */
readonly areElementsMeasured: boolean;
}

/**
* SVG element portal component. Subscribes only to the element's `data`
* slice, position and size are handled by JointJS's view transform and
* never cause a React re-render here. Clears cached views after
* measurement to force re-render with correct dimensions.
* never cause a React re-render here. Clears the cached view once its content
* is committed, so links resolve against the rendered magnets.
* @param props - render/portal props
* @internal
*/
function SVGElementItemComponent(props: ElementItemProps) {
const { renderElement: RenderElement, portalElement, areElementsMeasured } = props;
const { renderElement: RenderElement, portalElement } = props;
const id = useCellId();
// Subscribe to just this element's `data` slice (missing-tolerant — the portal
// can mount before the record lands in the store, and briefly after removal).
Expand All @@ -54,13 +52,13 @@ function SVGElementItemComponent(props: ElementItemProps) {
// inside `renderElement` has already registered with the size observer
// (in HTML overlay mode this item follows `HTMLElementItem`, which renders
// the user content, in sibling order). O(1), and a no-op on re-runs.
graphStore.markElementRendered(id);
graphStore.measurement.markRendered(id);
if (!paper) return;
graphStore.clearViewForElementAndLinks({
cellId: id,
paper,
});
}, [id, graphStore, areElementsMeasured, paper]);
}, [id, graphStore, paper]);

if (!portalElement) {
return null;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
/**
* Specification for when `useOnElementsMeasured` delivers an event.
* Specification for when the measurement version (`selectMeasuredState`)
* changes, written as the events a `useOnCellsChange` subscriber receives. It
* was written for the former `useOnElementsMeasured` hook; the cases and their
* expectations are unchanged.
*
* The hook exists so an application can run a layout once element sizes are
* known. That only works if one settled change delivers exactly one event: a
Expand All @@ -23,10 +26,11 @@ import { render, waitFor, act } from '@testing-library/react';
import { GraphProvider } from '../../components/graph/graph-provider';
import { Paper } from '../../components/paper/paper';
import { HTMLHost } from '../../components/html-host';
import { useOnElementsMeasured } from '../use-on-elements-measured';
import { useOnCellsChange } from '../use-on-cells-change';
import { useGraphStore } from '../use-graph-store';
import { ELEMENT_MODEL_TYPE } from '../../mvc/element-model';
import { AUTO_SIZE_OPTION } from '../../store/graph-store';
import { AUTO_SIZE_OPTION } from '../../store/measurement';
import { selectMeasuredState } from '../../selectors';
import type { CellRecord } from '../../types/cell.types';
import type { dia } from '@joint/core';

Expand Down Expand Up @@ -101,8 +105,10 @@ function renderGraph(initialCells: CellRecord[]): Harness {
function Probe() {
const { graph: currentGraph } = useGraphStore();
graph = currentGraph;
useOnElementsMeasured(PAPER_ID, ({ isInitial }) => {
events.push({ isInitial });
// An event is a change to a non-zero version; it is the initial one when
// the version before it was `0` (nothing measured) or the hook just mounted.
useOnCellsChange(selectMeasuredState, (version, previousMeasuredState) => {
if (version) events.push({ isInitial: !previousMeasuredState });
});
return null;
}
Expand Down Expand Up @@ -137,7 +143,7 @@ async function settleAndClear(harness: Harness) {
harness.events.length = 0;
}

describe('useOnElementsMeasured — one event per settled change', () => {
describe('selectMeasuredState — one event per settled change', () => {
it('delivers one event for the seed pass', async () => {
const harness = renderGraph([plain('a')]);

Expand Down Expand Up @@ -257,7 +263,7 @@ describe('useOnElementsMeasured — one event per settled change', () => {
});
});

describe('useOnElementsMeasured — a graph reset starts a new measurement history', () => {
describe('selectMeasuredState — a graph reset starts a new measurement history', () => {
// Resetting the graph replaces the diagram, so the next pass is that
// diagram's first one: a consumer that fits the paper on `isInitial` has a
// new set of contents to fit.
Expand Down Expand Up @@ -314,7 +320,7 @@ describe('useOnElementsMeasured — a graph reset starts a new measurement histo
// element is settled already, in which case nothing about readiness changed, or
// it is waiting to be measured, in which case the measurement is still owed and
// will overwrite the write anyway.
describe('useOnElementsMeasured — sizes written by the application', () => {
describe('selectMeasuredState — sizes written by the application', () => {
// #3514: a layout that resizes cells must not re-enter its own callback.
// Nothing was outstanding before the write and nothing is after it.
it('delivers no event when the application resizes an element nothing measures', async () => {
Expand Down Expand Up @@ -364,7 +370,7 @@ describe('useOnElementsMeasured — sizes written by the application', () => {
// An element can be zero-sized for good, rather than briefly on its way to a
// measurement. Nothing will ever give it a size, so treating it as outstanding
// holds every later batch open and the hook stops firing altogether.
describe('useOnElementsMeasured — an element that stays zero-sized', () => {
describe('selectMeasuredState — an element that stays zero-sized', () => {
it('delivers the seed pass with a zero-sized element in the graph', async () => {
const harness = renderGraph([plain('a'), anchor('anchor')]);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Scenarios beyond the #3520 specification in `use-on-elements-measured-events`:
* Scenarios beyond the #3520 specification in `measurement-events`:
* an element the paper does not render, a waiting element that is removed, and
* a content change that re-measures. Same harness and helpers as the spec,
* except that `flush()` also awaits the paper's render frame, in which a newly
Expand All @@ -9,8 +9,9 @@ import { render, waitFor, act } from '@testing-library/react';
import { GraphProvider } from '../../components/graph/graph-provider';
import { Paper } from '../../components/paper/paper';
import { HTMLHost } from '../../components/html-host';
import { useOnElementsMeasured } from '../use-on-elements-measured';
import { useOnCellsChange } from '../use-on-cells-change';
import { useGraphStore } from '../use-graph-store';
import { selectMeasuredState } from '../../selectors';
import { ELEMENT_MODEL_TYPE } from '../../mvc/element-model';
import type { CellRecord } from '../../types/cell.types';
import type { PaperProps } from '../../components/paper/paper.types';
Expand Down Expand Up @@ -87,8 +88,10 @@ function renderGraph(initialCells: CellRecord[], paperProps: Partial<PaperProps>
function Probe() {
const { graph: currentGraph } = useGraphStore();
graph = currentGraph;
useOnElementsMeasured(PAPER_ID, ({ isInitial }) => {
events.push({ isInitial });
// An event is a change to a non-zero version; it is the initial one when
// the version before it was `0` (nothing measured) or the hook just mounted.
useOnCellsChange(selectMeasuredState, (version, previousMeasuredState) => {
if (version) events.push({ isInitial: !previousMeasuredState });
});
return null;
}
Expand Down Expand Up @@ -116,7 +119,7 @@ async function settleAndClear(harness: Harness) {
// measurer, it becomes outstanding then.
const hideAnchor: PaperProps['cellVisibility'] = ({ model }) => model.id !== 'anchor';

describe('useOnElementsMeasured — an element the paper does not render', () => {
describe('selectMeasuredState — an element the paper does not render', () => {
it('delivers the seed pass with a culled zero-sized element in the graph', async () => {
const harness = renderGraph([plain('a'), anchor('anchor')], { cellVisibility: hideAnchor });

Expand All @@ -140,7 +143,7 @@ describe('useOnElementsMeasured — an element the paper does not render', () =>
});

// What else ends the wait: the waiting element leaves the graph.
describe('useOnElementsMeasured — a waiting element is removed', () => {
describe('selectMeasuredState — a waiting element is removed', () => {
it('delivers the batch once the waiting element is removed before it is measured', async () => {
const harness = renderGraph([plain('a')]);
await settleAndClear(harness);
Expand All @@ -160,11 +163,54 @@ describe('useOnElementsMeasured — a waiting element is removed', () => {
});
});

// Regression: an element that stopped measuring before it was measured stayed
// "waiting" forever and held back every later event.
describe('selectMeasuredState — a waiting element stops measuring', () => {
it('delivers the batch, and later changes, once nothing measures the element any more', async () => {
const harness = renderGraph([plain('a')]);
await settleAndClear(harness);

act(() => {
harness.graph.addCell(pending('b') as never);
});
await flush();
expect(harness.events).toHaveLength(0);

// Its content switches to a plain shape: the measuring node unmounts.
act(() => {
harness.graph.getCell('b').set('data', {});
});
await flush();
expect(harness.events).toHaveLength(1);

act(() => {
harness.graph.addCell(plain('c') as never);
});
await flush();
expect(harness.events).toHaveLength(2);
});
});

// A removal changes what a layout has to arrange, so it is a settled change too.
describe('selectMeasuredState — a settled element is removed', () => {
it('delivers one event for the removal', async () => {
const harness = renderGraph([plain('a'), plain('b')]);
await settleAndClear(harness);

act(() => {
harness.graph.getCell('b').remove();
});
await flush();

expect(harness.events).toEqual([{ isInitial: false }]);
});
});

// The case the hook exists for in a live diagram: `renderElement` renders
// something else (a longer label, an expanded card), the node grows, the
// ResizeObserver reports the new size and the layout runs again. jsdom has no
// layout, so a local ResizeObserver mock delivers the entry the browser would.
describe('useOnElementsMeasured — the content of an element changes', () => {
describe('selectMeasuredState — the content of an element changes', () => {
class TestResizeObserver {
static readonly instances: TestResizeObserver[] = [];
readonly observed = new Set<Element>();
Expand Down Expand Up @@ -194,15 +240,25 @@ describe('useOnElementsMeasured — the content of an element changes', () => {
globalThis.ResizeObserver = TestResizeObserver as unknown as typeof ResizeObserver;
});

/**
* The observer holding the node `<HTMLHost>` registered. The paper observes
* its own host with another `ResizeObserver`, so the instance is found by
* the node: the measured one lives inside the element's `foreignObject`.
*/
function findMeasuredNode() {
for (const observer of TestResizeObserver.instances) {
for (const node of observer.observed) {
if (node.closest('foreignObject')) return { observer, node };
}
}
throw new Error('no ResizeObserver has the measured node registered');
}

/** Mounts one measured element, lets it register, and measures it once. */
async function mountMeasured() {
const harness = renderGraph([pending('b')]);
await flush();
// StrictMode mounts the store twice; only the live store's observer has
// the node registered by `<HTMLHost>`.
const observer = TestResizeObserver.instances.find((instance) => instance.observed.size > 0);
if (!observer) throw new Error('no ResizeObserver has the measured node registered');
const [node] = observer.observed;
const { observer, node } = findMeasuredNode();

act(() => {
observer.report(node, 120, 40);
Expand Down Expand Up @@ -256,9 +312,7 @@ describe('useOnElementsMeasured — the content of an element changes', () => {
it('measures an element the application pre-sized to what it will measure', async () => {
const harness = renderGraph([pending('b')]);
await flush();
const observer = TestResizeObserver.instances.find((instance) => instance.observed.size > 0);
if (!observer) throw new Error('no ResizeObserver has the measured node registered');
const [node] = observer.observed;
const { observer, node } = findMeasuredNode();

act(() => {
(harness.graph.getCell('b') as dia.Element).resize(120, 40);
Expand Down
Loading
Loading