diff --git a/docs/adr/20260813-element-data-style-mappers.md b/docs/adr/20260813-element-data-style-mappers.md index 40a5def7a..4eea05f72 100644 --- a/docs/adr/20260813-element-data-style-mappers.md +++ b/docs/adr/20260813-element-data-style-mappers.md @@ -12,7 +12,7 @@ The graph canvas colours each vertex/edge by its type. The obvious Cytoscape idi Precompute each element's resolved style values onto its Cytoscape `ele.data()` as `ge_*` fields (`vertexStyleData` / `edgeStyleData` in `core/StateProvider/graphElementStyleData.ts`) at the element-construction seams (`renderedEntities.ts` for the explorer graph, `useSchemaGraphData.ts` for the schema view), and read them back through a single `node` rule and single `edge` rule using Cytoscape `data(…)` mappers (`CANVAS_STYLES` in `useGraphStyles.ts`). The stylesheet is now O(1) in the number of types. -Two structured/optional properties can't be a plain always-present mapper, so they keep a gated selector that applies only when the field exists: `node[__iconUrl]` (background image; the field is omitted when a type has no icon) and `edge[ge_lineDashPattern]` (omitted for solid lines). The dotted→dashed remap, the dash-pattern lookup, the border-opacity derivation, and the `isDark` label-text-colour pick are baked into the producer functions so the stylesheet stays pure `data(…)`. +**Every field is always set, so there are no gated selectors.** `cy.json({ elements })` _merges_ element data — `ele.data(obj)` adds and overwrites keys but never deletes ones missing from the new object. A sometimes-absent field therefore can never be cleared once applied, stranding a stale value on an already-drawn element: an icon that stopped resolving kept rendering the previous image. So `ge_iconUrl` carries `"none"` when a type has no icon, and `ge_lineDashPattern` carries cytoscape's default for solid lines, letting both live on the base `node` / `edge` rule. The dotted→dashed remap, the dash-pattern lookup, the border-opacity derivation, and the `isDark` label-text-colour pick are baked into the producer functions so the stylesheet stays pure `data(…)`. ## Consequences diff --git a/packages/graph-explorer/src/components/Graph/hooks/useManageStyles.ts b/packages/graph-explorer/src/components/Graph/hooks/useManageStyles.ts index d73c43094..28d73e343 100755 --- a/packages/graph-explorer/src/components/Graph/hooks/useManageStyles.ts +++ b/packages/graph-explorer/src/components/Graph/hooks/useManageStyles.ts @@ -125,13 +125,6 @@ export const getStyles = ({ addDefault("edge.connections-filter-out", toCyEdgeStyle(outOfFocusEdgeStyle)); addDefault("edge.out-of-focus", toCyEdgeStyle(outOfFocusEdgeStyle)); - rootStyles.push({ - selector: "node[__iconUrl]", - style: { - "background-image": "data(__iconUrl)", - }, - }); - return rootStyles; }; diff --git a/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.test.ts b/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.test.ts index 3d05d0049..8e32ff231 100644 --- a/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.test.ts @@ -47,9 +47,11 @@ describe("vertexStyleData", () => { ).toBe(1); }); - it("gates __iconUrl on backgroundImage presence", () => { - expect(vertexStyleData(vertex(), undefined).__iconUrl).toBeUndefined(); - expect(vertexStyleData(vertex(), "img").__iconUrl).toBe("img"); + // Always set, never omitted: cytoscape merges element data and never deletes a + // key, so an absent field could not clear a previously applied icon. + it("always sets ge_iconUrl, using none when there is no icon", () => { + expect(vertexStyleData(vertex(), undefined).ge_iconUrl).toBe("none"); + expect(vertexStyleData(vertex(), "img").ge_iconUrl).toBe("img"); }); }); @@ -77,10 +79,10 @@ describe("edgeStyleData", () => { ); }); - it("emits ge_lineDashPattern only for non-solid lines", () => { + it("always sets ge_lineDashPattern, using the default for solid lines", () => { expect( edgeStyleData(edge({ lineStyle: "solid" })).ge_lineDashPattern, - ).toBeUndefined(); + ).toEqual([6, 3]); expect( edgeStyleData(edge({ lineStyle: "dashed" })).ge_lineDashPattern, ).toEqual([5, 6]); @@ -123,7 +125,7 @@ describe("labelTextColorFor", () => { expect(labelTextColorFor("")).toBe(labelTextColorFor("")); }); - it("keys the memo per color rather than sharing one answer", () => { + it("answers per color rather than returning one answer for all", () => { expect(labelTextColorFor("#000000")).not.toBe(labelTextColorFor("#ffffff")); }); }); diff --git a/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.ts b/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.ts index a9e2fd440..1ed1c657d 100644 --- a/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.ts +++ b/packages/graph-explorer/src/core/StateProvider/graphElementStyleData.ts @@ -16,12 +16,32 @@ import { * dash-pattern remap so the style loop stays pure `data()`. */ -/** A `Map` so a type name colliding with `Object.prototype` cannot resolve to a function. */ +/** Cytoscape's own default, used for solid lines, which ignore the pattern. */ +const SOLID_PATTERN: readonly number[] = [6, 3]; + +/** + * A `Map` so a `lineStyle` colliding with `Object.prototype` cannot resolve to a + * function. + */ const LINE_PATTERN = new Map([ + ["solid", SOLID_PATTERN], ["dashed", [5, 6]], ["dotted", [1, 2]], ]); +/** Emitted when a vertex type has no resolved icon; cytoscape's "no image" value. */ +const NO_ICON = "none"; + +/** + * ALWAYS_SET: every field below is set on every element, never omitted. + * + * `cy.json({ elements })` *merges* element data — `ele.data(obj)` adds and + * overwrites keys but never deletes ones missing from the new object. A field + * that is sometimes absent can therefore never be cleared once it has been + * applied, stranding a stale value on an already-drawn element. That is why + * there are no gated `node[…]` / `edge[…]` selectors for these. + */ + /** Data-mapper fields set on every rendered vertex. Feeds the single `node` rule. */ export type VertexStyleData = { ge_color: string; @@ -31,16 +51,16 @@ export type VertexStyleData = { ge_borderOpacity: 0 | 1; ge_borderStyle: LineStyle; ge_shape: VertexStyle["shape"]; - /** Absent when the type has no resolved icon; the `node[__iconUrl]` selector gates on it. */ - __iconUrl?: string; + /** `"none"` when the type has no resolved icon. */ + ge_iconUrl: string; }; /** Data-mapper fields set on every rendered edge. Feeds the single `edge` rule. */ export type EdgeStyleData = { ge_lineColor: string; ge_lineStyle: LineStyle; - /** Absent for solid lines; the `edge[ge_lineDashPattern]` selector gates on it. */ - ge_lineDashPattern?: readonly number[]; + /** Cytoscape's default for solid lines, which ignore it. */ + ge_lineDashPattern: readonly number[]; ge_sourceArrowShape: EdgeStyle["sourceArrowStyle"]; ge_targetArrowShape: EdgeStyle["targetArrowStyle"]; ge_labelTextColor: "#FFFFFF" | "#000000"; @@ -72,7 +92,7 @@ export function vertexStyleData( style: VertexStyle, backgroundImage: string | undefined, ): VertexStyleData { - const data: VertexStyleData = { + return { ge_color: style.color, ge_backgroundOpacity: style.backgroundOpacity, ge_borderColor: style.borderColor, @@ -80,19 +100,15 @@ export function vertexStyleData( ge_borderOpacity: style.borderWidth > 0 ? 1 : 0, ge_borderStyle: style.borderStyle, ge_shape: style.shape, + ge_iconUrl: backgroundImage ?? NO_ICON, }; - if (backgroundImage !== undefined) { - data.__iconUrl = backgroundImage; - } - return data; } /** Precomputed cytoscape data-mapper fields for a rendered edge. */ export function edgeStyleData(style: EdgeStyle): EdgeStyleData { const lineStyle: LineStyle = style.lineStyle === "dotted" ? "dashed" : style.lineStyle; - const dashPattern = LINE_PATTERN.get(style.lineStyle); - const data: EdgeStyleData = { + return { ge_lineColor: style.lineColor, ge_lineStyle: lineStyle, ge_sourceArrowShape: style.sourceArrowStyle, @@ -104,9 +120,6 @@ export function edgeStyleData(style: EdgeStyle): EdgeStyleData { ge_labelBorderColor: style.labelBorderColor, ge_labelBorderStyle: style.labelBorderStyle, ge_lineThickness: style.lineThickness, + ge_lineDashPattern: LINE_PATTERN.get(style.lineStyle) ?? SOLID_PATTERN, }; - if (dashPattern !== undefined) { - data.ge_lineDashPattern = dashPattern; - } - return data; } diff --git a/packages/graph-explorer/src/core/StateProvider/renderedEntities.test.ts b/packages/graph-explorer/src/core/StateProvider/renderedEntities.test.ts index dd66fb031..9221f32cb 100644 --- a/packages/graph-explorer/src/core/StateProvider/renderedEntities.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/renderedEntities.test.ts @@ -207,7 +207,7 @@ describe("useRenderedVertices icon coverage", () => { ); await waitFor(() => { - expect(result.current.vertices[0].data.__iconUrl).toBe( + expect(result.current.vertices[0].data.ge_iconUrl).toBe( "https://example.test/icon.png", ); expect(result.current.vertices[0].data.ge_color).toBe("#abcdef"); diff --git a/packages/graph-explorer/src/core/StateProvider/styleDataResolvers.test.ts b/packages/graph-explorer/src/core/StateProvider/styleDataResolvers.test.ts index 574fa0b57..64b7476b2 100644 --- a/packages/graph-explorer/src/core/StateProvider/styleDataResolvers.test.ts +++ b/packages/graph-explorer/src/core/StateProvider/styleDataResolvers.test.ts @@ -71,9 +71,7 @@ describe("useVertexStyleDataResolver", () => { useVertexStyleDataResolver([]), ); - expect( - result.current(createVertexType("Person")).__iconUrl, - ).toBeUndefined(); + expect(result.current(createVertexType("Person")).ge_iconUrl).toBe("none"); }); }); diff --git a/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.contextCount.test.tsx b/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.contextCount.test.tsx index 75b4c4f49..f51b04f89 100644 --- a/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.contextCount.test.tsx +++ b/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.contextCount.test.tsx @@ -56,10 +56,6 @@ describe("useGraphStyles style-context count", () => { ); await waitFor(() => expect(result.current).toBeDefined()); - expect(Object.keys(result.current).sort()).toStrictEqual([ - "edge", - "edge[ge_lineDashPattern]", - "node", - ]); + expect(Object.keys(result.current).sort()).toStrictEqual(["edge", "node"]); }); }); diff --git a/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.test.tsx b/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.test.tsx index 242e69e38..be0838385 100644 --- a/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.test.tsx +++ b/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.test.tsx @@ -12,7 +12,7 @@ describe("useGraphStyles", () => { it("emits one node rule + one edge rule + one gated dash-pattern rule", () => { const { result } = renderHookWithState(() => useGraphStyles()); const selectors = Object.keys(result.current).sort(); - expect(selectors).toEqual(["edge", "edge[ge_lineDashPattern]", "node"]); + expect(selectors).toEqual(["edge", "node"]); }); it("uses data() mappers for every per-type property", () => { @@ -39,14 +39,17 @@ describe("useGraphStyles", () => { expect(edgeRule["width"]).toBe("data(ge_lineThickness)"); }); - it("gates line-dash-pattern on ge_lineDashPattern presence", () => { - // Solid edges omit the field and fall through to the default; only dashed/ - // dotted edges pick up the pattern via the gated selector. + // No gated selector: every element always sets the field, because cytoscape + // merges element data and could never clear an absent one. + it("maps line-dash-pattern on the base edge rule", () => { const { result } = renderHookWithState(() => useGraphStyles()); - const dashRule = result.current["edge[ge_lineDashPattern]"] as Record< - string, - unknown - >; - expect(dashRule["line-dash-pattern"]).toBe("data(ge_lineDashPattern)"); + const edgeRule = result.current["edge"] as Record; + expect(edgeRule["line-dash-pattern"]).toBe("data(ge_lineDashPattern)"); + }); + + it("maps background-image on the base node rule", () => { + const { result } = renderHookWithState(() => useGraphStyles()); + const nodeRule = result.current["node"] as Record; + expect(nodeRule["background-image"]).toBe("data(ge_iconUrl)"); }); }); diff --git a/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.ts b/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.ts index 9aa97d5bb..3d8d9bf6a 100644 --- a/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.ts +++ b/packages/graph-explorer/src/modules/GraphViewer/useGraphStyles.ts @@ -6,9 +6,12 @@ import type { GraphProps } from "@/components/Graph"; * Every per-type visual value is precomputed onto element `data()` by * `vertexStyleData` / `edgeStyleData` and read back through `data(...)` * mappers, so the stylesheet is O(1) in the number of types rather than one - * selector per type — see #2104. Two gated selectors handle absent-field - * cases (`__iconUrl` for typed icons, `ge_lineDashPattern` for non-solid - * edges). + * selector per type — see #2104. + * + * There are no gated `node[…]` / `edge[…]` selectors: cytoscape merges element + * data and never deletes a key, so a sometimes-absent field would strand a + * stale value on an already-drawn element. Every field is always set, so every + * mapper can live on the base rule. */ export default function useGraphStyles(): NonNullable { return CANVAS_STYLES; @@ -23,6 +26,7 @@ const CANVAS_STYLES: NonNullable = { "border-opacity": "data(ge_borderOpacity)", "border-style": "data(ge_borderStyle)", shape: "data(ge_shape)", + "background-image": "data(ge_iconUrl)", width: 24, height: 24, }, @@ -31,6 +35,7 @@ const CANVAS_STYLES: NonNullable = { color: "data(ge_labelTextColor)", "line-color": "data(ge_lineColor)", "line-style": "data(ge_lineStyle)", + "line-dash-pattern": "data(ge_lineDashPattern)", "source-arrow-shape": "data(ge_sourceArrowShape)", "source-arrow-color": "data(ge_lineColor)", "target-arrow-shape": "data(ge_targetArrowShape)", @@ -46,7 +51,4 @@ const CANVAS_STYLES: NonNullable = { "source-distance-from-node": "0", "target-distance-from-node": "0", }, - "edge[ge_lineDashPattern]": { - "line-dash-pattern": "data(ge_lineDashPattern)", - }, }; diff --git a/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphData.test.tsx b/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphData.test.tsx index b646276ba..7cc14844a 100644 --- a/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphData.test.tsx +++ b/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphData.test.tsx @@ -48,7 +48,7 @@ describe("useSchemaGraphData", () => { expect(node.data.ge_shape).toBe("hexagon"); expect(node.data.ge_borderWidth).toBe(2); expect(node.data.ge_borderOpacity).toBe(1); - expect(node.data.__iconUrl).toBe("img:Person"); + expect(node.data.ge_iconUrl).toBe("img:Person"); }); it("enriches edges with per-type ge_* style data (solid: no dash pattern)", async () => { @@ -84,10 +84,10 @@ describe("useSchemaGraphData", () => { expect(edge.data.ge_lineColor).toBe("#ff0000"); expect(edge.data.ge_lineStyle).toBe("solid"); expect(edge.data.ge_targetArrowShape).toBe("triangle"); - expect(edge.data.ge_lineDashPattern).toBeUndefined(); + expect(edge.data.ge_lineDashPattern).toEqual([6, 3]); }); - it("emits ge_lineDashPattern for dashed edges", async () => { + it("emits the dashed ge_lineDashPattern for dashed edges", async () => { const vertexConfig = { ...createRandomVertexTypeConfig(), type: createVertexType("Person"), diff --git a/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphStyles.test.tsx b/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphStyles.test.tsx index 4d88b2981..ff0e2aae5 100644 --- a/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphStyles.test.tsx +++ b/packages/graph-explorer/src/modules/SchemaGraph/useSchemaGraphStyles.test.tsx @@ -28,7 +28,6 @@ describe("useSchemaGraphStyles", () => { it("stays O(1) in selector count", () => { const { result } = renderHookWithState(() => useSchemaGraphStyles()); - // node + edge + gated edge[ge_lineDashPattern] - expect(Object.keys(result.current!).length).toBeLessThanOrEqual(6); + expect(Object.keys(result.current!).sort()).toStrictEqual(["edge", "node"]); }); });