From 83f5c475e9e806a6142e8c49e278cf55cd9997cf Mon Sep 17 00:00:00 2001 From: Ashish Jain Date: Tue, 25 Aug 2026 11:44:35 +0200 Subject: [PATCH 1/8] fix(build): externalize peerDependencies in core builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getBuildConfig built the core build's `external` list from `dependencies` only. mermaid is a *peer* dependency of the layout plugins, so it was not external, and a runtime (non-type) import of it resolved through the package `exports` to dist/mermaid.core.mjs — which esbuild then inlined wholesale. That stayed invisible while mermaid-layout-elk imported mermaid type-only (erased at compile time). Once src/render.ts began importing values (`import mermaid, { createCommonLayoutRenderer } from 'mermaid'`), the latent misconfiguration turned on: the published core entry went from 3 files / ~36 KB to 106 files / 6.6 MB, bundling every diagram renderer plus katex and a second, version-skewed mermaid. The duplicate matters beyond size. Module-level singletons are duplicated with it, so the plugin renders against its own inlined mermaid rather than the host's — meaning a mermaid rendering fix does not reach the ELK layout path until layout-elk is itself rebuilt and republished. That is how the `edgePaths` -> `edgePath` edge-container rename (#8124) stayed broken for ELK consumers after the mermaid-side fix. Push peerDependencies into `external` alongside dependencies. Scope is narrow: only the `if (core)` branch changes, so the self-contained `esm` entry still inlines mermaid for standalone and dev-server use, and mermaid itself declares no peerDependencies, so its own build is untouched. Verified: elk's core entry returns to 3 files / ~41 KB with mermaid as a bare external, and a consumer bundle drops from two copies of mermaid to one. Co-Authored-By: Claude Opus 5 --- .changeset/plain-crabs-listen.md | 7 +++++ .esbuild/util.spec.ts | 47 ++++++++++++++++++++++++++++++++ .esbuild/util.ts | 12 ++++++-- 3 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 .changeset/plain-crabs-listen.md create mode 100644 .esbuild/util.spec.ts diff --git a/.changeset/plain-crabs-listen.md b/.changeset/plain-crabs-listen.md new file mode 100644 index 00000000000..700d18b0df7 --- /dev/null +++ b/.changeset/plain-crabs-listen.md @@ -0,0 +1,7 @@ +--- +'@mermaid-js/layout-elk': patch +--- + +fix(build): externalize `peerDependencies` in core builds so the layout plugins no longer inline a second copy of mermaid + +`getBuildConfig` only externalized `dependencies`, so a runtime (non-type) import of the peer-depended mermaid resolved through `exports` to `dist/mermaid.core.mjs` and esbuild inlined the whole bundle. `@mermaid-js/layout-elk`'s core entry had grown to 106 files / 6.6 MB, carrying its own mermaid with separate module-level singletons — so mermaid rendering fixes did not reach the ELK layout path until the plugin itself was republished. The core entry is back to 3 files / ~41 KB and now defers to the host's mermaid. The self-contained `esm` entry is unchanged. diff --git a/.esbuild/util.spec.ts b/.esbuild/util.spec.ts new file mode 100644 index 00000000000..9213a996592 --- /dev/null +++ b/.esbuild/util.spec.ts @@ -0,0 +1,47 @@ +// @vitest-environment node +// util.ts resolves paths from `import.meta.url`, which is not a file:// URL +// under the default jsdom environment. +import { describe, expect, it } from 'vitest'; +import { packageOptions } from '../.build/common.js'; +import { defaultOptions, getBuildConfig } from './util.js'; + +const buildFor = (packageName: keyof typeof packageOptions, core: boolean) => + getBuildConfig({ + ...defaultOptions, + core, + format: 'esm', + options: packageOptions[packageName], + }); + +describe('getBuildConfig externals', () => { + it('externalizes peerDependencies in the core build', () => { + // The layout plugins peer-depend on mermaid. If it is not external, a + // runtime import of it resolves through `exports` to dist/mermaid.core.mjs + // and esbuild inlines the whole bundle — shipping a second mermaid, with + // its own module-level singletons, inside the plugin. The plugin then + // renders against its own stale copy instead of the host's, so mermaid + // fixes silently fail to reach it until the plugin is republished. + const external = buildFor('mermaid-layout-elk', true).external ?? []; + expect(external).toContain('mermaid'); + }); + + it('externalizes dependencies in the core build', () => { + const external = buildFor('mermaid-layout-elk', true).external ?? []; + expect(external).toContain('elkjs'); + expect(external).toContain('d3'); + }); + + it('bundles everything in the non-core build', () => { + // The esm entry is the self-contained one (standalone + dev server), so it + // must keep inlining mermaid rather than emitting a bare import. Only the + // node built-ins stay external there. + const external = buildFor('mermaid-layout-elk', false).external ?? []; + expect(external).not.toContain('mermaid'); + expect(external).not.toContain('elkjs'); + }); + + it('leaves mermaid itself unaffected — it has no peerDependencies', () => { + const external = buildFor('mermaid', true).external ?? []; + expect(external).not.toContain('mermaid'); + }); +}); diff --git a/.esbuild/util.ts b/.esbuild/util.ts index 6288bb73e33..e44affe5ea8 100644 --- a/.esbuild/util.ts +++ b/.esbuild/util.ts @@ -81,7 +81,7 @@ export const getBuildConfig = (options: MermaidBuildOptions): BuildOptions => { const external: string[] = ['require', 'fs', 'path']; const outFileName = getFileName(name, options); - const { dependencies, version } = JSON.parse( + const { dependencies, peerDependencies, version } = JSON.parse( readFileSync(resolve(__dirname, `../packages/${packageName}/package.json`), 'utf-8') ); const output: BuildOptions = buildOptions({ @@ -106,7 +106,15 @@ export const getBuildConfig = (options: MermaidBuildOptions): BuildOptions => { // Core build is used to generate file without bundled dependencies. // This is used by downstream projects to bundle dependencies themselves. // Ignore dependencies and any dependencies of dependencies - external.push(...Object.keys(dependencies)); + // + // peerDependencies must be external too. The consumer is the one that + // supplies them, so inlining one ships a second copy of that package — + // with its own module-level singletons — inside this bundle. For the + // layout plugins that peer dep is mermaid itself: a runtime (non-type) + // import of it resolves through `exports` to dist/mermaid.core.mjs and + // esbuild would inline the whole thing, so the plugin would run against + // its own stale mermaid rather than the host's. + external.push(...Object.keys(dependencies ?? {}), ...Object.keys(peerDependencies ?? {})); output.external = external; } From 43d9fbcea919c37b1baa3de06dbdc66592683d52 Mon Sep 17 00:00:00 2001 From: Ashish Jain Date: Thu, 27 Aug 2026 15:07:13 +0200 Subject: [PATCH 2/8] feat(themes): colour class boxes and flowchart subgraphs in the redux colour themes Until now only ER, sequence, git and requirement diagrams read the colour themes' `borderColorArray` / `bkgColorArray`. Class boxes and flowchart subgraph containers rendered in a single uniform colour under `redux-color` and `redux-dark-color`. Class diagrams: each class gets its own border and fill from the palette, cycling every 12, exactly as ER entities do. A class box is the structural twin of an ER entity -- a titled box with member rows naming one distinct participant -- so the same index-based palette applies. Namespaces and notes stay outside the cycle: a namespace is a container, and a note carries the theme's fixed note colour. Flowchart subgraphs: each container gets its own border and fill. The nodes inside are deliberately left uniform -- a flowchart node is a step in a flow rather than a distinct participant, and node colour is already how `classDef` / `style` convey meaning. `clusters.js` is shared by every diagram that has subgraphs, so it stamps `data-color-id` unconditionally and each diagram opts in by emitting the matching rules in its own stylesheet. State, block and class-namespace containers get an inert attribute and no colour. Subgraph colours key off the declaration index rather than a running counter: `getData()` walks subgraphs in reverse and skips ones hidden inside a collapsed ancestor, so a counter would hand out colours in reverse reading order and reshuffle them whenever a subgraph was collapsed. Explicit user styling still wins. `classDef` / `style` declarations are applied as inline `style` attributes and none of the new rules are `!important`, so `style MySubgraph fill:#00ff00` keeps painting the container green -- there is a test for that. --- .../redux-color-class-flowchart-palette.md | 16 +++++ .../mermaid/src/diagrams/class/classDb.ts | 4 ++ .../class/classDiagram-colorIndex.spec.ts | 58 +++++++++++++++++ packages/mermaid/src/diagrams/class/styles.js | 40 +++++++++++- .../diagrams/common/colorThemeGate.spec.ts | 65 +++++++++++++++++++ .../src/diagrams/flowchart/flowDb.spec.ts | 63 ++++++++++++++++++ .../mermaid/src/diagrams/flowchart/flowDb.ts | 4 ++ .../mermaid/src/diagrams/flowchart/styles.ts | 47 +++++++++++++- .../rendering-elements/clusters.js | 13 +++- .../rendering-elements/shapes/classBox.ts | 11 +++- 10 files changed, 315 insertions(+), 6 deletions(-) create mode 100644 .changeset/redux-color-class-flowchart-palette.md create mode 100644 packages/mermaid/src/diagrams/class/classDiagram-colorIndex.spec.ts create mode 100644 packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts diff --git a/.changeset/redux-color-class-flowchart-palette.md b/.changeset/redux-color-class-flowchart-palette.md new file mode 100644 index 00000000000..f8d36d2d0d7 --- /dev/null +++ b/.changeset/redux-color-class-flowchart-palette.md @@ -0,0 +1,16 @@ +--- +'mermaid': minor +--- + +feat(themes): class diagrams and flowchart subgraphs pick up the per-item colour palette under the `redux-color` and `redux-dark-color` themes. + +Until now only ER, sequence, git and requirement diagrams read the colour themes' `borderColorArray` / `bkgColorArray`. Class boxes and flowchart subgraph containers rendered in a single uniform colour under those themes. + +- **Class diagrams** — each class gets its own border and fill from the palette, cycling every 12, exactly as ER entities do. A class box is the structural twin of an ER entity (a titled box with member rows naming one distinct participant), so the same index-based palette applies. Namespaces and notes stay outside the cycle: a namespace is a container, and a note carries the theme's fixed note colour. +- **Flowchart subgraphs** — each subgraph container gets its own border and fill. Nodes _inside_ the subgraph are deliberately left uniform: a flowchart node is a step in a flow rather than a distinct participant, and node colour is already how `classDef` / `style` convey meaning. + +Subgraph colours follow declaration order and are stable across collapse: a subgraph carrying `@{ view: collapsed }` keeps its slot, so collapsing one does not reshuffle the others. + +Explicit user styling continues to win over the palette. `classDef` and `style` declarations are applied as inline `style` attributes and none of the new rules are `!important`, so `style MySubgraph fill:#00ff00` still paints the container green. + +Other themes are unaffected — the new rules are emitted only for `redux-color` and `redux-dark-color`. diff --git a/packages/mermaid/src/diagrams/class/classDb.ts b/packages/mermaid/src/diagrams/class/classDb.ts index 21f42946ced..c39a0c76df6 100644 --- a/packages/mermaid/src/diagrams/class/classDb.ts +++ b/packages/mermaid/src/diagrams/class/classDb.ts @@ -769,6 +769,9 @@ export class ClassDB implements DiagramDB { nodes.push(node); } + // Only classes consume a colour slot -- namespaces are containers and notes have + // their own fixed note colour, so neither should shift the cycle. + let classColorIndex = 0; for (const classNode of this.classes.values()) { const parentId = hierarchical ? classNode.parent @@ -779,6 +782,7 @@ export class ClassDB implements DiagramDB { isGroup: false, parentId, look: config.look, + colorIndex: classColorIndex++, }; nodes.push(node); } diff --git a/packages/mermaid/src/diagrams/class/classDiagram-colorIndex.spec.ts b/packages/mermaid/src/diagrams/class/classDiagram-colorIndex.spec.ts new file mode 100644 index 00000000000..83cef0b456f --- /dev/null +++ b/packages/mermaid/src/diagrams/class/classDiagram-colorIndex.spec.ts @@ -0,0 +1,58 @@ +/** + * `colorIndex` is what drives the per-class palette under the `redux-color` / + * `redux-dark-color` themes: `classDb` assigns the slot, `classBox` stamps it as + * `data-color-id`, and `class/styles.js` maps it to a border and fill. + * + * The failure mode is silent. If the slots stop being assigned, or start being shared, + * every box falls back to `color-0` and the diagram renders in one colour — which looks + * like a theme problem, not a db problem. So pin the assignment here rather than relying + * on a screenshot to notice. + */ +import { describe, expect, it, beforeEach } from 'vitest'; +import { ClassDB } from './classDb.js'; + +describe('class diagram colour slots', () => { + let classDb: ClassDB; + beforeEach(() => { + classDb = new ClassDB(); + }); + + const colorIndexById = () => + new Map(classDb.getData().nodes.map((node) => [node.id, node.colorIndex])); + + it('gives each class its own slot in declaration order', () => { + classDb.addClass('Order'); + classDb.addClass('Customer'); + classDb.addClass('Payment'); + + const slots = colorIndexById(); + expect(slots.get('Order')).toBe(0); + expect(slots.get('Customer')).toBe(1); + expect(slots.get('Payment')).toBe(2); + }); + + it('does not spend a slot on a namespace container', () => { + classDb.addClassesToNamespace('shop', [], []); + classDb.addClass('Order'); + classDb.addClass('Customer'); + + const slots = colorIndexById(); + // The namespace is a container, not a participant -- it must not shift the cycle. + expect(slots.get('shop')).toBeUndefined(); + expect(slots.get('Order')).toBe(0); + expect(slots.get('Customer')).toBe(1); + }); + + it('does not spend a slot on a note', () => { + classDb.addClass('Order'); + classDb.addNote('a note', 'Order'); + classDb.addClass('Customer'); + + const slots = colorIndexById(); + const noteEntry = [...slots.entries()].find(([id]) => id.startsWith('note')); + // Notes carry the theme's fixed note colour, so they stay outside the cycle. + expect(noteEntry?.[1]).toBeUndefined(); + expect(slots.get('Order')).toBe(0); + expect(slots.get('Customer')).toBe(1); + }); +}); diff --git a/packages/mermaid/src/diagrams/class/styles.js b/packages/mermaid/src/diagrams/class/styles.js index ccab6191923..4804330f56a 100644 --- a/packages/mermaid/src/diagrams/class/styles.js +++ b/packages/mermaid/src/diagrams/class/styles.js @@ -1,7 +1,45 @@ import { getIconStyles } from '../globalStyles.js'; +const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); + +/** + * Cycling per-class colour, mirroring `er/styles.ts`. A class box is the structural twin + * of an ER entity -- a titled box with member rows, naming one distinct participant -- so + * the same index-based palette applies. + * + * Targets `.outer-path` and `.divider` rather than a bare `.node path`, so member icons + * and other inner paths are left alone. Nothing here is `!important`: `classBox.ts` puts + * user `classDef` / `style` declarations in an inline `style` attribute, which must keep + * winning over the theme palette. + */ +const genColor = (options) => { + const { theme, look, bkgColorArray, borderColorArray } = options; + if (!COLOR_THEMES.has(theme) || !borderColorArray?.length) { + return ''; + } + const hasBkgColors = bkgColorArray?.length > 0; + let sections = ''; + + for (let i = 0; i < options.THEME_COLOR_LIMIT; i++) { + const borderColor = borderColorArray[i % borderColorArray.length]; + sections += ` + + [data-look="${look}"][data-color-id="color-${i}"].node .outer-path path { + stroke: ${borderColor}; + ${hasBkgColors ? `fill: ${bkgColorArray[i % bkgColorArray.length]};` : ''} + } + + [data-look="${look}"][data-color-id="color-${i}"].node .divider path { + stroke: ${borderColor}; + } + `; + } + return sections; +}; + const getStyles = (options) => - `g.classGroup text { + `${genColor(options)} + g.classGroup text { fill: ${options.nodeBorder || options.classText}; stroke: none; font-family: ${options.fontFamily}; diff --git a/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts b/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts new file mode 100644 index 00000000000..7df85e53624 --- /dev/null +++ b/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts @@ -0,0 +1,65 @@ +/** + * The per-item palette is opt-in per diagram: `clusters.js` and the shape files stamp a + * `data-color-id` attribute unconditionally, and a diagram only renders colour if its + * stylesheet emits the matching rules. That gate is a `COLOR_THEMES` set duplicated in + * each stylesheet, so it is easy to widen one by accident. + * + * These assertions are about the gate, not the colours: a stylesheet must stay silent for + * every theme outside the colour pair, and must emit one rule per palette slot inside it. + */ +import { describe, expect, it } from 'vitest'; +import themes from '../../themes/index.js'; +import classStyles from '../class/styles.js'; +import flowchartStyles from '../flowchart/styles.js'; + +const STYLESHEETS = { + class: classStyles, + flowchart: flowchartStyles, +} as const; + +const COLOUR_THEMES = ['redux-color', 'redux-dark-color']; +const PLAIN_THEMES = [ + 'default', + 'base', + 'dark', + 'forest', + 'neutral', + 'neo', + 'neo-dark', + 'redux', + 'redux-dark', +]; + +const render = (name: keyof typeof STYLESHEETS, themeName: string) => { + const themeVariables = themes[themeName as keyof typeof themes].getThemeVariables({}); + return STYLESHEETS[name]({ + ...(themeVariables as unknown as Record), + theme: themeName, + look: 'classic', + } as never); +}; + +describe.each(Object.keys(STYLESHEETS) as (keyof typeof STYLESHEETS)[])('%s stylesheet', (name) => { + it.each(PLAIN_THEMES)('emits no per-item colour rules for %s', (themeName) => { + expect(render(name, themeName)).not.toContain('data-color-id'); + }); + + it.each(COLOUR_THEMES)('emits one rule per palette slot for %s', (themeName) => { + const css = render(name, themeName); + const slots = new Set([...css.matchAll(/data-color-id="(color-\d+)"/g)].map((m) => m[1])); + expect(slots.size).toBe(12); + }); + + it.each(COLOUR_THEMES)('never marks palette rules !important for %s', (themeName) => { + // User `classDef` / `style` declarations land in an inline `style` attribute. If the + // palette rules were !important they would silently outrank explicit user intent. + // Match only the declaration body of each palette rule -- the rest of the stylesheet + // uses !important legitimately. + const css = render(name, themeName); + const bodies = [...css.matchAll(/\[data-color-id="color-\d+"][^{]*{([^}]*)}/g)].map( + (m) => m[1] + ); + expect(bodies.length).toBeGreaterThan(0); + expect(bodies.filter((body) => body.includes('!important'))).toEqual([]); + }); +}); diff --git a/packages/mermaid/src/diagrams/flowchart/flowDb.spec.ts b/packages/mermaid/src/diagrams/flowchart/flowDb.spec.ts index 7833a7b9480..ffca5818311 100644 --- a/packages/mermaid/src/diagrams/flowchart/flowDb.spec.ts +++ b/packages/mermaid/src/diagrams/flowchart/flowDb.spec.ts @@ -296,3 +296,66 @@ describe('flow db direction', () => { expect(flowDb.getDirection()).toBe('TB'); }); }); + +/** + * `colorIndex` drives the per-subgraph palette under the `redux-color` / + * `redux-dark-color` themes: `clusters.js` stamps it as `data-color-id` and + * `flowchart/styles.ts` maps it to a container border and fill. + * + * It is deliberately the declaration index rather than a running counter, because + * `getData()` walks subgraphs in reverse and skips ones hidden inside a collapsed + * ancestor. A counter would hand out colours in reverse reading order and reshuffle them + * whenever a subgraph is collapsed. + */ +describe('flow db subgraph colour slots', () => { + let flowDb: FlowDB; + beforeEach(() => { + flowDb = new FlowDB(); + }); + + const addVertex = (id: string) => + flowDb.addVertex(id, { text: id, type: 'text' }, undefined, [], [], '', {}, undefined); + + const attachMeta = (id: string, meta: string) => + flowDb.addVertex(id, undefined as unknown as FlowText, undefined, [], [], '', {}, meta); + + it('numbers subgraphs in declaration order, not the reverse order getData walks', () => { + for (const id of ['A', 'B', 'C']) { + addVertex(id); + } + flowDb.addSubGraph({ text: 'first' }, ['A'], { text: 'First', type: 'text' }); + flowDb.addSubGraph({ text: 'second' }, ['B'], { text: 'Second', type: 'text' }); + flowDb.addSubGraph({ text: 'third' }, ['C'], { text: 'Third', type: 'text' }); + + const { nodes } = flowDb.getData(); + const slot = (id: string) => nodes.find((n) => n.id === id)?.colorIndex; + expect([slot('first'), slot('second'), slot('third')]).toEqual([0, 1, 2]); + }); + + it('keeps a collapsed subgraph on its own slot so the cycle does not shift', () => { + for (const id of ['A', 'B', 'C']) { + addVertex(id); + } + flowDb.addSubGraph({ text: 'first' }, ['A'], { text: 'First', type: 'text' }); + flowDb.addSubGraph({ text: 'second' }, ['B'], { text: 'Second', type: 'text' }); + flowDb.addSubGraph({ text: 'third' }, ['C'], { text: 'Third', type: 'text' }); + attachMeta('second', ' view: collapsed '); + + const { nodes } = flowDb.getData(); + const slot = (id: string) => nodes.find((n) => n.id === id)?.colorIndex; + // `second` is drawn as a compact node rather than a container, but `third` keeps + // slot 2 either way. + expect(slot('first')).toBe(0); + expect(slot('second')).toBe(1); + expect(slot('third')).toBe(2); + }); + + it('leaves plain vertices without a slot', () => { + addVertex('A'); + flowDb.addSubGraph({ text: 'only' }, ['A'], { text: 'Only', type: 'text' }); + + const { nodes } = flowDb.getData(); + expect(nodes.find((n) => n.id === 'A')?.colorIndex).toBeUndefined(); + expect(nodes.find((n) => n.id === 'only')?.colorIndex).toBe(0); + }); +}); diff --git a/packages/mermaid/src/diagrams/flowchart/flowDb.ts b/packages/mermaid/src/diagrams/flowchart/flowDb.ts index 520580999ca..49424aab555 100644 --- a/packages/mermaid/src/diagrams/flowchart/flowDb.ts +++ b/packages/mermaid/src/diagrams/flowchart/flowDb.ts @@ -1193,6 +1193,9 @@ You have to call mermaid.initialize.` dir: subGraph.dir, isGroup: false, look: config.look, + // A collapsed subgraph still consumes its slot so the colour cycle does not + // shift when one is collapsed. `collapsedGroup` does not paint it yet. + colorIndex: i, }); } else { nodes.push({ @@ -1208,6 +1211,7 @@ You have to call mermaid.initialize.` dir: subGraph.dir, isGroup: true, look: config.look, + colorIndex: i, // Forwarded so layout engines can read per-container settings such as // `@{ algorithm: elk.box }`. `view` is consumed above; everything else // is opaque here and simply passed through. The cast is the diff --git a/packages/mermaid/src/diagrams/flowchart/styles.ts b/packages/mermaid/src/diagrams/flowchart/styles.ts index d54a79a6738..2e69bdefbab 100644 --- a/packages/mermaid/src/diagrams/flowchart/styles.ts +++ b/packages/mermaid/src/diagrams/flowchart/styles.ts @@ -18,8 +18,52 @@ export interface FlowChartStyleOptions { textColor: string; titleColor: string; strokeWidth: string; + theme?: string; + look?: string; + THEME_COLOR_LIMIT?: number; + borderColorArray?: string[]; + bkgColorArray?: string[]; } +const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); + +/** + * Cycling per-subgraph colour. Only the containers are painted -- the nodes inside keep + * the uniform look, because a flowchart node is a step in a flow rather than a distinct + * participant, and node colour is already how `classDef` / `style` carry meaning. + * + * Emits both the `rect` (classic/neo) and `path` (handDrawn) forms since the container is + * a plain rect in one look and a roughjs path pair in the other. Not `!important`: + * `clusters.js` puts user styles in an inline `style` attribute, which has to keep + * winning over the theme palette. + */ +const genColor = (options: FlowChartStyleOptions) => { + const { theme, look, bkgColorArray, borderColorArray } = options; + if (!theme || !COLOR_THEMES.has(theme) || !borderColorArray?.length) { + return ''; + } + const hasBkgColors = (bkgColorArray?.length ?? 0) > 0; + let sections = ''; + + for (let i = 0; i < (options.THEME_COLOR_LIMIT ?? 12); i++) { + const borderColor = borderColorArray[i % borderColorArray.length]; + const fill = hasBkgColors ? `fill: ${bkgColorArray![i % bkgColorArray!.length]};` : ''; + sections += ` + + [data-look="${look}"][data-color-id="color-${i}"].cluster rect { + stroke: ${borderColor}; + ${fill} + } + + [data-look="${look}"][data-color-id="color-${i}"].cluster path { + stroke: ${borderColor}; + ${fill} + } + `; + } + return sections; +}; + const fade = (color: string, opacity: number) => { // @ts-ignore TODO: incorrect types from khroma const channel = khroma.channel; @@ -33,7 +77,8 @@ const fade = (color: string, opacity: number) => { }; const getStyles = (options: FlowChartStyleOptions) => - `.label { + `${genColor(options)} + .label { font-family: ${options.fontFamily}; color: ${options.nodeTextColor || options.textColor}; } diff --git a/packages/mermaid/src/rendering-util/rendering-elements/clusters.js b/packages/mermaid/src/rendering-util/rendering-elements/clusters.js index d841f782011..d5a9e658279 100644 --- a/packages/mermaid/src/rendering-util/rendering-elements/clusters.js +++ b/packages/mermaid/src/rendering-util/rendering-elements/clusters.js @@ -11,11 +11,13 @@ import { createRoundedRectPathD } from './shapes/roundedRectPath.ts'; import { compileStyles, styles2String, userNodeOverrides } from './shapes/handDrawnShapeStyles.js'; import { swimlane } from './clusters/swimlane.js'; +const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); + const rect = async (parent, node) => { log.info('Creating subgraph rect for ', node.id, node); const siteConfig = getConfig(); - const { themeVariables, handDrawnSeed } = siteConfig; - const { clusterBkg, clusterBorder } = themeVariables; + const { theme, themeVariables, handDrawnSeed } = siteConfig; + const { clusterBkg, clusterBorder, borderColorArray } = themeVariables; const { labelStyles, nodeStyles, borderStyles, backgroundStyles } = styles2String(node); @@ -26,6 +28,13 @@ const rect = async (parent, node) => { .attr('id', node.domId) .attr('data-look', node.look); + // Per-container colour slot. Only diagrams whose stylesheet defines the matching + // `[data-color-id]` rules paint it; for the rest this is an inert attribute. + if (theme != null && COLOR_THEMES.has(theme) && borderColorArray?.length) { + const colorIndex = node.colorIndex ?? 0; + shapeSvg.attr('data-color-id', `color-${colorIndex % borderColorArray.length}`); + } + const useHtmlLabels = getEffectiveHtmlLabels(siteConfig); // Create the label and insert it after the rect diff --git a/packages/mermaid/src/rendering-util/rendering-elements/shapes/classBox.ts b/packages/mermaid/src/rendering-util/rendering-elements/shapes/classBox.ts index dd20852c23a..f65a1974178 100644 --- a/packages/mermaid/src/rendering-util/rendering-elements/shapes/classBox.ts +++ b/packages/mermaid/src/rendering-util/rendering-elements/shapes/classBox.ts @@ -10,10 +10,12 @@ import { textHelper } from '../../../diagrams/class/shapeUtil.js'; import { evaluate } from '../../../diagrams/common/common.js'; import type { D3Selection } from '../../../types.js'; +const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); + export async function classBox(parent: D3Selection, node: Node) { const config = getConfig(); - const { themeVariables } = config; - const { useGradient } = themeVariables; + const { theme, themeVariables } = config; + const { useGradient, borderColorArray } = themeVariables; const PADDING = config.class!.padding ?? 12; const GAP = PADDING; const useHtmlLabels = node.useHtmlLabels ?? evaluate(config.htmlLabels) ?? true; @@ -25,6 +27,11 @@ export async function classBox(parent: D3Selection const { shapeSvg, bbox } = await textHelper(parent, node, config, useHtmlLabels, GAP); + if (theme != null && COLOR_THEMES.has(theme) && borderColorArray?.length) { + const colorIndex = node.colorIndex ?? 0; + shapeSvg.attr('data-color-id', `color-${colorIndex % borderColorArray.length}`); + } + const { labelStyles, nodeStyles } = styles2String(node); node.labelStyle = labelStyles; From 2f1f69eda7a07126d6a4e49ba6e219d0aae48b15 Mon Sep 17 00:00:00 2001 From: Ashish Jain Date: Fri, 28 Aug 2026 09:33:38 +0200 Subject: [PATCH 3/8] fix(themes): address review on class and flowchart palette Extract the shared gate. `COLOR_THEMES` existed in five copies and the stamping block was duplicated verbatim between `clusters.js` and `classBox.ts`; two idioms for the same gate had already diverged (`er/styles.ts` keys off the theme name, `requirement/styles.js` off the array being non-empty). New `diagrams/common/colorThemeGate.ts` owns `COLOR_THEMES`, `isColorTheme`, `hasPalette`, `colorSlotCount`, `safeLook` and `stampColorSlot`; the four call sites import it. The spec was already sitting at the path that implied this module. Harden `look` before it reaches a CSS selector. `look` is a top-level config key, so it is settable from diagram text, and `config.sanitize` only drops values containing `<`, `>` or `url(data:` -- braces and quotes survive, which is enough to close the attribute selector early and escape the `#svgId` scoping stylis applies. Confirmed by probe: the value lands verbatim in the emitted rule. `safeLook` rejects anything that is not a bare word. NOTE: this pattern is pre-existing in `er/styles.ts` and `requirement/styles.js` on develop, so it affects released versions; those are deliberately left for a coordinated fix and can now import `safeLook` rather than adding a third copy. Close the collapsedGroup seam. A collapsed subgraph rendered uncoloured beside tinted siblings. It renders as `.node .collapsed-group` rather than `.cluster`, so the flowchart stylesheet gained that selector; its own colours are presentation attributes, which the rules outrank while still losing to user inline style. Verified by render: First/Second(collapsed)/Third take slots 0/1/2, so collapsing does not reshuffle siblings. Cover the join. The unit tests checked each end -- the db hands out slots, the stylesheet emits rules -- but not that the stamped attribute meets the emitted selector on a real element. Added the two colour themes to `classDiagram-neo.spec.js` (42 tests) and a new `flowchart-redux-color-subgraphs.spec.ts` (16) covering nesting, collapse and user-styled precedence. Also: derive `PLAIN_THEMES` from the theme registry so a new theme is covered automatically; use `colorSlotCount` in both stylesheets so they agree when `THEME_COLOR_LIMIT` is absent; `hasPalette` uses `Array.isArray` so a string cannot pass as a palette; reconcile the `clusters.js` comment with the code, which is conditional on the theme rather than unconditional. Changeset shortened. --- .../redux-color-class-flowchart-palette.md | 13 +- e2e/rendering/class/classDiagram-neo.spec.js | 6 + .../flowchart-redux-color-subgraphs.spec.ts | 115 ++++++++++++++++++ packages/mermaid/src/diagrams/class/styles.js | 12 +- .../diagrams/common/colorThemeGate.spec.ts | 74 ++++++++--- .../src/diagrams/common/colorThemeGate.ts | 78 ++++++++++++ .../mermaid/src/diagrams/flowchart/styles.ts | 39 ++++-- .../rendering-elements/clusters.js | 13 +- .../rendering-elements/shapes/classBox.ts | 8 +- .../shapes/collapsedGroup.ts | 7 ++ 10 files changed, 303 insertions(+), 62 deletions(-) create mode 100644 e2e/rendering/flowchart/flowchart-redux-color-subgraphs.spec.ts create mode 100644 packages/mermaid/src/diagrams/common/colorThemeGate.ts diff --git a/.changeset/redux-color-class-flowchart-palette.md b/.changeset/redux-color-class-flowchart-palette.md index f8d36d2d0d7..bc61fe2c1d2 100644 --- a/.changeset/redux-color-class-flowchart-palette.md +++ b/.changeset/redux-color-class-flowchart-palette.md @@ -2,15 +2,8 @@ 'mermaid': minor --- -feat(themes): class diagrams and flowchart subgraphs pick up the per-item colour palette under the `redux-color` and `redux-dark-color` themes. +feat(themes): class boxes and flowchart subgraph containers now pick up the per-item colour palette under the `redux-color` and `redux-dark-color` themes. -Until now only ER, sequence, git and requirement diagrams read the colour themes' `borderColorArray` / `bkgColorArray`. Class boxes and flowchart subgraph containers rendered in a single uniform colour under those themes. +Previously only ER, sequence, git and requirement diagrams read `borderColorArray` / `bkgColorArray`. Each class now gets its own border and fill, cycling every 12 as ER entities do; namespaces and notes stay outside the cycle. Each flowchart subgraph container gets its own colour — including collapsed ones, which keep the slot they would have had expanded — while nodes inside stay uniform, since node colour is already how `classDef` / `style` carry meaning. -- **Class diagrams** — each class gets its own border and fill from the palette, cycling every 12, exactly as ER entities do. A class box is the structural twin of an ER entity (a titled box with member rows naming one distinct participant), so the same index-based palette applies. Namespaces and notes stay outside the cycle: a namespace is a container, and a note carries the theme's fixed note colour. -- **Flowchart subgraphs** — each subgraph container gets its own border and fill. Nodes _inside_ the subgraph are deliberately left uniform: a flowchart node is a step in a flow rather than a distinct participant, and node colour is already how `classDef` / `style` convey meaning. - -Subgraph colours follow declaration order and are stable across collapse: a subgraph carrying `@{ view: collapsed }` keeps its slot, so collapsing one does not reshuffle the others. - -Explicit user styling continues to win over the palette. `classDef` and `style` declarations are applied as inline `style` attributes and none of the new rules are `!important`, so `style MySubgraph fill:#00ff00` still paints the container green. - -Other themes are unaffected — the new rules are emitted only for `redux-color` and `redux-dark-color`. +Explicit user styling still wins: `classDef` and `style` become inline `style` attributes and none of the new rules are `!important`. diff --git a/e2e/rendering/class/classDiagram-neo.spec.js b/e2e/rendering/class/classDiagram-neo.spec.js index c0043ff4267..c2b26c41460 100644 --- a/e2e/rendering/class/classDiagram-neo.spec.js +++ b/e2e/rendering/class/classDiagram-neo.spec.js @@ -6,6 +6,12 @@ const themes = [ { theme: 'neo-dark', label: 'neo-dark' }, { theme: 'redux', label: 'redux' }, { theme: 'redux-dark', label: 'redux-dark' }, + // The colour themes give each class box its own border and fill. Without these two + // entries nothing in the suite renders a class diagram under them, so the palette + // wiring — the slot stamped on the element meeting the selector emitted by the + // stylesheet — had no visual coverage at all. + { theme: 'redux-color', label: 'redux-color' }, + { theme: 'redux-dark-color', label: 'redux-dark-color' }, ]; const diagrams = { diff --git a/e2e/rendering/flowchart/flowchart-redux-color-subgraphs.spec.ts b/e2e/rendering/flowchart/flowchart-redux-color-subgraphs.spec.ts new file mode 100644 index 00000000000..8b3f4bd646b --- /dev/null +++ b/e2e/rendering/flowchart/flowchart-redux-color-subgraphs.spec.ts @@ -0,0 +1,115 @@ +import { test } from '@playwright/test'; +import { imgSnapshotTest } from '../../helpers/util.ts'; + +/** + * Flowchart subgraph containers take a per-container colour under the redux colour + * themes. Nothing in the suite rendered a flowchart under those themes, so the wiring + * had no visual coverage: the unit tests check that `flowDb` hands out slots and that the + * stylesheet emits rules, but only a render proves the stamped `data-color-id` actually + * meets the emitted selector on the element. + */ +const reduxThemes = ['redux', 'redux-color', 'redux-dark', 'redux-dark-color'] as const; + +/** + * Five subgraphs, one more than the four in the demo fixtures, so the ordering is + * unambiguous and a reversed cycle would be obvious. Nodes inside stay uniform by design. + */ +const subgraphs = ` + flowchart TB + subgraph Ingest + A[Fetch] --> B[Validate] + end + subgraph Transform + C[Normalise] --> D[Enrich] + end + subgraph Store + E[(Warehouse)] + end + subgraph Serve + F[API] --> G[Cache] + end + subgraph Observe + H[Metrics] + end + B --> C + D --> E + E --> F + F --> H +`; + +/** Nested containers, to show the palette applying at more than one depth. */ +const nested = ` + flowchart LR + subgraph Outer + subgraph InnerOne + A[one] --> B[two] + end + subgraph InnerTwo + C[three] + end + end + subgraph Sibling + D[four] + end + B --> C + C --> D +`; + +/** + * A collapsed subgraph renders as a compact node rather than a container. It is still a + * container, so it takes a palette slot too — and it keeps the slot it would have had + * expanded, so collapsing one does not reshuffle its siblings' colours. + */ +const collapsed = ` + flowchart TB + subgraph first[First] + A[a] --> B[b] + end + subgraph second[Second] + C[c] + end + second@{ view: collapsed } + subgraph third[Third] + D[d] + end + B --> C + C --> D +`; + +/** + * Explicit user styling has to keep winning over the palette: `classDef` / `style` + * declarations become inline `style` attributes and none of the palette rules are + * `!important`. `Two` should stay green here while `One` takes its slot colour. + */ +const userStyled = ` + flowchart LR + subgraph One + X[node] --> Y[node] + end + subgraph Two + Z[node] + end + classDef mine fill:#ff0000,stroke:#000000 + class X mine + style Two fill:#00ff00,stroke:#0000ff + Y --> Z +`; + +const diagrams = { + subgraphs, + nested, + collapsed, + 'user-styled': userStyled, +} as const; + +test.describe('Flowchart - Redux colour theme subgraphs', () => { + for (const theme of reduxThemes) { + test.describe(`Theme: ${theme}`, () => { + for (const [name, diagram] of Object.entries(diagrams)) { + test(`should render ${name} subgraph containers`, async ({ page }, testInfo) => { + await imgSnapshotTest(page, testInfo, diagram, { theme }); + }); + } + }); + } +}); diff --git a/packages/mermaid/src/diagrams/class/styles.js b/packages/mermaid/src/diagrams/class/styles.js index 4804330f56a..7d59807b3e1 100644 --- a/packages/mermaid/src/diagrams/class/styles.js +++ b/packages/mermaid/src/diagrams/class/styles.js @@ -1,6 +1,5 @@ import { getIconStyles } from '../globalStyles.js'; - -const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); +import { colorSlotCount, hasPalette, isColorTheme, safeLook } from '../common/colorThemeGate.js'; /** * Cycling per-class colour, mirroring `er/styles.ts`. A class box is the structural twin @@ -13,14 +12,15 @@ const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); * winning over the theme palette. */ const genColor = (options) => { - const { theme, look, bkgColorArray, borderColorArray } = options; - if (!COLOR_THEMES.has(theme) || !borderColorArray?.length) { + const { theme, bkgColorArray, borderColorArray } = options; + if (!isColorTheme(theme, borderColorArray)) { return ''; } - const hasBkgColors = bkgColorArray?.length > 0; + const look = safeLook(options.look); + const hasBkgColors = hasPalette(bkgColorArray); let sections = ''; - for (let i = 0; i < options.THEME_COLOR_LIMIT; i++) { + for (let i = 0; i < colorSlotCount(options.THEME_COLOR_LIMIT); i++) { const borderColor = borderColorArray[i % borderColorArray.length]; sections += ` diff --git a/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts b/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts index 7df85e53624..880a7686737 100644 --- a/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts +++ b/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts @@ -1,44 +1,45 @@ /** - * The per-item palette is opt-in per diagram: `clusters.js` and the shape files stamp a - * `data-color-id` attribute unconditionally, and a diagram only renders colour if its - * stylesheet emits the matching rules. That gate is a `COLOR_THEMES` set duplicated in - * each stylesheet, so it is easy to widen one by accident. + * The per-item palette is opt-in per diagram: shapes and containers stamp a + * `data-color-id` slot via `stampColorSlot`, and a diagram renders colour only if its + * stylesheet emits the matching rules. * * These assertions are about the gate, not the colours: a stylesheet must stay silent for - * every theme outside the colour pair, and must emit one rule per palette slot inside it. + * every theme outside the colour pair, must emit one rule per palette slot inside it, and + * must never outrank a user's own styling. */ import { describe, expect, it } from 'vitest'; import themes from '../../themes/index.js'; import classStyles from '../class/styles.js'; import flowchartStyles from '../flowchart/styles.js'; +import { COLOR_THEMES, safeLook } from './colorThemeGate.js'; const STYLESHEETS = { class: classStyles, flowchart: flowchartStyles, } as const; -const COLOUR_THEMES = ['redux-color', 'redux-dark-color']; -const PLAIN_THEMES = [ - 'default', - 'base', - 'dark', - 'forest', - 'neutral', - 'neo', - 'neo-dark', - 'redux', - 'redux-dark', -]; +const COLOUR_THEMES = [...COLOR_THEMES]; -const render = (name: keyof typeof STYLESHEETS, themeName: string) => { +/** + * Derived rather than listed. A hardcoded list stops being exhaustive the moment someone + * registers a new theme — the same "widened without anyone noticing" failure this file + * exists to catch, one level up. + */ +const PLAIN_THEMES = Object.keys(themes).filter((name) => !COLOR_THEMES.has(name)); + +const render = (name: keyof typeof STYLESHEETS, themeName: string, look = 'classic') => { const themeVariables = themes[themeName as keyof typeof themes].getThemeVariables({}); return STYLESHEETS[name]({ ...(themeVariables as unknown as Record), theme: themeName, - look: 'classic', + look, } as never); }; +it('covers every registered theme between the two lists', () => { + expect([...PLAIN_THEMES, ...COLOUR_THEMES].sort()).toEqual(Object.keys(themes).sort()); +}); + describe.each(Object.keys(STYLESHEETS) as (keyof typeof STYLESHEETS)[])('%s stylesheet', (name) => { it.each(PLAIN_THEMES)('emits no per-item colour rules for %s', (themeName) => { expect(render(name, themeName)).not.toContain('data-color-id'); @@ -62,4 +63,39 @@ describe.each(Object.keys(STYLESHEETS) as (keyof typeof STYLESHEETS)[])('%s styl expect(bodies.length).toBeGreaterThan(0); expect(bodies.filter((body) => body.includes('!important'))).toEqual([]); }); + + it.each(COLOUR_THEMES)('keeps a hostile look out of the selector for %s', (themeName) => { + const css = render(name, themeName, 'classic"]{a'); + expect(css).not.toContain('classic"]{a'); + expect(css).toContain('[data-look="classic"]'); + }); +}); + +/** + * `look` is interpolated straight into the palette rules' selector, and it is a top-level + * config key — so it is reachable from diagram text through frontmatter or an init + * directive, and `config.sanitize` only drops values containing `<`, `>` or `url(data:`. + * Braces and quotes survive, which is enough to close the attribute selector early and + * open a rule block of the author's choosing, escaping the `#svgId` scoping that stylis + * applies. Every real look is a bare word, so anything else is rejected outright rather + * than escaped. + */ +describe('safeLook', () => { + it.each(['classic', 'handDrawn', 'neo', 'some-look', 'A_1'])( + 'passes the bare word %s', + (look) => { + expect(safeLook(look)).toBe(look); + } + ); + + it.each(['classic"]{a{b', 'classic"] *', 'classic}', 'classic ', '', 'a"]{}'])( + 'falls back to classic for %j', + (look) => { + expect(safeLook(look)).toBe('classic'); + } + ); + + it('falls back to classic when look is absent', () => { + expect(safeLook(undefined)).toBe('classic'); + }); }); diff --git a/packages/mermaid/src/diagrams/common/colorThemeGate.ts b/packages/mermaid/src/diagrams/common/colorThemeGate.ts new file mode 100644 index 00000000000..6cdfd2bacd8 --- /dev/null +++ b/packages/mermaid/src/diagrams/common/colorThemeGate.ts @@ -0,0 +1,78 @@ +/** + * The per-item colour palette is opt-in per diagram. A shape or container stamps a + * `data-color-id` slot on its rendered element, and the diagram's stylesheet maps that + * slot to a border and fill. Both halves need the same answers to the same three + * questions — is this a colour theme, does it actually carry a palette, and which slot + * does this item get — so they live here rather than being restated per diagram. + * + * Before this module the `COLOR_THEMES` list existed in five copies and the stamping + * block was duplicated verbatim between `clusters.js` and `classBox.ts`. Two idioms for + * the same gate had already appeared: `er/styles.ts` keys off the theme name while + * `requirement/styles.js` keys off the array being non-empty. Anything added here is + * added once. + */ +import type { D3Selection } from '../../types.js'; + +/** Themes that carry a categorical palette for per-item colouring. */ +export const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); + +/** How many palette slots to emit when the theme does not say. */ +export const DEFAULT_COLOR_SLOTS = 12; + +/** + * A palette array is usable only if it is genuinely a non-empty array. A truthy `.length` + * check passes for a plain string too, which would yield a per-character "palette" and + * declarations like `stroke: r;`. + */ +export const hasPalette = (palette: unknown): palette is string[] => + Array.isArray(palette) && palette.length > 0; + +/** Whether `theme` should render per-item colour at all. */ +export const isColorTheme = (theme: string | undefined, palette: unknown): boolean => + theme != null && COLOR_THEMES.has(theme) && hasPalette(palette); + +/** + * `look` is interpolated into a CSS selector by every stylesheet that emits palette + * rules, and it is a top-level config key — so it is settable from diagram text via + * frontmatter or an init directive, and `config.sanitize` only removes values containing + * `<`, `>` or `url(data:`. Braces and quotes survive, which is enough to close the + * attribute selector early and open a rule block of the caller's choosing, escaping the + * `#svgId` scoping stylis applies. + * + * Every real look is a bare word, so anything else is rejected outright rather than + * escaped. Validate here, at the point of interpolation, so no caller has to remember. + */ +const SAFE_LOOK = /^[\w-]+$/; + +export const safeLook = (look: string | undefined): string => + look != null && SAFE_LOOK.test(look) ? look : 'classic'; + +/** + * Number of palette slots a stylesheet should emit, clamped to what the palette can + * actually supply so a slot never resolves to `undefined`. + */ +export const colorSlotCount = (themeColorLimit: unknown): number => + typeof themeColorLimit === 'number' && themeColorLimit > 0 + ? themeColorLimit + : DEFAULT_COLOR_SLOTS; + +/** + * Stamp the element with its palette slot, so the diagram's `[data-color-id]` rules can + * find it. A no-op for every theme without a palette, which is what keeps this safe to + * call from shared rendering code used by diagrams that never opt in. + * + * The slot wraps at the palette length rather than indexing raw, so a palette shorter + * than the emitted slot count cannot produce `stroke: undefined`. + */ +export const stampColorSlot = ( + shapeSvg: D3Selection, + colorIndex: number | undefined, + theme: string | undefined, + palette: unknown +): void => { + if (!isColorTheme(theme, palette)) { + return; + } + const slot = (colorIndex ?? 0) % (palette as string[]).length; + shapeSvg.attr('data-color-id', `color-${slot}`); +}; diff --git a/packages/mermaid/src/diagrams/flowchart/styles.ts b/packages/mermaid/src/diagrams/flowchart/styles.ts index 2e69bdefbab..c6ee55a6c6b 100644 --- a/packages/mermaid/src/diagrams/flowchart/styles.ts +++ b/packages/mermaid/src/diagrams/flowchart/styles.ts @@ -1,6 +1,7 @@ // import khroma from 'khroma'; import * as khroma from 'khroma'; import { getIconStyles } from '../globalStyles.js'; +import { colorSlotCount, hasPalette, isColorTheme, safeLook } from '../common/colorThemeGate.js'; /** Returns the styles given options */ export interface FlowChartStyleOptions { @@ -25,37 +26,49 @@ export interface FlowChartStyleOptions { bkgColorArray?: string[]; } -const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); - /** * Cycling per-subgraph colour. Only the containers are painted -- the nodes inside keep * the uniform look, because a flowchart node is a step in a flow rather than a distinct * participant, and node colour is already how `classDef` / `style` carry meaning. * * Emits both the `rect` (classic/neo) and `path` (handDrawn) forms since the container is - * a plain rect in one look and a roughjs path pair in the other. Not `!important`: - * `clusters.js` puts user styles in an inline `style` attribute, which has to keep - * winning over the theme palette. + * a plain rect in one look and a roughjs path pair in the other. `.collapsed-group` is + * the same container drawn as a compact node by `collapsedGroup.ts` — it is a container, + * not one of the flow's steps, so it takes the palette too; without it a collapsed + * subgraph rendered uncoloured beside tinted siblings. + * + * Not `!important`: `clusters.js` and `collapsedGroup.ts` both put user styles in an + * inline `style` attribute, which has to keep winning over the theme palette. The + * collapsed form's own colours are presentation attributes (`fill=` / `stroke=`), which + * these rules correctly outrank while still losing to that inline style. */ const genColor = (options: FlowChartStyleOptions) => { - const { theme, look, bkgColorArray, borderColorArray } = options; - if (!theme || !COLOR_THEMES.has(theme) || !borderColorArray?.length) { + const { theme, bkgColorArray, borderColorArray } = options; + if (!isColorTheme(theme, borderColorArray)) { return ''; } - const hasBkgColors = (bkgColorArray?.length ?? 0) > 0; + const look = safeLook(options.look); + const hasBkgColors = hasPalette(bkgColorArray); let sections = ''; - for (let i = 0; i < (options.THEME_COLOR_LIMIT ?? 12); i++) { - const borderColor = borderColorArray[i % borderColorArray.length]; - const fill = hasBkgColors ? `fill: ${bkgColorArray![i % bkgColorArray!.length]};` : ''; + for (let i = 0; i < colorSlotCount(options.THEME_COLOR_LIMIT); i++) { + const borderColor = borderColorArray![i % borderColorArray!.length]; + const fill = hasBkgColors ? `fill: ${bkgColorArray[i % bkgColorArray.length]};` : ''; + const slot = `[data-look="${look}"][data-color-id="color-${i}"]`; sections += ` - [data-look="${look}"][data-color-id="color-${i}"].cluster rect { + ${slot}.cluster rect { + stroke: ${borderColor}; + ${fill} + } + + ${slot}.cluster path { stroke: ${borderColor}; ${fill} } - [data-look="${look}"][data-color-id="color-${i}"].cluster path { + ${slot}.node .collapsed-group, + ${slot}.node .collapsed-group path { stroke: ${borderColor}; ${fill} } diff --git a/packages/mermaid/src/rendering-util/rendering-elements/clusters.js b/packages/mermaid/src/rendering-util/rendering-elements/clusters.js index d5a9e658279..5d0722e1b66 100644 --- a/packages/mermaid/src/rendering-util/rendering-elements/clusters.js +++ b/packages/mermaid/src/rendering-util/rendering-elements/clusters.js @@ -10,8 +10,7 @@ import createLabel from './createLabel.js'; import { createRoundedRectPathD } from './shapes/roundedRectPath.ts'; import { compileStyles, styles2String, userNodeOverrides } from './shapes/handDrawnShapeStyles.js'; import { swimlane } from './clusters/swimlane.js'; - -const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); +import { stampColorSlot } from '../../diagrams/common/colorThemeGate.js'; const rect = async (parent, node) => { log.info('Creating subgraph rect for ', node.id, node); @@ -28,12 +27,10 @@ const rect = async (parent, node) => { .attr('id', node.domId) .attr('data-look', node.look); - // Per-container colour slot. Only diagrams whose stylesheet defines the matching - // `[data-color-id]` rules paint it; for the rest this is an inert attribute. - if (theme != null && COLOR_THEMES.has(theme) && borderColorArray?.length) { - const colorIndex = node.colorIndex ?? 0; - shapeSvg.attr('data-color-id', `color-${colorIndex % borderColorArray.length}`); - } + // Per-container colour slot. A no-op unless the active theme carries a palette, and + // painted only by diagrams whose stylesheet defines the matching `[data-color-id]` + // rules -- for state, block and class namespaces this is an inert attribute. + stampColorSlot(shapeSvg, node.colorIndex, theme, borderColorArray); const useHtmlLabels = getEffectiveHtmlLabels(siteConfig); diff --git a/packages/mermaid/src/rendering-util/rendering-elements/shapes/classBox.ts b/packages/mermaid/src/rendering-util/rendering-elements/shapes/classBox.ts index f65a1974178..6288bcbfef0 100644 --- a/packages/mermaid/src/rendering-util/rendering-elements/shapes/classBox.ts +++ b/packages/mermaid/src/rendering-util/rendering-elements/shapes/classBox.ts @@ -9,8 +9,7 @@ import intersect from '../intersect/index.js'; import { textHelper } from '../../../diagrams/class/shapeUtil.js'; import { evaluate } from '../../../diagrams/common/common.js'; import type { D3Selection } from '../../../types.js'; - -const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); +import { stampColorSlot } from '../../../diagrams/common/colorThemeGate.js'; export async function classBox(parent: D3Selection, node: Node) { const config = getConfig(); @@ -27,10 +26,7 @@ export async function classBox(parent: D3Selection const { shapeSvg, bbox } = await textHelper(parent, node, config, useHtmlLabels, GAP); - if (theme != null && COLOR_THEMES.has(theme) && borderColorArray?.length) { - const colorIndex = node.colorIndex ?? 0; - shapeSvg.attr('data-color-id', `color-${colorIndex % borderColorArray.length}`); - } + stampColorSlot(shapeSvg, node.colorIndex, theme, borderColorArray); const { labelStyles, nodeStyles } = styles2String(node); node.labelStyle = labelStyles; diff --git a/packages/mermaid/src/rendering-util/rendering-elements/shapes/collapsedGroup.ts b/packages/mermaid/src/rendering-util/rendering-elements/shapes/collapsedGroup.ts index a8fff4e5743..8bc7ca70361 100644 --- a/packages/mermaid/src/rendering-util/rendering-elements/shapes/collapsedGroup.ts +++ b/packages/mermaid/src/rendering-util/rendering-elements/shapes/collapsedGroup.ts @@ -8,6 +8,7 @@ import { createRoundedRectPathD } from './roundedRectPath.js'; import { userNodeOverrides, styles2String } from './handDrawnShapeStyles.js'; import rough from 'roughjs'; import { handleUndefinedAttr } from '../../../utils.js'; +import { stampColorSlot } from '../../../diagrams/common/colorThemeGate.js'; /** Height reserved for the ellipsis indicator row below the title */ const INDICATOR_ROW_HEIGHT = 20; @@ -83,6 +84,12 @@ export async function collapsedGroup( const { shapeSvg, bbox } = await labelHelper(parent, node, getNodeClasses(node)); + // A collapsed subgraph is still a container, so it takes the same palette slot its + // expanded form would. Without this it renders uncoloured beside tinted siblings, and + // the seam shows up exactly when someone uses the collapse feature. + const { theme, themeVariables } = getConfig(); + stampColorSlot(shapeSvg, node.colorIndex, theme, themeVariables.borderColorArray); + const padding = node.padding ?? 8; const titleHeight = bbox.height; const totalWidth = Math.max(bbox.width + padding * 2, MIN_WIDTH, node?.width ?? 0); From 84f394c9c452982d18e45221a5fb2c725842d7e4 Mon Sep 17 00:00:00 2001 From: Ashish Jain Date: Fri, 28 Aug 2026 13:59:07 +0200 Subject: [PATCH 4/8] docs(changeset): shorten the class/flowchart palette changeset Review feedback: cut to one paragraph, matching the norm of the existing changesets on develop. --- .changeset/redux-color-class-flowchart-palette.md | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.changeset/redux-color-class-flowchart-palette.md b/.changeset/redux-color-class-flowchart-palette.md index bc61fe2c1d2..3e56712d33c 100644 --- a/.changeset/redux-color-class-flowchart-palette.md +++ b/.changeset/redux-color-class-flowchart-palette.md @@ -2,8 +2,4 @@ 'mermaid': minor --- -feat(themes): class boxes and flowchart subgraph containers now pick up the per-item colour palette under the `redux-color` and `redux-dark-color` themes. - -Previously only ER, sequence, git and requirement diagrams read `borderColorArray` / `bkgColorArray`. Each class now gets its own border and fill, cycling every 12 as ER entities do; namespaces and notes stay outside the cycle. Each flowchart subgraph container gets its own colour — including collapsed ones, which keep the slot they would have had expanded — while nodes inside stay uniform, since node colour is already how `classDef` / `style` carry meaning. - -Explicit user styling still wins: `classDef` and `style` become inline `style` attributes and none of the new rules are `!important`. +feat(themes): class boxes and flowchart subgraph containers now take a per-item colour under the `redux-color` and `redux-dark-color` themes, cycling every 12 as ER entities already do. Collapsed subgraphs keep the slot they would have had expanded; nodes inside a subgraph stay uniform, and explicit `classDef` or `style` still wins over the palette. From 33468883251c39009616ab711bad5acc40e4eef8 Mon Sep 17 00:00:00 2001 From: Ashish Jain Date: Fri, 28 Aug 2026 17:21:47 +0200 Subject: [PATCH 5/8] fix(flowchart): give subgraph colour slots real source order, and close two seams The namespace test could not fail. `addClassesToNamespace` early-returns when the namespace does not exist, and the test never called `addNamespace` -- so no namespace node was created and the assertion passed on absence rather than on behaviour. Verified the reviewer's check: giving namespaces `colorIndex: 99`, the exact thing the test forbids, left all three passing. It now creates the namespace and attaches a class, and fails on that regression. Subgraph colour slots did not follow source order for nested subgraphs, contrary to what the code, the test name and the description all claimed. `addSubGraph` runs when a subgraph *closes*, so `subGraphs` holds nested ones before their parent: `Outer { InnerOne, InnerTwo }, Sibling` arrives as [InnerOne, InnerTwo, Outer, Sibling], and taking the array index handed Outer slot 2 while its own children took 0 and 1. A pre-order walk of the containment forest recovers source order, assigned once so the collapsed and expanded branches cannot drift. The existing test could not have caught it -- three flat subgraphs, where close order and source order are identical. Replaced with a nested fixture, and confirmed it is the only test in the file that fails against the old `colorIndex: i`. Collapsed markers kept their default colour. The palette reached `.collapsed-group` but not `.collapsed-indicator` or `.collapsed-separator`, which take `clusterBorder` further down -- so a collapsed subgraph rendered a tinted container with default-coloured dots and separator, the same seam one level down from the one this PR set out to close. Those selectors also now cover `rough-node`. `collapsedGroup.ts` goes through `getNodeClasses`, which returns `rough-node` rather than `node` under the handDrawn look, so a `.node`-only selector left handDrawn collapsed containers uncoloured. Each descendant is appended to both prefixes separately: writing `SLOT.node, SLOT.rough-node .thing` would attach the descendant to the last item of the list only and silently match nothing under the classic look. Checked the emitted CSS rather than assuming. Clusters were never affected -- `clusters.js` sets the `cluster` class directly. Corrected `colorSlotCount`'s docstring: it claimed to clamp to what the palette can supply, but it never sees a palette -- it only floors a non-numeric THEME_COLOR_LIMIT. The clamping is `stampColorSlot`'s, via `% palette.length`. --- .../class/classDiagram-colorIndex.spec.ts | 6 ++- .../src/diagrams/common/colorThemeGate.ts | 6 ++- .../src/diagrams/flowchart/flowDb.spec.ts | 29 ++++++++++++- .../mermaid/src/diagrams/flowchart/flowDb.ts | 41 +++++++++++++++++-- .../mermaid/src/diagrams/flowchart/styles.ts | 25 ++++++++++- 5 files changed, 97 insertions(+), 10 deletions(-) diff --git a/packages/mermaid/src/diagrams/class/classDiagram-colorIndex.spec.ts b/packages/mermaid/src/diagrams/class/classDiagram-colorIndex.spec.ts index 83cef0b456f..ac1ad0a73de 100644 --- a/packages/mermaid/src/diagrams/class/classDiagram-colorIndex.spec.ts +++ b/packages/mermaid/src/diagrams/class/classDiagram-colorIndex.spec.ts @@ -32,8 +32,12 @@ describe('class diagram colour slots', () => { }); it('does not spend a slot on a namespace container', () => { - classDb.addClassesToNamespace('shop', [], []); + // `addNamespace` first: `addClassesToNamespace` early-returns when the namespace does + // not exist, so without it no namespace node is created and the assertion below passes + // on absence rather than on behaviour. + classDb.addNamespace('shop'); classDb.addClass('Order'); + classDb.addClassesToNamespace('shop', ['Order'], []); classDb.addClass('Customer'); const slots = colorIndexById(); diff --git a/packages/mermaid/src/diagrams/common/colorThemeGate.ts b/packages/mermaid/src/diagrams/common/colorThemeGate.ts index 6cdfd2bacd8..808418a226d 100644 --- a/packages/mermaid/src/diagrams/common/colorThemeGate.ts +++ b/packages/mermaid/src/diagrams/common/colorThemeGate.ts @@ -48,8 +48,10 @@ export const safeLook = (look: string | undefined): string => look != null && SAFE_LOOK.test(look) ? look : 'classic'; /** - * Number of palette slots a stylesheet should emit, clamped to what the palette can - * actually supply so a slot never resolves to `undefined`. + * Number of palette slots a stylesheet should emit. This only floors a missing or + * non-numeric `THEME_COLOR_LIMIT` to the default -- it never sees a palette, so it cannot + * clamp to one. Keeping a slot from resolving to `undefined` is `stampColorSlot`'s job, + * via the `% palette.length` wrap, and each stylesheet's own wrap when it indexes directly. */ export const colorSlotCount = (themeColorLimit: unknown): number => typeof themeColorLimit === 'number' && themeColorLimit > 0 diff --git a/packages/mermaid/src/diagrams/flowchart/flowDb.spec.ts b/packages/mermaid/src/diagrams/flowchart/flowDb.spec.ts index ffca5818311..5b40d0b642a 100644 --- a/packages/mermaid/src/diagrams/flowchart/flowDb.spec.ts +++ b/packages/mermaid/src/diagrams/flowchart/flowDb.spec.ts @@ -319,7 +319,7 @@ describe('flow db subgraph colour slots', () => { const attachMeta = (id: string, meta: string) => flowDb.addVertex(id, undefined as unknown as FlowText, undefined, [], [], '', {}, meta); - it('numbers subgraphs in declaration order, not the reverse order getData walks', () => { + it('numbers flat subgraphs in source order', () => { for (const id of ['A', 'B', 'C']) { addVertex(id); } @@ -332,6 +332,33 @@ describe('flow db subgraph colour slots', () => { expect([slot('first'), slot('second'), slot('third')]).toEqual([0, 1, 2]); }); + it('numbers nested subgraphs in source order, parent before its children', () => { + /* The discriminating case. `addSubGraph` is called when a subgraph *closes*, so + * `subGraphs` holds nested ones before their parent -- here [InnerOne, InnerTwo, + * Outer, Sibling]. Taking the array index directly gave Outer slot 2 while its own + * children took 0 and 1. + * + * Three flat subgraphs cannot catch that: for siblings, close order and source order + * are the same. Only nesting separates them. + */ + for (const id of ['A', 'B', 'C', 'D']) { + addVertex(id); + } + flowDb.addSubGraph({ text: 'InnerOne' }, ['A'], { text: 'InnerOne', type: 'text' }); + flowDb.addSubGraph({ text: 'InnerTwo' }, ['B'], { text: 'InnerTwo', type: 'text' }); + flowDb.addSubGraph({ text: 'Outer' }, ['InnerOne', 'InnerTwo'], { + text: 'Outer', + type: 'text', + }); + flowDb.addSubGraph({ text: 'Sibling' }, ['D'], { text: 'Sibling', type: 'text' }); + + const { nodes } = flowDb.getData(); + const slot = (id: string) => nodes.find((n) => n.id === id)?.colorIndex; + expect([slot('Outer'), slot('InnerOne'), slot('InnerTwo'), slot('Sibling')]).toEqual([ + 0, 1, 2, 3, + ]); + }); + it('keeps a collapsed subgraph on its own slot so the cycle does not shift', () => { for (const id of ['A', 'B', 'C']) { addVertex(id); diff --git a/packages/mermaid/src/diagrams/flowchart/flowDb.ts b/packages/mermaid/src/diagrams/flowchart/flowDb.ts index 49424aab555..258ef9018c8 100644 --- a/packages/mermaid/src/diagrams/flowchart/flowDb.ts +++ b/packages/mermaid/src/diagrams/flowchart/flowDb.ts @@ -1118,6 +1118,39 @@ You have to call mermaid.initialize.` } } } + /* Colour slots follow the order the subgraphs appear in the source. + * + * `subGraphs` is not in that order: the grammar reduces a subgraph when it *closes*, + * so a nested one lands before its parent -- `Outer { InnerOne, InnerTwo }, Sibling` + * arrives as [InnerOne, InnerTwo, Outer, Sibling]. Taking the array index directly + * would hand Outer slot 2 while its own children took 0 and 1. + * + * A pre-order walk of the containment forest recovers source order: roots complete in + * source order relative to each other, and a parent is always declared before the + * children it contains. Assigned once here so the collapsed and expanded branches + * below cannot drift apart. + */ + const declarationIndex = new Map(); + const childrenOf = new Map(); + for (const sg of subGraphs) { + const parent = subGraphParent.get(sg.id); + if (parent !== undefined) { + childrenOf.set(parent, [...(childrenOf.get(parent) ?? []), sg.id]); + } + } + let nextDeclarationIndex = 0; + const walk = (sgId: string) => { + declarationIndex.set(sgId, nextDeclarationIndex++); + for (const childId of childrenOf.get(sgId) ?? []) { + walk(childId); + } + }; + for (const sg of subGraphs) { + if (!subGraphParent.has(sg.id)) { + walk(sg.id); + } + } + const isCollapsed = (sgId: string) => this.subGraphLookup.get(sgId)?.metadata?.view === 'collapsed'; const outermostCollapsed = (sgId: string): string | undefined => { @@ -1193,9 +1226,9 @@ You have to call mermaid.initialize.` dir: subGraph.dir, isGroup: false, look: config.look, - // A collapsed subgraph still consumes its slot so the colour cycle does not - // shift when one is collapsed. `collapsedGroup` does not paint it yet. - colorIndex: i, + // A collapsed subgraph still consumes its slot, so the cycle does not shift + // when one is collapsed. + colorIndex: declarationIndex.get(subGraph.id), }); } else { nodes.push({ @@ -1211,7 +1244,7 @@ You have to call mermaid.initialize.` dir: subGraph.dir, isGroup: true, look: config.look, - colorIndex: i, + colorIndex: declarationIndex.get(subGraph.id), // Forwarded so layout engines can read per-container settings such as // `@{ algorithm: elk.box }`. `view` is consumed above; everything else // is opaque here and simply passed through. The cast is the diff --git a/packages/mermaid/src/diagrams/flowchart/styles.ts b/packages/mermaid/src/diagrams/flowchart/styles.ts index c6ee55a6c6b..86add1adfb1 100644 --- a/packages/mermaid/src/diagrams/flowchart/styles.ts +++ b/packages/mermaid/src/diagrams/flowchart/styles.ts @@ -55,6 +55,17 @@ const genColor = (options: FlowChartStyleOptions) => { const borderColor = borderColorArray![i % borderColorArray!.length]; const fill = hasBkgColors ? `fill: ${bkgColorArray[i % bkgColorArray.length]};` : ''; const slot = `[data-look="${look}"][data-color-id="color-${i}"]`; + /* A collapsed subgraph is drawn by `collapsedGroup.ts` through `getNodeClasses`, which + * returns `rough-node` instead of `node` for the handDrawn look -- so a `.node`-only + * selector leaves handDrawn collapsed containers uncoloured beside their tinted + * siblings. Clusters are unaffected: `clusters.js` sets the `cluster` class directly. + * + * Each descendant has to be appended to *both* prefixes separately. Writing + * `${slot}.node, ${slot}.rough-node .thing` would attach the descendant to the last + * item of the list only, silently matching nothing under the classic look. + */ + const collapsedRule = (suffix: string) => + `${slot}.node ${suffix}, ${slot}.rough-node ${suffix}`; sections += ` ${slot}.cluster rect { @@ -67,11 +78,21 @@ const genColor = (options: FlowChartStyleOptions) => { ${fill} } - ${slot}.node .collapsed-group, - ${slot}.node .collapsed-group path { + ${collapsedRule('.collapsed-group')}, + ${collapsedRule('.collapsed-group path')} { stroke: ${borderColor}; ${fill} } + + /* The ellipsis dots and the separator take clusterBorder further down, so without + these the container is palette-coloured while its own markers are not. */ + ${collapsedRule('.collapsed-indicator')} { + fill: ${borderColor}; + } + + ${collapsedRule('.collapsed-separator')} { + stroke: ${borderColor}; + } `; } return sections; From fe5fec18f2e4dbe62e38874e73bd722ce4d31104 Mon Sep 17 00:00:00 2001 From: Ashish Jain Date: Fri, 28 Aug 2026 19:39:16 +0200 Subject: [PATCH 6/8] fix(themes): emit a palette rule for every slot that can be stamped `colorSlotCount` returned `THEME_COLOR_LIMIT`, but `stampColorSlot` wraps at `palette.length`. The two disagree whenever a palette is longer than the limit: its tail gets stamped `color-N` with no rule emitted for that slot, and those items render uncoloured beside their neighbours. Confirmed with a limit of 3 and a five-entry palette -- the stylesheet emitted slots 0..2 while the fifth item was stamped `color-4`. The count now covers the palette as well as the limit, so every slot that can be stamped has a rule. A shorter palette still emits the full limit and cycles, as before. No change for either shipped colour theme: both carry exactly THEME_COLOR_LIMIT entries, so the two counts already agreed -- which is what hid this. It takes a `themeVariables` override to reach, the same shape as the length mismatches this PR's shared gate exists to prevent. The new assertions name the slots that lost their rule rather than counting them, and were checked against the old `return limit` -- three fail, listing slots 3..19. --- packages/mermaid/src/diagrams/class/styles.js | 2 +- .../diagrams/common/colorThemeGate.spec.ts | 78 ++++++++++++++++++- .../src/diagrams/common/colorThemeGate.ts | 26 +++++-- .../mermaid/src/diagrams/flowchart/styles.ts | 2 +- 4 files changed, 96 insertions(+), 12 deletions(-) diff --git a/packages/mermaid/src/diagrams/class/styles.js b/packages/mermaid/src/diagrams/class/styles.js index 7d59807b3e1..4d55b43f986 100644 --- a/packages/mermaid/src/diagrams/class/styles.js +++ b/packages/mermaid/src/diagrams/class/styles.js @@ -20,7 +20,7 @@ const genColor = (options) => { const hasBkgColors = hasPalette(bkgColorArray); let sections = ''; - for (let i = 0; i < colorSlotCount(options.THEME_COLOR_LIMIT); i++) { + for (let i = 0; i < colorSlotCount(options.THEME_COLOR_LIMIT, borderColorArray); i++) { const borderColor = borderColorArray[i % borderColorArray.length]; sections += ` diff --git a/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts b/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts index 880a7686737..980c4184989 100644 --- a/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts +++ b/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts @@ -11,7 +11,7 @@ import { describe, expect, it } from 'vitest'; import themes from '../../themes/index.js'; import classStyles from '../class/styles.js'; import flowchartStyles from '../flowchart/styles.js'; -import { COLOR_THEMES, safeLook } from './colorThemeGate.js'; +import { COLOR_THEMES, colorSlotCount, safeLook } from './colorThemeGate.js'; const STYLESHEETS = { class: classStyles, @@ -27,15 +27,25 @@ const COLOUR_THEMES = [...COLOR_THEMES]; */ const PLAIN_THEMES = Object.keys(themes).filter((name) => !COLOR_THEMES.has(name)); -const render = (name: keyof typeof STYLESHEETS, themeName: string, look = 'classic') => { +const render = ( + name: keyof typeof STYLESHEETS, + themeName: string, + look = 'classic', + overrides: Record = {} +) => { const themeVariables = themes[themeName as keyof typeof themes].getThemeVariables({}); return STYLESHEETS[name]({ ...(themeVariables as unknown as Record), theme: themeName, look, + ...overrides, } as never); }; +/** The distinct `color-N` slots a stylesheet emits rules for. */ +const emittedSlots = (css: string): Set => + new Set([...css.matchAll(/data-color-id="color-(\d+)"/g)].map((m) => Number(m[1]))); + it('covers every registered theme between the two lists', () => { expect([...PLAIN_THEMES, ...COLOUR_THEMES].sort()).toEqual(Object.keys(themes).sort()); }); @@ -99,3 +109,67 @@ describe('safeLook', () => { expect(safeLook(undefined)).toBe('classic'); }); }); + +/** + * `stampColorSlot` wraps at `palette.length`; the stylesheets emit one rule per slot up to + * `colorSlotCount`. Those two counts have to agree, or an item gets stamped `color-N` with + * no rule emitted for it and renders uncoloured beside its neighbours. + * + * Both shipped colour themes carry exactly `THEME_COLOR_LIMIT` entries, so they agree by + * coincidence -- which is what hid this. A `themeVariables` override with a longer palette + * is where they come apart. + */ +describe('colorSlotCount', () => { + it('floors a missing or non-numeric limit to the default', () => { + expect(colorSlotCount(undefined)).toBe(12); + expect(colorSlotCount('12')).toBe(12); + expect(colorSlotCount(0)).toBe(12); + }); + + it('covers the palette when it is longer than the limit', () => { + expect( + colorSlotCount( + 3, + Array.from({ length: 20 }, () => '#000') + ) + ).toBe(20); + }); + + it('keeps the limit when the palette is shorter, so the cycle still repeats', () => { + expect(colorSlotCount(12, ['#a', '#b'])).toBe(12); + }); + + it('keeps the plain limit when given no palette', () => { + expect(colorSlotCount(7)).toBe(7); + }); +}); + +describe.each(Object.keys(STYLESHEETS) as (keyof typeof STYLESHEETS)[])( + '%s stylesheet slot coverage', + (name) => { + it('emits a rule for every slot a palette longer than THEME_COLOR_LIMIT can stamp', () => { + const palette = Array.from({ length: 20 }, (_, i) => `#${(i + 16).toString(16)}0000`); + const slots = emittedSlots( + render(name, 'redux-color', 'classic', { + THEME_COLOR_LIMIT: 3, + borderColorArray: palette, + bkgColorArray: palette, + }) + ); + // Named rather than counted, so a failure says which slots lost their rule. + const missing = [...palette.keys()].filter((i) => !slots.has(i)); + expect(missing).toEqual([]); + }); + + it('still emits the full limit for a palette shorter than it', () => { + const slots = emittedSlots( + render(name, 'redux-color', 'classic', { + THEME_COLOR_LIMIT: 12, + borderColorArray: ['#ff0000', '#00ff00'], + bkgColorArray: ['#ffeeee', '#eeffee'], + }) + ); + expect(slots.size).toBe(12); + }); + } +); diff --git a/packages/mermaid/src/diagrams/common/colorThemeGate.ts b/packages/mermaid/src/diagrams/common/colorThemeGate.ts index 808418a226d..5e24b74ded8 100644 --- a/packages/mermaid/src/diagrams/common/colorThemeGate.ts +++ b/packages/mermaid/src/diagrams/common/colorThemeGate.ts @@ -48,15 +48,25 @@ export const safeLook = (look: string | undefined): string => look != null && SAFE_LOOK.test(look) ? look : 'classic'; /** - * Number of palette slots a stylesheet should emit. This only floors a missing or - * non-numeric `THEME_COLOR_LIMIT` to the default -- it never sees a palette, so it cannot - * clamp to one. Keeping a slot from resolving to `undefined` is `stampColorSlot`'s job, - * via the `% palette.length` wrap, and each stylesheet's own wrap when it indexes directly. + * Number of palette slots a stylesheet should emit. + * + * A missing or non-numeric `THEME_COLOR_LIMIT` floors to the default. Beyond that, the + * count must cover every slot `stampColorSlot` can actually assign, which is + * `palette.length` -- it wraps there, not at the limit. A palette longer than the limit + * would otherwise have its tail stamped as `color-N` with no rule emitted for it, and + * those items would render uncoloured beside their neighbours. Both shipped colour themes + * carry exactly `THEME_COLOR_LIMIT` entries, so this only bites a `themeVariables` + * override -- but the two counts agreeing today is what hid it. + * + * Passing no palette keeps the plain limit, for callers that emit slots without stamping. */ -export const colorSlotCount = (themeColorLimit: unknown): number => - typeof themeColorLimit === 'number' && themeColorLimit > 0 - ? themeColorLimit - : DEFAULT_COLOR_SLOTS; +export const colorSlotCount = (themeColorLimit: unknown, palette?: unknown): number => { + const limit = + typeof themeColorLimit === 'number' && themeColorLimit > 0 + ? themeColorLimit + : DEFAULT_COLOR_SLOTS; + return hasPalette(palette) ? Math.max(limit, palette.length) : limit; +}; /** * Stamp the element with its palette slot, so the diagram's `[data-color-id]` rules can diff --git a/packages/mermaid/src/diagrams/flowchart/styles.ts b/packages/mermaid/src/diagrams/flowchart/styles.ts index 86add1adfb1..69b7b43cf7b 100644 --- a/packages/mermaid/src/diagrams/flowchart/styles.ts +++ b/packages/mermaid/src/diagrams/flowchart/styles.ts @@ -51,7 +51,7 @@ const genColor = (options: FlowChartStyleOptions) => { const hasBkgColors = hasPalette(bkgColorArray); let sections = ''; - for (let i = 0; i < colorSlotCount(options.THEME_COLOR_LIMIT); i++) { + for (let i = 0; i < colorSlotCount(options.THEME_COLOR_LIMIT, borderColorArray); i++) { const borderColor = borderColorArray![i % borderColorArray!.length]; const fill = hasBkgColors ? `fill: ${bkgColorArray[i % bkgColorArray.length]};` : ''; const slot = `[data-look="${look}"][data-color-id="color-${i}"]`; From c7a9ad2adf284134b98e9514cc916ab93195a23c Mon Sep 17 00:00:00 2001 From: Ashish Jain Date: Fri, 28 Aug 2026 20:07:14 +0200 Subject: [PATCH 7/8] fix(themes): keep colorSlotCount's result usable as a loop bound The stylesheets pass this straight into a `for` condition, so it has to be a value a loop can finish on. `typeof x === 'number' && x > 0` was not: `Infinity` satisfies both, and the class and flowchart generators then loop forever instead of rendering something merely wrong. Reachable from diagram text, not only from site config, which is worse than the review put it: `THEME_COLOR_LIMIT: .inf` in front matter parses to `Infinity` under the `JSON_SCHEMA` mermaid loads YAML with -- confirmed against the pinned js-yaml. A large finite integer such as `1e9` wedges generation just as effectively while passing every previous check. Now accepts only a positive integer up to `MAX_COLOR_SLOTS`, and falls back to the default otherwise. The cap bounds the loop rather than expressing a design limit -- every shipped palette has twelve entries, so 64 is generous. Verified in both directions: the pre-fix bound for `Infinity` is `Infinity` and a loop on it was still running after 121 million iterations, while class stylesheet generation with `THEME_COLOR_LIMIT: Infinity` now terminates and emits exactly twelve slots. --- .../diagrams/common/colorThemeGate.spec.ts | 42 ++++++++++++++++++- .../src/diagrams/common/colorThemeGate.ts | 36 ++++++++++++---- 2 files changed, 68 insertions(+), 10 deletions(-) diff --git a/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts b/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts index 980c4184989..91f7963c66c 100644 --- a/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts +++ b/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts @@ -11,7 +11,13 @@ import { describe, expect, it } from 'vitest'; import themes from '../../themes/index.js'; import classStyles from '../class/styles.js'; import flowchartStyles from '../flowchart/styles.js'; -import { COLOR_THEMES, colorSlotCount, safeLook } from './colorThemeGate.js'; +import { + COLOR_THEMES, + DEFAULT_COLOR_SLOTS, + MAX_COLOR_SLOTS, + colorSlotCount, + safeLook, +} from './colorThemeGate.js'; const STYLESHEETS = { class: classStyles, @@ -173,3 +179,37 @@ describe.each(Object.keys(STYLESHEETS) as (keyof typeof STYLESHEETS)[])( }); } ); + +/** + * Whatever the limit, the result is used directly as a `for` bound, so it has to be a value + * a loop can finish on. `Infinity` is reachable from diagram text: + * `THEME_COLOR_LIMIT: .inf` in front matter parses to it under the `JSON_SCHEMA` mermaid + * loads YAML with, and a large finite value such as `1e9` wedges generation just as + * effectively. + */ +describe('colorSlotCount stays a usable loop bound', () => { + it.each([ + ['Infinity', Number.POSITIVE_INFINITY], + ['-Infinity', Number.NEGATIVE_INFINITY], + ['NaN', Number.NaN], + ['a huge integer', 1e9], + ['a fraction', 12.5], + ['a negative', -1], + ])('falls back to the default for %s', (_label, value) => { + expect(colorSlotCount(value)).toBe(DEFAULT_COLOR_SLOTS); + }); + + it('caps the palette-covering path too', () => { + const huge = Array.from({ length: MAX_COLOR_SLOTS + 50 }, () => '#000'); + expect(colorSlotCount(12, huge)).toBe(MAX_COLOR_SLOTS); + }); + + it('always yields a finite positive integer within the cap', () => { + for (const value of [Number.POSITIVE_INFINITY, 1e9, Number.NaN, -1, 12.5, 'x', {}]) { + const bound = colorSlotCount(value); + expect(Number.isInteger(bound)).toBe(true); + expect(bound).toBeGreaterThan(0); + expect(bound).toBeLessThanOrEqual(MAX_COLOR_SLOTS); + } + }); +}); diff --git a/packages/mermaid/src/diagrams/common/colorThemeGate.ts b/packages/mermaid/src/diagrams/common/colorThemeGate.ts index 5e24b74ded8..b417b7f74ce 100644 --- a/packages/mermaid/src/diagrams/common/colorThemeGate.ts +++ b/packages/mermaid/src/diagrams/common/colorThemeGate.ts @@ -19,6 +19,12 @@ export const COLOR_THEMES = new Set(['redux-color', 'redux-dark-color']); /** How many palette slots to emit when the theme does not say. */ export const DEFAULT_COLOR_SLOTS = 12; +/** + * Upper bound on slots. Every shipped palette has 12 entries, so this is generous -- it + * exists to bound the loop, not to express a design limit. See `colorSlotCount`. + */ +export const MAX_COLOR_SLOTS = 64; + /** * A palette array is usable only if it is genuinely a non-empty array. A truthy `.length` * check passes for a plain string too, which would yield a per-character "palette" and @@ -50,22 +56,34 @@ export const safeLook = (look: string | undefined): string => /** * Number of palette slots a stylesheet should emit. * - * A missing or non-numeric `THEME_COLOR_LIMIT` floors to the default. Beyond that, the - * count must cover every slot `stampColorSlot` can actually assign, which is - * `palette.length` -- it wraps there, not at the limit. A palette longer than the limit - * would otherwise have its tail stamped as `color-N` with no rule emitted for it, and - * those items would render uncoloured beside their neighbours. Both shipped colour themes - * carry exactly `THEME_COLOR_LIMIT` entries, so this only bites a `themeVariables` - * override -- but the two counts agreeing today is what hid it. + * A missing, non-integer, non-positive or absurdly large `THEME_COLOR_LIMIT` falls back to + * the default. The stylesheets use the result directly as a `for` bound, so it has to be a + * value a loop can finish on: `typeof x === 'number' && x > 0` was not, because `Infinity` + * satisfies both. That is reachable from diagram text rather than only site config -- + * `THEME_COLOR_LIMIT: .inf` in front matter parses to `Infinity` under the `JSON_SCHEMA` + * mermaid loads YAML with -- and a large finite value such as `1e9` wedges generation just + * as effectively. + * + * Beyond that, the count must cover every slot `stampColorSlot` can actually assign, which + * is `palette.length` -- it wraps there, not at the limit. A palette longer than the limit + * would otherwise have its tail stamped as `color-N` with no rule emitted for it, and those + * items would render uncoloured beside their neighbours. Both shipped colour themes carry + * exactly `THEME_COLOR_LIMIT` entries, so that only bites a `themeVariables` override -- + * but the two counts agreeing today is what hid it. * * Passing no palette keeps the plain limit, for callers that emit slots without stamping. */ export const colorSlotCount = (themeColorLimit: unknown, palette?: unknown): number => { const limit = - typeof themeColorLimit === 'number' && themeColorLimit > 0 + typeof themeColorLimit === 'number' && + Number.isInteger(themeColorLimit) && + themeColorLimit > 0 && + themeColorLimit <= MAX_COLOR_SLOTS ? themeColorLimit : DEFAULT_COLOR_SLOTS; - return hasPalette(palette) ? Math.max(limit, palette.length) : limit; + // A real palette is inherently bounded, but cap anyway so no single input can make the + // bound unusable. + return hasPalette(palette) ? Math.min(Math.max(limit, palette.length), MAX_COLOR_SLOTS) : limit; }; /** From e48442d600959ede6ace6ec2dd5ec4eddd8b8168 Mon Sep 17 00:00:00 2001 From: Ashish Jain Date: Fri, 28 Aug 2026 20:28:35 +0200 Subject: [PATCH 8/8] fix(themes): derive the slot count from the palette, not from a bound Three rounds of review have now found the same bug in different clothes, because two places decided the slot space independently: `stampColorSlot` assigns `colorIndex % palette.length`, while the stylesheets emitted `0 .. limit - 1`. Every fix kept both deciders and tried to keep them lined up. - a limit longer than the palette: tail stamped, no rule emitted - a palette longer than the limit (fe5fec18): same, other direction - a palette longer than MAX_COLOR_SLOTS: the cap I added to stop `Infinity` hanging the loop reintroduced it a third time There is only one correct count, and it is not a policy choice. `stampColorSlot` can produce exactly the ids `0 .. palette.length - 1` -- verified by enumerating 500 indices: a two-entry palette yields only slots 0 and 1, a seventy-entry palette reaches slot 69. So the count is `palette.length`; anything less leaves items stamped with no rule, anything more is dead CSS. `paletteSlotCount` is now the single source of truth and both sides read it, which makes the two impossible to disagree rather than something a bound has to police. `THEME_COLOR_LIMIT` still governs callers that emit slots without stamping -- timeline numbers `.section-N` classes rather than palette slots -- and stays bounded there, because a loop runs on it directly and `Infinity` is reachable from front matter. `MAX_COLOR_SLOTS` now applies only to that path. The missing test is the point. Nothing asserted the two sides agree, only that each behaved as its author expected, which is why each round passed its own tests and failed the next review. `emitted slots and stampable slots agree` compares the two sets across palette lengths from 1 to MAX+50 and limits including `Infinity` and `undefined`, per stylesheet as well as per function. Replaying all three historical forms against it: 21, 13 and 10 assertions fail respectively. Two of the corrected expectations were fe5fec18's -- "still emits the full limit for a palette shorter than it" and "keeps the limit when the palette is shorter, so the cycle still repeats". Both are wrong on the merits: with a two-entry palette, slots 2..11 can never be stamped, so those were rules nothing could match. The cycling happens in `stampColorSlot`, not in the rule count. One of the corrected expectations was my own cap, which was the bug this round found. --- .../diagrams/common/colorThemeGate.spec.ts | 83 +++++++++++++++++-- .../src/diagrams/common/colorThemeGate.ts | 46 +++++----- 2 files changed, 101 insertions(+), 28 deletions(-) diff --git a/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts b/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts index 91f7963c66c..7c2adb06f56 100644 --- a/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts +++ b/packages/mermaid/src/diagrams/common/colorThemeGate.spec.ts @@ -16,6 +16,7 @@ import { DEFAULT_COLOR_SLOTS, MAX_COLOR_SLOTS, colorSlotCount, + paletteSlotCount, safeLook, } from './colorThemeGate.js'; @@ -141,8 +142,12 @@ describe('colorSlotCount', () => { ).toBe(20); }); - it('keeps the limit when the palette is shorter, so the cycle still repeats', () => { - expect(colorSlotCount(12, ['#a', '#b'])).toBe(12); + it('matches the palette when it is shorter than the limit', () => { + // Changed from expecting the limit. `stampColorSlot` assigns + // `colorIndex % palette.length`, so a two-entry palette can only ever produce color-0 + // and color-1 -- enumerated over 500 indices to check. Emitting twelve rules would + // leave ten that nothing can match. + expect(colorSlotCount(12, ['#a', '#b'])).toBe(2); }); it('keeps the plain limit when given no palette', () => { @@ -167,7 +172,7 @@ describe.each(Object.keys(STYLESHEETS) as (keyof typeof STYLESHEETS)[])( expect(missing).toEqual([]); }); - it('still emits the full limit for a palette shorter than it', () => { + it('emits exactly the slots a shorter palette can stamp, and no dead rules', () => { const slots = emittedSlots( render(name, 'redux-color', 'classic', { THEME_COLOR_LIMIT: 12, @@ -175,7 +180,9 @@ describe.each(Object.keys(STYLESHEETS) as (keyof typeof STYLESHEETS)[])( bkgColorArray: ['#ffeeee', '#eeffee'], }) ); - expect(slots.size).toBe(12); + // Changed from expecting the full limit: slots 2..11 could never be stamped, so + // they were rules nothing could match. + expect([...slots].sort((a, b) => a - b)).toEqual([0, 1]); }); } ); @@ -199,9 +206,12 @@ describe('colorSlotCount stays a usable loop bound', () => { expect(colorSlotCount(value)).toBe(DEFAULT_COLOR_SLOTS); }); - it('caps the palette-covering path too', () => { + it('does not cap the palette path, because the palette is the bound', () => { + // This assertion previously expected MAX_COLOR_SLOTS, which was the bug: capping here + // let a palette longer than the cap stamp `color-64` and above with no rule emitted. + // A palette is inherently finite, so its own length is the bound. const huge = Array.from({ length: MAX_COLOR_SLOTS + 50 }, () => '#000'); - expect(colorSlotCount(12, huge)).toBe(MAX_COLOR_SLOTS); + expect(colorSlotCount(12, huge)).toBe(huge.length); }); it('always yields a finite positive integer within the cap', () => { @@ -213,3 +223,64 @@ describe('colorSlotCount stays a usable loop bound', () => { } }); }); + +/** + * The invariant every previous round was missing. + * + * Three bugs came out of the emitted slot count and the stamped slot disagreeing: a limit + * longer than the palette (tail stamped, no rule), a palette longer than the limit (same, + * other direction), and a palette longer than the cap added to bound the limit. Each was + * found by review and fixed one at a time, because nothing asserted the two sides agree -- + * only that each behaved as its author expected. + * + * This ties them together. `stampColorSlot` produces `colorIndex % paletteSlotCount`, and + * a stylesheet emits `0 .. colorSlotCount - 1`; those two sets have to be equal for any + * palette and any limit. Both now derive from the palette, so this holds by construction -- + * and if anyone reintroduces a separate bound, it fails here rather than in review. + */ +describe('emitted slots and stampable slots agree', () => { + const paletteOf = (n: number) => Array.from({ length: n }, (_, i) => `#${i}`); + + it.each([ + [1, 12], + [2, 12], + [11, 12], + [12, 12], + [13, 12], + [MAX_COLOR_SLOTS, 12], + [MAX_COLOR_SLOTS + 50, 12], + [12, 3], + [12, Number.POSITIVE_INFINITY], + [12, undefined], + ])('palette of %s with limit %s', (paletteLength, limit) => { + const palette = paletteOf(paletteLength); + + const emitted = new Set(Array.from({ length: colorSlotCount(limit, palette) }, (_, i) => i)); + // Replicates stampColorSlot's arithmetic across far more items than slots. + const stampable = new Set( + Array.from({ length: paletteLength + 200 }, (_, i) => i % paletteSlotCount(palette)) + ); + + expect([...emitted].sort((a, b) => a - b)).toEqual([...stampable].sort((a, b) => a - b)); + }); + + it.each(Object.keys(STYLESHEETS) as (keyof typeof STYLESHEETS)[])( + 'holds end to end for the %s stylesheet', + (name) => { + for (const paletteLength of [2, 12, MAX_COLOR_SLOTS + 50]) { + const palette = paletteOf(paletteLength); + const emitted = emittedSlots( + render(name, 'redux-color', 'classic', { + THEME_COLOR_LIMIT: 12, + borderColorArray: palette, + bkgColorArray: palette, + }) + ); + const stampable = new Set( + Array.from({ length: paletteLength + 200 }, (_, i) => i % paletteSlotCount(palette)) + ); + expect([...emitted].sort((a, b) => a - b)).toEqual([...stampable].sort((a, b) => a - b)); + } + } + ); +}); diff --git a/packages/mermaid/src/diagrams/common/colorThemeGate.ts b/packages/mermaid/src/diagrams/common/colorThemeGate.ts index b417b7f74ce..7bba4757ab9 100644 --- a/packages/mermaid/src/diagrams/common/colorThemeGate.ts +++ b/packages/mermaid/src/diagrams/common/colorThemeGate.ts @@ -56,34 +56,36 @@ export const safeLook = (look: string | undefined): string => /** * Number of palette slots a stylesheet should emit. * - * A missing, non-integer, non-positive or absurdly large `THEME_COLOR_LIMIT` falls back to - * the default. The stylesheets use the result directly as a `for` bound, so it has to be a - * value a loop can finish on: `typeof x === 'number' && x > 0` was not, because `Infinity` - * satisfies both. That is reachable from diagram text rather than only site config -- - * `THEME_COLOR_LIMIT: .inf` in front matter parses to `Infinity` under the `JSON_SCHEMA` - * mermaid loads YAML with -- and a large finite value such as `1e9` wedges generation just - * as effectively. + * With a palette, this is exactly `palette.length` -- not the limit, not a cap. That is not + * a policy choice: `stampColorSlot` assigns `colorIndex % palette.length`, so the ids it + * can produce are precisely `0 .. palette.length - 1`. Emitting fewer leaves items stamped + * with no rule to match, and emitting more is dead CSS. Deriving both from the same length + * is what makes the two impossible to disagree, rather than something a bound has to keep + * lined up. * - * Beyond that, the count must cover every slot `stampColorSlot` can actually assign, which - * is `palette.length` -- it wraps there, not at the limit. A palette longer than the limit - * would otherwise have its tail stamped as `color-N` with no rule emitted for it, and those - * items would render uncoloured beside their neighbours. Both shipped colour themes carry - * exactly `THEME_COLOR_LIMIT` entries, so that only bites a `themeVariables` override -- - * but the two counts agreeing today is what hid it. + * Three separate bugs came out of letting these drift apart -- a limit longer than the + * palette, a palette longer than the limit, and then a palette longer than the cap that was + * added to bound the limit. `paletteSlotCount` below is the single source of truth for both + * sides, and `slotsAgree` in the spec pins that they never diverge again. * - * Passing no palette keeps the plain limit, for callers that emit slots without stamping. + * `THEME_COLOR_LIMIT` still governs callers that emit slots *without* stamping -- timeline + * numbers `.section-N` classes rather than palette slots -- and is bounded there, because a + * loop runs on it directly and `Infinity` is reachable from front matter + * (`THEME_COLOR_LIMIT: .inf` parses to it under the `JSON_SCHEMA` mermaid uses). */ +export const paletteSlotCount = (palette: unknown): number => + hasPalette(palette) ? palette.length : 0; + export const colorSlotCount = (themeColorLimit: unknown, palette?: unknown): number => { - const limit = - typeof themeColorLimit === 'number' && + if (hasPalette(palette)) { + return paletteSlotCount(palette); + } + return typeof themeColorLimit === 'number' && Number.isInteger(themeColorLimit) && themeColorLimit > 0 && themeColorLimit <= MAX_COLOR_SLOTS - ? themeColorLimit - : DEFAULT_COLOR_SLOTS; - // A real palette is inherently bounded, but cap anyway so no single input can make the - // bound unusable. - return hasPalette(palette) ? Math.min(Math.max(limit, palette.length), MAX_COLOR_SLOTS) : limit; + ? themeColorLimit + : DEFAULT_COLOR_SLOTS; }; /** @@ -103,6 +105,6 @@ export const stampColorSlot = ( if (!isColorTheme(theme, palette)) { return; } - const slot = (colorIndex ?? 0) % (palette as string[]).length; + const slot = (colorIndex ?? 0) % paletteSlotCount(palette); shapeSvg.attr('data-color-id', `color-${slot}`); };