Skip to content

Remove broken round-polygon shapes from the picker and coerce stored values#1923

Open
kmcginnes wants to merge 5 commits into
mainfrom
fix-round-polygon-shapes
Open

Remove broken round-polygon shapes from the picker and coerce stored values#1923
kmcginnes wants to merge 5 commits into
mainfrom
fix-round-polygon-shapes

Conversation

@kmcginnes

@kmcginnes kmcginnes commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Description

Cytoscape's round-polygon renderer degenerates at the 24px node size used on the canvas for six shapes (round-triangle, round-pentagon, round-hexagon, round-heptagon, round-octagon, round-tag): the fill renders as a blob and the edge-intersection computation returns no valid point, so connected edges disappear. These shapes became selectable via #1886; no upstream cytoscape fix exists (3.34.0 is latest).

This PR removes the six shapes from the picker and coerces any previously stored value to its non-round counterpart at the read boundary, preserving the user's visual-differentiation intent:

Broken shape Coerced to
round-triangle triangle
round-pentagon pentagon
round-hexagon hexagon
round-heptagon heptagon
round-octagon octagon
round-tag tag
  • Picker — the six shapes are removed from NODE_SHAPE.
  • Storage edge — a ReadTransform on the user-vertex-styles and shared-vertex-styles atoms coerces stored broken shapes at load time (never writes back, so originals are recoverable if cytoscape fixes the bug).
  • Import — stores the value as-is; the ReadTransform coerces on next load, keeping the import path non-destructive.
  • SHAPE_STYLES stays inclusive — the six values remain in the enum and ShapeStyle type, so older styling files still parse cleanly.

How to read

  1. vertexStylesTransform.ts — the per-shape mapping, coerceBrokenShape(), and the ReadTransform
  2. storageAtoms.ts — wires the transform onto the two vertex-styles atoms
  3. nodeShape.ts — picker list trimmed
  4. vertexStylesTransform.test.ts — unit + backward-compat integration tests
  5. ADR — documents the coerce-vs-reject decision and reversal condition
  6. ReadTransform ADR update — notes this as the second consumer

Validation

  • pnpm check:types and pnpm check:format pass.
  • pnpm test passes — 2128 tests across 185 files.
  • Backward-compat integration tests prove the full localForage → atom pipeline coerces correctly and does not write back.

Related Issues

Check List

  • I confirm that my contribution is made under the terms of the Apache 2.0 license.
  • I have verified pnpm checks passes with no errors.
  • I have verified pnpm test passes with no failures.
  • I have covered new added functionality with unit tests if necessary.
  • I have updated documentation if necessary.

@kmcginnes kmcginnes force-pushed the fix-round-polygon-shapes branch from e52fd8c to 28b2054 Compare July 10, 2026 18:27
…values (#1922)

Cytoscape's round-polygon renderer degenerates at 24px for six shapes
(round-triangle, round-pentagon, round-hexagon, round-heptagon,
round-octagon, round-tag), rendering them as blobs and causing edges to
disappear. This removes them from the shape picker so they can't be
newly selected, and coerces any previously-stored values to
round-rectangle at both the storage-read boundary (ReadTransform) and
the styling-import boundary (Zod transform). The shapes remain in
SHAPE_STYLES so older files still parse without rejection.
Move coerceBrokenShape() and the broken-shape set into
vertexStylesTransform.ts so the import from storageAtoms is type-only
into graphStyles.ts, breaking the runtime cycle that caused a
ReferenceError during initialization.

Change the coercion target from "round-rectangle" to "roundrectangle"
(the value NODE_SHAPE uses for the picker) so the style dialog displays
"Round Rectangle" instead of a blank field for coerced entries.
…me coercion

Review feedback applied:

- Each broken round-polygon shape now maps to its sharp-cornered counterpart
  (round-hexagon → hexagon, etc.) instead of collapsing all six to a single
  flat replacement. This preserves the user's visual-differentiation intent.

- Remove the Zod .transform() from stylingParser.ts — import stores the value
  as-is and the ReadTransform coerces on next load, keeping the import path
  non-destructive and the original recoverable.

- Add debug logging to transformVertexStyles so coercions are observable.

- Convert the transform test loop to it.each for better failure diagnostics.

- Add ADR documenting the coerce-vs-reject decision and reversal condition.

- Update the ReadTransform ADR with transformVertexStyles as a second consumer.
@kmcginnes kmcginnes force-pushed the fix-round-polygon-shapes branch from 1d3c409 to 9fad9e0 Compare July 10, 2026 22:29
Proves the end-to-end pipeline: legacy data seeded in localForage with a
broken round-polygon shape → atomWithLocalForage with the ReadTransform
→ atom value has the shape coerced to its non-round counterpart. Also
verifies no write-back occurs (stored original is preserved).
The common case (no broken shapes stored) now avoids allocating a Map
that is immediately discarded, matching the pattern used by the sibling
transformGraphViewLayout.
@kmcginnes kmcginnes marked this pull request as ready for review July 10, 2026 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant