Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .changeset/7140-sankey-no-positive-flow.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
---
'@object-ui/plugin-charts': minor
---

A sankey with no positive flow says so, instead of rendering an empty div
(objectui#7140).

`AdvancedChartImpl`'s sankey arm keeps only strictly positive measures, so a
chart handed **real rows** whose measure is all `0`, all `null`, all negative,
or unparseable built no links and returned a bare `<div>`. Measured in Chromium
against a populated control: the control drew 1 `<svg>` / 7 `<path>` /
26 descendants; each of those four tiles rendered `descendantCount: 1`,
`svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to one
another. No marks, no text, no `role` — a tile indistinguishable from a widget
that had crashed, which is the one distinction the file's other refusals exist
to make.

It now renders through the `ChartRefusal` shell those refusals already use —
same box, same `role="status"`, and a new `data-chart-error="no-positive-flow"`
— reading *"This chart has no flow to draw: no row's `<measure>` is above
zero."*

Two boundaries are deliberate and pinned:

- **No rows at all is untouched.** That is the empty-result question, answered
upstream in `ObjectChart` where the query outcome is known; a sentence about
what the rows contain would be false about a dataset with no rows in it.
- **One positive row among zeros still draws.** The refusal fires on an empty
link set, never on a thin one.

One code and one sentence for three causes (a genuinely all-zero flow, values a
flow cannot represent because they are negative, and measures `Number(…) || 0`
folds to zero): naming any single cause would be false for the other two, so
the copy names the predicate the filter actually applies, which is true for all
three. No recovery is promised. Every other chart family is byte-identical —
eight of the twelve tiles in the browser sweep hashed unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* Sankey — rows arrived, none of them is a positive number (objectui#7140).
*
* The sankey arm keeps only strictly positive measures
* (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a chart handed
* REAL rows whose measure is all `0`, all `null`, all negative, or unparseable
* built no links and returned a bare `<div className={className} />`.
*
* Measured in Chromium against a populated control before the fix, at
* `origin/main` e8e4c4df5: the control drew 1 `<svg>` / 7 `<path>` /
* 26 descendants; each of the four blank tiles rendered `descendantCount: 1`,
* `svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to
* one another. Nothing was on the page — no marks, no text, no `role` — so the
* tile was indistinguishable from a widget that had crashed, which is the one
* distinction every other refusal in that file exists to make.
*
* The two boundaries this pins are the ones that make the message TRUE rather
* than merely present:
*
* - **no rows at all still returns the bare div.** That is the empty-RESULT
* question (objectui#7130), answered upstream in `ObjectChart` where the
* query outcome is known. "No row's measure is above zero" would be a false
* sentence about a dataset with no rows in it.
* - **one positive row among zeros still DRAWS.** The refusal fires on an
* empty link set, never on a thin one; a sankey that can draw anything is
* never replaced by prose.
*/
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 480, height: 320 }),
};
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
render(
<AdvancedChartImpl
chartType="sankey"
xAxisKey="stage"
series={SERIES as any}
data={data as any}
/>,
);

const refusalOf = (container: HTMLElement) =>
container.querySelector('[data-chart-error="no-positive-flow"]');

/**
* The four row shapes that reach the empty link set. They are NOT the same
* situation — a genuinely all-zero flow, values a flow cannot represent because
* they are negative, and measures `Number(…) || 0` folds to zero — but the
* predicate the filter applies is the one thing true of all of them, so one
* message serves all four without saying anything false about any of them.
*/
const NO_POSITIVE_ROWS: Array<[string, Array<Record<string, unknown>>]> = [
['every measure is 0', [{ stage: 'Prospecting', amount: 0 }, { stage: 'Won', amount: 0 }]],
['every measure is null', [{ stage: 'Prospecting', amount: null }, { stage: 'Won', amount: null }]],
['every measure is negative', [{ stage: 'Refunds', amount: -40 }, { stage: 'Credits', amount: -12 }]],
['every measure is unparseable', [{ stage: 'A', amount: 'n/a' }, { stage: 'B', amount: 'n/a' }]],
];

describe('AdvancedChartImpl — sankey with no positive flow (objectui#7140)', () => {
it.each(NO_POSITIVE_ROWS)('says so instead of rendering nothing when %s', (_label, rows) => {
const { container } = renderSankey(rows);

const refusal = refusalOf(container);
expect(refusal, 'renders the explanatory placeholder').not.toBeNull();
// A refusal is a STATE, not an alert — the shell the other two refusals in
// this file render through.
expect(refusal?.getAttribute('role')).toBe('status');
// Names the measure it tested, so the author knows WHICH column was all
// zero rather than being told the chart is empty.
expect(refusal?.textContent).toContain('amount');
expect(refusal?.textContent).toContain('above zero');
// The old behaviour was a bare `<div>` with nothing in it. Anything that
// paints a plot here would be a sankey drawn from links that do not exist.
expect(container.querySelector('svg')).toBeNull();
});

it('leaves the no-rows case alone — that is the empty-result question, answered upstream', () => {
const { container } = renderSankey([]);

expect(refusalOf(container), 'no refusal without rows to refuse over').toBeNull();
expect(container.querySelector('svg')).toBeNull();
// Byte-for-byte what this arm returned before objectui#7140: an empty div,
// carrying only the className it was handed.
expect(container.textContent).toBe('');
});

it('still draws when ONE row is positive among zeros', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 0 },
{ stage: 'Proposal', amount: 7 },
{ stage: 'Won', amount: 0 },
]);

expect(refusalOf(container), 'a drawable sankey is never replaced by prose').toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('CONTROL — an all-positive sankey draws, and carries no refusal', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 40 },
{ stage: 'Proposal', amount: 25 },
{ stage: 'Won', amount: 12 },
]);

expect(refusalOf(container)).toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('does not fire for other chart families handed the same all-zero rows', () => {
// The guard lives inside the sankey arm and reads the sankey filter's own
// result, so it cannot reach a family that has no such filter. Pinned
// because a hoisted copy of the predicate is the obvious refactor and would
// blank four working charts: bar/pie/funnel/treemap all render an all-zero
// dataset today (measured in Chromium — axes, labels and legend).
for (const chartType of ['bar', 'pie', 'funnel', 'treemap'] as const) {
const { container } = render(
<AdvancedChartImpl
chartType={chartType}
xAxisKey="stage"
series={SERIES as any}
data={[{ stage: 'A', amount: 0 }, { stage: 'B', amount: 0 }] as any}
/>,
);
expect(refusalOf(container), `${chartType} must be untouched`).toBeNull();
cleanup();
}
});
});
40 changes: 39 additions & 1 deletion packages/plugin-charts/src/AdvancedChartImpl.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1048,7 +1048,45 @@ function AdvancedChartImplInner({
const nodes = [{ name: rootName }, ...rows.map((r) => ({ name: String(r?.[xAxisKey] ?? '') }))];
const links = rows.map((r, i) => ({ source: 0, target: i + 1, value: Number(r?.[dataKey]) || 0 }));
if (links.length === 0) {
return <div className={className} />;
// Rows ARRIVED and the filter above kept none of them, so there is no
// flow to draw. This used to return a bare `<div>` — objectui#7140.
//
// Measured in Chromium before it was changed, against a populated
// control that drew 1 `<svg>` / 7 `<path>`: the all-zero, all-null,
// all-negative and unparseable-measure tiles each rendered ONE element
// and nothing else (`descendantCount: 1`, `svg: 0`, `textContent: ''`),
// and their screenshots were byte-identical to each other. No marks, no
// text, no `role` — the one path in this file that put nothing at all on
// the page, and pixel-identical to a render that crashed. A reader could
// not tell a genuinely all-zero flow from a broken widget, which is the
// distinction every other refusal here exists to make.
//
// Gated on rows being present for the same reason `hasNoCategoryKey` and
// `hasNoPlottableSeries` are: handed NO rows the sentence below would be
// false — there is no row whose measure could be anything. That is the
// empty-RESULT question (objectui#7130), answered upstream in
// `ObjectChart` where the query outcome is known, so this arm leaves the
// no-rows case byte-for-byte as it was.
//
// ONE code and ONE sentence, for the reason `hasNoPlottableSeries`'
// docstring gives: three causes reach here — a genuinely all-zero flow,
// values a flow cannot represent because they are negative, and
// unparseable measures that `Number(…) || 0` folds to zero — and naming
// any ONE of them is a sentence that is false for the other two. The
// predicate the filter actually applies is true for all three, so the
// copy names THAT. No console warning either, unlike the two refusals
// below: those carry a diagnostic pair that does not fit on screen,
// whereas this message already names the key and the exact test it
// failed.
if (data.length === 0) {
return <div className={className} />;
}
return (
<ChartRefusal code="no-positive-flow" className={className}>
This chart has no flow to draw: no row&apos;s{' '}
<code className="font-mono">{dataKey}</code> is above zero.
</ChartRefusal>
);
}
return (
<ChartContainer config={config} className={className} {...containerProps}>
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .changeset/7140-sankey-no-positive-flow.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
---
'@object-ui/plugin-charts': minor
---

A sankey with no positive flow says so, instead of rendering an empty div
(objectui#7140).

`AdvancedChartImpl`'s sankey arm keeps only strictly positive measures, so a
chart handed **real rows** whose measure is all `0`, all `null`, all negative,
or unparseable built no links and returned a bare `<div>`. Measured in Chromium
against a populated control: the control drew 1 `<svg>` / 7 `<path>` /
26 descendants; each of those four tiles rendered `descendantCount: 1`,
`svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to one
another. No marks, no text, no `role` — a tile indistinguishable from a widget
that had crashed, which is the one distinction the file's other refusals exist
to make.

It now renders through the `ChartRefusal` shell those refusals already use —
same box, same `role="status"`, and a new `data-chart-error="no-positive-flow"`
— reading *"This chart has no flow to draw: no row's `<measure>` is above
zero."*

Two boundaries are deliberate and pinned:

- **No rows at all is untouched.** That is the empty-result question, answered
upstream in `ObjectChart` where the query outcome is known; a sentence about
what the rows contain would be false about a dataset with no rows in it.
- **One positive row among zeros still draws.** The refusal fires on an empty
link set, never on a thin one.

One code and one sentence for three causes (a genuinely all-zero flow, values a
flow cannot represent because they are negative, and measures `Number(…) || 0`
folds to zero): naming any single cause would be false for the other two, so
the copy names the predicate the filter actually applies, which is true for all
three. No recovery is promised. Every other chart family is byte-identical —
eight of the twelve tiles in the browser sweep hashed unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* Sankey — rows arrived, none of them is a positive number (objectui#7140).
*
* The sankey arm keeps only strictly positive measures
* (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a chart handed
* REAL rows whose measure is all `0`, all `null`, all negative, or unparseable
* built no links and returned a bare `<div className={className} />`.
*
* Measured in Chromium against a populated control before the fix, at
* `origin/main` e8e4c4df5: the control drew 1 `<svg>` / 7 `<path>` /
* 26 descendants; each of the four blank tiles rendered `descendantCount: 1`,
* `svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to
* one another. Nothing was on the page — no marks, no text, no `role` — so the
* tile was indistinguishable from a widget that had crashed, which is the one
* distinction every other refusal in that file exists to make.
*
* The two boundaries this pins are the ones that make the message TRUE rather
* than merely present:
*
* - **no rows at all still returns the bare div.** That is the empty-RESULT
* question (objectui#7130), answered upstream in `ObjectChart` where the
* query outcome is known. "No row's measure is above zero" would be a false
* sentence about a dataset with no rows in it.
* - **one positive row among zeros still DRAWS.** The refusal fires on an
* empty link set, never on a thin one; a sankey that can draw anything is
* never replaced by prose.
*/
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 480, height: 320 }),
};
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
render(
<AdvancedChartImpl
chartType="sankey"
xAxisKey="stage"
series={SERIES as any}
data={data as any}
/>,
);

const refusalOf = (container: HTMLElement) =>
container.querySelector('[data-chart-error="no-positive-flow"]');

/**
* The four row shapes that reach the empty link set. They are NOT the same
* situation — a genuinely all-zero flow, values a flow cannot represent because
* they are negative, and measures `Number(…) || 0` folds to zero — but the
* predicate the filter applies is the one thing true of all of them, so one
* message serves all four without saying anything false about any of them.
*/
const NO_POSITIVE_ROWS: Array<[string, Array<Record<string, unknown>>]> = [
['every measure is 0', [{ stage: 'Prospecting', amount: 0 }, { stage: 'Won', amount: 0 }]],
['every measure is null', [{ stage: 'Prospecting', amount: null }, { stage: 'Won', amount: null }]],
['every measure is negative', [{ stage: 'Refunds', amount: -40 }, { stage: 'Credits', amount: -12 }]],
['every measure is unparseable', [{ stage: 'A', amount: 'n/a' }, { stage: 'B', amount: 'n/a' }]],
];

describe('AdvancedChartImpl — sankey with no positive flow (objectui#7140)', () => {
it.each(NO_POSITIVE_ROWS)('says so instead of rendering nothing when %s', (_label, rows) => {
const { container } = renderSankey(rows);

const refusal = refusalOf(container);
expect(refusal, 'renders the explanatory placeholder').not.toBeNull();
// A refusal is a STATE, not an alert — the shell the other two refusals in
// this file render through.
expect(refusal?.getAttribute('role')).toBe('status');
// Names the measure it tested, so the author knows WHICH column was all
// zero rather than being told the chart is empty.
expect(refusal?.textContent).toContain('amount');
expect(refusal?.textContent).toContain('above zero');
// The old behaviour was a bare `<div>` with nothing in it. Anything that
// paints a plot here would be a sankey drawn from links that do not exist.
expect(container.querySelector('svg')).toBeNull();
});

it('leaves the no-rows case alone — that is the empty-result question, answered upstream', () => {
const { container } = renderSankey([]);

expect(refusalOf(container), 'no refusal without rows to refuse over').toBeNull();
expect(container.querySelector('svg')).toBeNull();
// Byte-for-byte what this arm returned before objectui#7140: an empty div,
// carrying only the className it was handed.
expect(container.textContent).toBe('');
});

it('still draws when ONE row is positive among zeros', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 0 },
{ stage: 'Proposal', amount: 7 },
{ stage: 'Won', amount: 0 },
]);

expect(refusalOf(container), 'a drawable sankey is never replaced by prose').toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('CONTROL — an all-positive sankey draws, and carries no refusal', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 40 },
{ stage: 'Proposal', amount: 25 },
{ stage: 'Won', amount: 12 },
]);

expect(refusalOf(container)).toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('does not fire for other chart families handed the same all-zero rows', () => {
// The guard lives inside the sankey arm and reads the sankey filter's own
// result, so it cannot reach a family that has no such filter. Pinned
// because a hoisted copy of the predicate is the obvious refactor and would
// blank four working charts: bar/pie/funnel/treemap all render an all-zero
// dataset today (measured in Chromium — axes, labels and legend).
for (const chartType of ['bar', 'pie', 'funnel', 'treemap'] as const) {
const { container } = render(
<AdvancedChartImpl
chartType={chartType}
xAxisKey="stage"
series={SERIES as any}
data={[{ stage: 'A', amount: 0 }, { stage: 'B', amount: 0 }] as any}
/>,
);
expect(refusalOf(container), `${chartType} must be untouched`).toBeNull();
cleanup();
}
});
});
40 changes: 39 additions & 1 deletion packages/plugin-charts/src/AdvancedChartImpl.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1048,7 +1048,45 @@ function AdvancedChartImplInner({
const nodes = [{ name: rootName }, ...rows.map((r) => ({ name: String(r?.[xAxisKey] ?? '') }))];
const links = rows.map((r, i) => ({ source: 0, target: i + 1, value: Number(r?.[dataKey]) || 0 }));
if (links.length === 0) {
return <div className={className} />;
// Rows ARRIVED and the filter above kept none of them, so there is no
// flow to draw. This used to return a bare `<div>` — objectui#7140.
//
// Measured in Chromium before it was changed, against a populated
// control that drew 1 `<svg>` / 7 `<path>`: the all-zero, all-null,
// all-negative and unparseable-measure tiles each rendered ONE element
// and nothing else (`descendantCount: 1`, `svg: 0`, `textContent: ''`),
// and their screenshots were byte-identical to each other. No marks, no
// text, no `role` — the one path in this file that put nothing at all on
// the page, and pixel-identical to a render that crashed. A reader could
// not tell a genuinely all-zero flow from a broken widget, which is the
// distinction every other refusal here exists to make.
//
// Gated on rows being present for the same reason `hasNoCategoryKey` and
// `hasNoPlottableSeries` are: handed NO rows the sentence below would be
// false — there is no row whose measure could be anything. That is the
// empty-RESULT question (objectui#7130), answered upstream in
// `ObjectChart` where the query outcome is known, so this arm leaves the
// no-rows case byte-for-byte as it was.
//
// ONE code and ONE sentence, for the reason `hasNoPlottableSeries`'
// docstring gives: three causes reach here — a genuinely all-zero flow,
// values a flow cannot represent because they are negative, and
// unparseable measures that `Number(…) || 0` folds to zero — and naming
// any ONE of them is a sentence that is false for the other two. The
// predicate the filter actually applies is true for all three, so the
// copy names THAT. No console warning either, unlike the two refusals
// below: those carry a diagnostic pair that does not fit on screen,
// whereas this message already names the key and the exact test it
// failed.
if (data.length === 0) {
return <div className={className} />;
}
return (
<ChartRefusal code="no-positive-flow" className={className}>
This chart has no flow to draw: no row&apos;s{' '}
<code className="font-mono">{dataKey}</code> is above zero.
</ChartRefusal>
);
}
return (
<ChartContainer config={config} className={className} {...containerProps}>
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .changeset/7140-sankey-no-positive-flow.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
---
'@object-ui/plugin-charts': minor
---

A sankey with no positive flow says so, instead of rendering an empty div
(objectui#7140).

`AdvancedChartImpl`'s sankey arm keeps only strictly positive measures, so a
chart handed **real rows** whose measure is all `0`, all `null`, all negative,
or unparseable built no links and returned a bare `<div>`. Measured in Chromium
against a populated control: the control drew 1 `<svg>` / 7 `<path>` /
26 descendants; each of those four tiles rendered `descendantCount: 1`,
`svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to one
another. No marks, no text, no `role` — a tile indistinguishable from a widget
that had crashed, which is the one distinction the file's other refusals exist
to make.

It now renders through the `ChartRefusal` shell those refusals already use —
same box, same `role="status"`, and a new `data-chart-error="no-positive-flow"`
— reading *"This chart has no flow to draw: no row's `<measure>` is above
zero."*

Two boundaries are deliberate and pinned:

- **No rows at all is untouched.** That is the empty-result question, answered
upstream in `ObjectChart` where the query outcome is known; a sentence about
what the rows contain would be false about a dataset with no rows in it.
- **One positive row among zeros still draws.** The refusal fires on an empty
link set, never on a thin one.

One code and one sentence for three causes (a genuinely all-zero flow, values a
flow cannot represent because they are negative, and measures `Number(…) || 0`
folds to zero): naming any single cause would be false for the other two, so
the copy names the predicate the filter actually applies, which is true for all
three. No recovery is promised. Every other chart family is byte-identical —
eight of the twelve tiles in the browser sweep hashed unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* Sankey — rows arrived, none of them is a positive number (objectui#7140).
*
* The sankey arm keeps only strictly positive measures
* (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a chart handed
* REAL rows whose measure is all `0`, all `null`, all negative, or unparseable
* built no links and returned a bare `<div className={className} />`.
*
* Measured in Chromium against a populated control before the fix, at
* `origin/main` e8e4c4df5: the control drew 1 `<svg>` / 7 `<path>` /
* 26 descendants; each of the four blank tiles rendered `descendantCount: 1`,
* `svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to
* one another. Nothing was on the page — no marks, no text, no `role` — so the
* tile was indistinguishable from a widget that had crashed, which is the one
* distinction every other refusal in that file exists to make.
*
* The two boundaries this pins are the ones that make the message TRUE rather
* than merely present:
*
* - **no rows at all still returns the bare div.** That is the empty-RESULT
* question (objectui#7130), answered upstream in `ObjectChart` where the
* query outcome is known. "No row's measure is above zero" would be a false
* sentence about a dataset with no rows in it.
* - **one positive row among zeros still DRAWS.** The refusal fires on an
* empty link set, never on a thin one; a sankey that can draw anything is
* never replaced by prose.
*/
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 480, height: 320 }),
};
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
render(
<AdvancedChartImpl
chartType="sankey"
xAxisKey="stage"
series={SERIES as any}
data={data as any}
/>,
);

const refusalOf = (container: HTMLElement) =>
container.querySelector('[data-chart-error="no-positive-flow"]');

/**
* The four row shapes that reach the empty link set. They are NOT the same
* situation — a genuinely all-zero flow, values a flow cannot represent because
* they are negative, and measures `Number(…) || 0` folds to zero — but the
* predicate the filter applies is the one thing true of all of them, so one
* message serves all four without saying anything false about any of them.
*/
const NO_POSITIVE_ROWS: Array<[string, Array<Record<string, unknown>>]> = [
['every measure is 0', [{ stage: 'Prospecting', amount: 0 }, { stage: 'Won', amount: 0 }]],
['every measure is null', [{ stage: 'Prospecting', amount: null }, { stage: 'Won', amount: null }]],
['every measure is negative', [{ stage: 'Refunds', amount: -40 }, { stage: 'Credits', amount: -12 }]],
['every measure is unparseable', [{ stage: 'A', amount: 'n/a' }, { stage: 'B', amount: 'n/a' }]],
];

describe('AdvancedChartImpl — sankey with no positive flow (objectui#7140)', () => {
it.each(NO_POSITIVE_ROWS)('says so instead of rendering nothing when %s', (_label, rows) => {
const { container } = renderSankey(rows);

const refusal = refusalOf(container);
expect(refusal, 'renders the explanatory placeholder').not.toBeNull();
// A refusal is a STATE, not an alert — the shell the other two refusals in
// this file render through.
expect(refusal?.getAttribute('role')).toBe('status');
// Names the measure it tested, so the author knows WHICH column was all
// zero rather than being told the chart is empty.
expect(refusal?.textContent).toContain('amount');
expect(refusal?.textContent).toContain('above zero');
// The old behaviour was a bare `<div>` with nothing in it. Anything that
// paints a plot here would be a sankey drawn from links that do not exist.
expect(container.querySelector('svg')).toBeNull();
});

it('leaves the no-rows case alone — that is the empty-result question, answered upstream', () => {
const { container } = renderSankey([]);

expect(refusalOf(container), 'no refusal without rows to refuse over').toBeNull();
expect(container.querySelector('svg')).toBeNull();
// Byte-for-byte what this arm returned before objectui#7140: an empty div,
// carrying only the className it was handed.
expect(container.textContent).toBe('');
});

it('still draws when ONE row is positive among zeros', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 0 },
{ stage: 'Proposal', amount: 7 },
{ stage: 'Won', amount: 0 },
]);

expect(refusalOf(container), 'a drawable sankey is never replaced by prose').toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('CONTROL — an all-positive sankey draws, and carries no refusal', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 40 },
{ stage: 'Proposal', amount: 25 },
{ stage: 'Won', amount: 12 },
]);

expect(refusalOf(container)).toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('does not fire for other chart families handed the same all-zero rows', () => {
// The guard lives inside the sankey arm and reads the sankey filter's own
// result, so it cannot reach a family that has no such filter. Pinned
// because a hoisted copy of the predicate is the obvious refactor and would
// blank four working charts: bar/pie/funnel/treemap all render an all-zero
// dataset today (measured in Chromium — axes, labels and legend).
for (const chartType of ['bar', 'pie', 'funnel', 'treemap'] as const) {
const { container } = render(
<AdvancedChartImpl
chartType={chartType}
xAxisKey="stage"
series={SERIES as any}
data={[{ stage: 'A', amount: 0 }, { stage: 'B', amount: 0 }] as any}
/>,
);
expect(refusalOf(container), `${chartType} must be untouched`).toBeNull();
cleanup();
}
});
});
40 changes: 39 additions & 1 deletion packages/plugin-charts/src/AdvancedChartImpl.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1048,7 +1048,45 @@ function AdvancedChartImplInner({
const nodes = [{ name: rootName }, ...rows.map((r) => ({ name: String(r?.[xAxisKey] ?? '') }))];
const links = rows.map((r, i) => ({ source: 0, target: i + 1, value: Number(r?.[dataKey]) || 0 }));
if (links.length === 0) {
return <div className={className} />;
// Rows ARRIVED and the filter above kept none of them, so there is no
// flow to draw. This used to return a bare `<div>` — objectui#7140.
//
// Measured in Chromium before it was changed, against a populated
// control that drew 1 `<svg>` / 7 `<path>`: the all-zero, all-null,
// all-negative and unparseable-measure tiles each rendered ONE element
// and nothing else (`descendantCount: 1`, `svg: 0`, `textContent: ''`),
// and their screenshots were byte-identical to each other. No marks, no
// text, no `role` — the one path in this file that put nothing at all on
// the page, and pixel-identical to a render that crashed. A reader could
// not tell a genuinely all-zero flow from a broken widget, which is the
// distinction every other refusal here exists to make.
//
// Gated on rows being present for the same reason `hasNoCategoryKey` and
// `hasNoPlottableSeries` are: handed NO rows the sentence below would be
// false — there is no row whose measure could be anything. That is the
// empty-RESULT question (objectui#7130), answered upstream in
// `ObjectChart` where the query outcome is known, so this arm leaves the
// no-rows case byte-for-byte as it was.
//
// ONE code and ONE sentence, for the reason `hasNoPlottableSeries`'
// docstring gives: three causes reach here — a genuinely all-zero flow,
// values a flow cannot represent because they are negative, and
// unparseable measures that `Number(…) || 0` folds to zero — and naming
// any ONE of them is a sentence that is false for the other two. The
// predicate the filter actually applies is true for all three, so the
// copy names THAT. No console warning either, unlike the two refusals
// below: those carry a diagnostic pair that does not fit on screen,
// whereas this message already names the key and the exact test it
// failed.
if (data.length === 0) {
return <div className={className} />;
}
return (
<ChartRefusal code="no-positive-flow" className={className}>
This chart has no flow to draw: no row&apos;s{' '}
<code className="font-mono">{dataKey}</code> is above zero.
</ChartRefusal>
);
}
return (
<ChartContainer config={config} className={className} {...containerProps}>
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .changeset/7140-sankey-no-positive-flow.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
---
'@object-ui/plugin-charts': minor
---

A sankey with no positive flow says so, instead of rendering an empty div
(objectui#7140).

`AdvancedChartImpl`'s sankey arm keeps only strictly positive measures, so a
chart handed **real rows** whose measure is all `0`, all `null`, all negative,
or unparseable built no links and returned a bare `<div>`. Measured in Chromium
against a populated control: the control drew 1 `<svg>` / 7 `<path>` /
26 descendants; each of those four tiles rendered `descendantCount: 1`,
`svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to one
another. No marks, no text, no `role` — a tile indistinguishable from a widget
that had crashed, which is the one distinction the file's other refusals exist
to make.

It now renders through the `ChartRefusal` shell those refusals already use —
same box, same `role="status"`, and a new `data-chart-error="no-positive-flow"`
— reading *"This chart has no flow to draw: no row's `<measure>` is above
zero."*

Two boundaries are deliberate and pinned:

- **No rows at all is untouched.** That is the empty-result question, answered
upstream in `ObjectChart` where the query outcome is known; a sentence about
what the rows contain would be false about a dataset with no rows in it.
- **One positive row among zeros still draws.** The refusal fires on an empty
link set, never on a thin one.

One code and one sentence for three causes (a genuinely all-zero flow, values a
flow cannot represent because they are negative, and measures `Number(…) || 0`
folds to zero): naming any single cause would be false for the other two, so
the copy names the predicate the filter actually applies, which is true for all
three. No recovery is promised. Every other chart family is byte-identical —
eight of the twelve tiles in the browser sweep hashed unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* Sankey — rows arrived, none of them is a positive number (objectui#7140).
*
* The sankey arm keeps only strictly positive measures
* (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a chart handed
* REAL rows whose measure is all `0`, all `null`, all negative, or unparseable
* built no links and returned a bare `<div className={className} />`.
*
* Measured in Chromium against a populated control before the fix, at
* `origin/main` e8e4c4df5: the control drew 1 `<svg>` / 7 `<path>` /
* 26 descendants; each of the four blank tiles rendered `descendantCount: 1`,
* `svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to
* one another. Nothing was on the page — no marks, no text, no `role` — so the
* tile was indistinguishable from a widget that had crashed, which is the one
* distinction every other refusal in that file exists to make.
*
* The two boundaries this pins are the ones that make the message TRUE rather
* than merely present:
*
* - **no rows at all still returns the bare div.** That is the empty-RESULT
* question (objectui#7130), answered upstream in `ObjectChart` where the
* query outcome is known. "No row's measure is above zero" would be a false
* sentence about a dataset with no rows in it.
* - **one positive row among zeros still DRAWS.** The refusal fires on an
* empty link set, never on a thin one; a sankey that can draw anything is
* never replaced by prose.
*/
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 480, height: 320 }),
};
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
render(
<AdvancedChartImpl
chartType="sankey"
xAxisKey="stage"
series={SERIES as any}
data={data as any}
/>,
);

const refusalOf = (container: HTMLElement) =>
container.querySelector('[data-chart-error="no-positive-flow"]');

/**
* The four row shapes that reach the empty link set. They are NOT the same
* situation — a genuinely all-zero flow, values a flow cannot represent because
* they are negative, and measures `Number(…) || 0` folds to zero — but the
* predicate the filter applies is the one thing true of all of them, so one
* message serves all four without saying anything false about any of them.
*/
const NO_POSITIVE_ROWS: Array<[string, Array<Record<string, unknown>>]> = [
['every measure is 0', [{ stage: 'Prospecting', amount: 0 }, { stage: 'Won', amount: 0 }]],
['every measure is null', [{ stage: 'Prospecting', amount: null }, { stage: 'Won', amount: null }]],
['every measure is negative', [{ stage: 'Refunds', amount: -40 }, { stage: 'Credits', amount: -12 }]],
['every measure is unparseable', [{ stage: 'A', amount: 'n/a' }, { stage: 'B', amount: 'n/a' }]],
];

describe('AdvancedChartImpl — sankey with no positive flow (objectui#7140)', () => {
it.each(NO_POSITIVE_ROWS)('says so instead of rendering nothing when %s', (_label, rows) => {
const { container } = renderSankey(rows);

const refusal = refusalOf(container);
expect(refusal, 'renders the explanatory placeholder').not.toBeNull();
// A refusal is a STATE, not an alert — the shell the other two refusals in
// this file render through.
expect(refusal?.getAttribute('role')).toBe('status');
// Names the measure it tested, so the author knows WHICH column was all
// zero rather than being told the chart is empty.
expect(refusal?.textContent).toContain('amount');
expect(refusal?.textContent).toContain('above zero');
// The old behaviour was a bare `<div>` with nothing in it. Anything that
// paints a plot here would be a sankey drawn from links that do not exist.
expect(container.querySelector('svg')).toBeNull();
});

it('leaves the no-rows case alone — that is the empty-result question, answered upstream', () => {
const { container } = renderSankey([]);

expect(refusalOf(container), 'no refusal without rows to refuse over').toBeNull();
expect(container.querySelector('svg')).toBeNull();
// Byte-for-byte what this arm returned before objectui#7140: an empty div,
// carrying only the className it was handed.
expect(container.textContent).toBe('');
});

it('still draws when ONE row is positive among zeros', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 0 },
{ stage: 'Proposal', amount: 7 },
{ stage: 'Won', amount: 0 },
]);

expect(refusalOf(container), 'a drawable sankey is never replaced by prose').toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('CONTROL — an all-positive sankey draws, and carries no refusal', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 40 },
{ stage: 'Proposal', amount: 25 },
{ stage: 'Won', amount: 12 },
]);

expect(refusalOf(container)).toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('does not fire for other chart families handed the same all-zero rows', () => {
// The guard lives inside the sankey arm and reads the sankey filter's own
// result, so it cannot reach a family that has no such filter. Pinned
// because a hoisted copy of the predicate is the obvious refactor and would
// blank four working charts: bar/pie/funnel/treemap all render an all-zero
// dataset today (measured in Chromium — axes, labels and legend).
for (const chartType of ['bar', 'pie', 'funnel', 'treemap'] as const) {
const { container } = render(
<AdvancedChartImpl
chartType={chartType}
xAxisKey="stage"
series={SERIES as any}
data={[{ stage: 'A', amount: 0 }, { stage: 'B', amount: 0 }] as any}
/>,
);
expect(refusalOf(container), `${chartType} must be untouched`).toBeNull();
cleanup();
}
});
});
40 changes: 39 additions & 1 deletion packages/plugin-charts/src/AdvancedChartImpl.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1048,7 +1048,45 @@ function AdvancedChartImplInner({
const nodes = [{ name: rootName }, ...rows.map((r) => ({ name: String(r?.[xAxisKey] ?? '') }))];
const links = rows.map((r, i) => ({ source: 0, target: i + 1, value: Number(r?.[dataKey]) || 0 }));
if (links.length === 0) {
return <div className={className} />;
// Rows ARRIVED and the filter above kept none of them, so there is no
// flow to draw. This used to return a bare `<div>` — objectui#7140.
//
// Measured in Chromium before it was changed, against a populated
// control that drew 1 `<svg>` / 7 `<path>`: the all-zero, all-null,
// all-negative and unparseable-measure tiles each rendered ONE element
// and nothing else (`descendantCount: 1`, `svg: 0`, `textContent: ''`),
// and their screenshots were byte-identical to each other. No marks, no
// text, no `role` — the one path in this file that put nothing at all on
// the page, and pixel-identical to a render that crashed. A reader could
// not tell a genuinely all-zero flow from a broken widget, which is the
// distinction every other refusal here exists to make.
//
// Gated on rows being present for the same reason `hasNoCategoryKey` and
// `hasNoPlottableSeries` are: handed NO rows the sentence below would be
// false — there is no row whose measure could be anything. That is the
// empty-RESULT question (objectui#7130), answered upstream in
// `ObjectChart` where the query outcome is known, so this arm leaves the
// no-rows case byte-for-byte as it was.
//
// ONE code and ONE sentence, for the reason `hasNoPlottableSeries`'
// docstring gives: three causes reach here — a genuinely all-zero flow,
// values a flow cannot represent because they are negative, and
// unparseable measures that `Number(…) || 0` folds to zero — and naming
// any ONE of them is a sentence that is false for the other two. The
// predicate the filter actually applies is true for all three, so the
// copy names THAT. No console warning either, unlike the two refusals
// below: those carry a diagnostic pair that does not fit on screen,
// whereas this message already names the key and the exact test it
// failed.
if (data.length === 0) {
return <div className={className} />;
}
return (
<ChartRefusal code="no-positive-flow" className={className}>
This chart has no flow to draw: no row&apos;s{' '}
<code className="font-mono">{dataKey}</code> is above zero.
</ChartRefusal>
);
}
return (
<ChartContainer config={config} className={className} {...containerProps}>
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .changeset/7140-sankey-no-positive-flow.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
---
'@object-ui/plugin-charts': minor
---

A sankey with no positive flow says so, instead of rendering an empty div
(objectui#7140).

`AdvancedChartImpl`'s sankey arm keeps only strictly positive measures, so a
chart handed **real rows** whose measure is all `0`, all `null`, all negative,
or unparseable built no links and returned a bare `<div>`. Measured in Chromium
against a populated control: the control drew 1 `<svg>` / 7 `<path>` /
26 descendants; each of those four tiles rendered `descendantCount: 1`,
`svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to one
another. No marks, no text, no `role` — a tile indistinguishable from a widget
that had crashed, which is the one distinction the file's other refusals exist
to make.

It now renders through the `ChartRefusal` shell those refusals already use —
same box, same `role="status"`, and a new `data-chart-error="no-positive-flow"`
— reading *"This chart has no flow to draw: no row's `<measure>` is above
zero."*

Two boundaries are deliberate and pinned:

- **No rows at all is untouched.** That is the empty-result question, answered
upstream in `ObjectChart` where the query outcome is known; a sentence about
what the rows contain would be false about a dataset with no rows in it.
- **One positive row among zeros still draws.** The refusal fires on an empty
link set, never on a thin one.

One code and one sentence for three causes (a genuinely all-zero flow, values a
flow cannot represent because they are negative, and measures `Number(…) || 0`
folds to zero): naming any single cause would be false for the other two, so
the copy names the predicate the filter actually applies, which is true for all
three. No recovery is promised. Every other chart family is byte-identical —
eight of the twelve tiles in the browser sweep hashed unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* Sankey — rows arrived, none of them is a positive number (objectui#7140).
*
* The sankey arm keeps only strictly positive measures
* (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a chart handed
* REAL rows whose measure is all `0`, all `null`, all negative, or unparseable
* built no links and returned a bare `<div className={className} />`.
*
* Measured in Chromium against a populated control before the fix, at
* `origin/main` e8e4c4df5: the control drew 1 `<svg>` / 7 `<path>` /
* 26 descendants; each of the four blank tiles rendered `descendantCount: 1`,
* `svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to
* one another. Nothing was on the page — no marks, no text, no `role` — so the
* tile was indistinguishable from a widget that had crashed, which is the one
* distinction every other refusal in that file exists to make.
*
* The two boundaries this pins are the ones that make the message TRUE rather
* than merely present:
*
* - **no rows at all still returns the bare div.** That is the empty-RESULT
* question (objectui#7130), answered upstream in `ObjectChart` where the
* query outcome is known. "No row's measure is above zero" would be a false
* sentence about a dataset with no rows in it.
* - **one positive row among zeros still DRAWS.** The refusal fires on an
* empty link set, never on a thin one; a sankey that can draw anything is
* never replaced by prose.
*/
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 480, height: 320 }),
};
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
render(
<AdvancedChartImpl
chartType="sankey"
xAxisKey="stage"
series={SERIES as any}
data={data as any}
/>,
);

const refusalOf = (container: HTMLElement) =>
container.querySelector('[data-chart-error="no-positive-flow"]');

/**
* The four row shapes that reach the empty link set. They are NOT the same
* situation — a genuinely all-zero flow, values a flow cannot represent because
* they are negative, and measures `Number(…) || 0` folds to zero — but the
* predicate the filter applies is the one thing true of all of them, so one
* message serves all four without saying anything false about any of them.
*/
const NO_POSITIVE_ROWS: Array<[string, Array<Record<string, unknown>>]> = [
['every measure is 0', [{ stage: 'Prospecting', amount: 0 }, { stage: 'Won', amount: 0 }]],
['every measure is null', [{ stage: 'Prospecting', amount: null }, { stage: 'Won', amount: null }]],
['every measure is negative', [{ stage: 'Refunds', amount: -40 }, { stage: 'Credits', amount: -12 }]],
['every measure is unparseable', [{ stage: 'A', amount: 'n/a' }, { stage: 'B', amount: 'n/a' }]],
];

describe('AdvancedChartImpl — sankey with no positive flow (objectui#7140)', () => {
it.each(NO_POSITIVE_ROWS)('says so instead of rendering nothing when %s', (_label, rows) => {
const { container } = renderSankey(rows);

const refusal = refusalOf(container);
expect(refusal, 'renders the explanatory placeholder').not.toBeNull();
// A refusal is a STATE, not an alert — the shell the other two refusals in
// this file render through.
expect(refusal?.getAttribute('role')).toBe('status');
// Names the measure it tested, so the author knows WHICH column was all
// zero rather than being told the chart is empty.
expect(refusal?.textContent).toContain('amount');
expect(refusal?.textContent).toContain('above zero');
// The old behaviour was a bare `<div>` with nothing in it. Anything that
// paints a plot here would be a sankey drawn from links that do not exist.
expect(container.querySelector('svg')).toBeNull();
});

it('leaves the no-rows case alone — that is the empty-result question, answered upstream', () => {
const { container } = renderSankey([]);

expect(refusalOf(container), 'no refusal without rows to refuse over').toBeNull();
expect(container.querySelector('svg')).toBeNull();
// Byte-for-byte what this arm returned before objectui#7140: an empty div,
// carrying only the className it was handed.
expect(container.textContent).toBe('');
});

it('still draws when ONE row is positive among zeros', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 0 },
{ stage: 'Proposal', amount: 7 },
{ stage: 'Won', amount: 0 },
]);

expect(refusalOf(container), 'a drawable sankey is never replaced by prose').toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('CONTROL — an all-positive sankey draws, and carries no refusal', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 40 },
{ stage: 'Proposal', amount: 25 },
{ stage: 'Won', amount: 12 },
]);

expect(refusalOf(container)).toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('does not fire for other chart families handed the same all-zero rows', () => {
// The guard lives inside the sankey arm and reads the sankey filter's own
// result, so it cannot reach a family that has no such filter. Pinned
// because a hoisted copy of the predicate is the obvious refactor and would
// blank four working charts: bar/pie/funnel/treemap all render an all-zero
// dataset today (measured in Chromium — axes, labels and legend).
for (const chartType of ['bar', 'pie', 'funnel', 'treemap'] as const) {
const { container } = render(
<AdvancedChartImpl
chartType={chartType}
xAxisKey="stage"
series={SERIES as any}
data={[{ stage: 'A', amount: 0 }, { stage: 'B', amount: 0 }] as any}
/>,
);
expect(refusalOf(container), `${chartType} must be untouched`).toBeNull();
cleanup();
}
});
});
40 changes: 39 additions & 1 deletion packages/plugin-charts/src/AdvancedChartImpl.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1048,7 +1048,45 @@ function AdvancedChartImplInner({
const nodes = [{ name: rootName }, ...rows.map((r) => ({ name: String(r?.[xAxisKey] ?? '') }))];
const links = rows.map((r, i) => ({ source: 0, target: i + 1, value: Number(r?.[dataKey]) || 0 }));
if (links.length === 0) {
return <div className={className} />;
// Rows ARRIVED and the filter above kept none of them, so there is no
// flow to draw. This used to return a bare `<div>` — objectui#7140.
//
// Measured in Chromium before it was changed, against a populated
// control that drew 1 `<svg>` / 7 `<path>`: the all-zero, all-null,
// all-negative and unparseable-measure tiles each rendered ONE element
// and nothing else (`descendantCount: 1`, `svg: 0`, `textContent: ''`),
// and their screenshots were byte-identical to each other. No marks, no
// text, no `role` — the one path in this file that put nothing at all on
// the page, and pixel-identical to a render that crashed. A reader could
// not tell a genuinely all-zero flow from a broken widget, which is the
// distinction every other refusal here exists to make.
//
// Gated on rows being present for the same reason `hasNoCategoryKey` and
// `hasNoPlottableSeries` are: handed NO rows the sentence below would be
// false — there is no row whose measure could be anything. That is the
// empty-RESULT question (objectui#7130), answered upstream in
// `ObjectChart` where the query outcome is known, so this arm leaves the
// no-rows case byte-for-byte as it was.
//
// ONE code and ONE sentence, for the reason `hasNoPlottableSeries`'
// docstring gives: three causes reach here — a genuinely all-zero flow,
// values a flow cannot represent because they are negative, and
// unparseable measures that `Number(…) || 0` folds to zero — and naming
// any ONE of them is a sentence that is false for the other two. The
// predicate the filter actually applies is true for all three, so the
// copy names THAT. No console warning either, unlike the two refusals
// below: those carry a diagnostic pair that does not fit on screen,
// whereas this message already names the key and the exact test it
// failed.
if (data.length === 0) {
return <div className={className} />;
}
return (
<ChartRefusal code="no-positive-flow" className={className}>
This chart has no flow to draw: no row&apos;s{' '}
<code className="font-mono">{dataKey}</code> is above zero.
</ChartRefusal>
);
}
return (
<ChartContainer config={config} className={className} {...containerProps}>
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .changeset/7140-sankey-no-positive-flow.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
---
'@object-ui/plugin-charts': minor
---

A sankey with no positive flow says so, instead of rendering an empty div
(objectui#7140).

`AdvancedChartImpl`'s sankey arm keeps only strictly positive measures, so a
chart handed **real rows** whose measure is all `0`, all `null`, all negative,
or unparseable built no links and returned a bare `<div>`. Measured in Chromium
against a populated control: the control drew 1 `<svg>` / 7 `<path>` /
26 descendants; each of those four tiles rendered `descendantCount: 1`,
`svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to one
another. No marks, no text, no `role` — a tile indistinguishable from a widget
that had crashed, which is the one distinction the file's other refusals exist
to make.

It now renders through the `ChartRefusal` shell those refusals already use —
same box, same `role="status"`, and a new `data-chart-error="no-positive-flow"`
— reading *"This chart has no flow to draw: no row's `<measure>` is above
zero."*

Two boundaries are deliberate and pinned:

- **No rows at all is untouched.** That is the empty-result question, answered
upstream in `ObjectChart` where the query outcome is known; a sentence about
what the rows contain would be false about a dataset with no rows in it.
- **One positive row among zeros still draws.** The refusal fires on an empty
link set, never on a thin one.

One code and one sentence for three causes (a genuinely all-zero flow, values a
flow cannot represent because they are negative, and measures `Number(…) || 0`
folds to zero): naming any single cause would be false for the other two, so
the copy names the predicate the filter actually applies, which is true for all
three. No recovery is promised. Every other chart family is byte-identical —
eight of the twelve tiles in the browser sweep hashed unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* Sankey — rows arrived, none of them is a positive number (objectui#7140).
*
* The sankey arm keeps only strictly positive measures
* (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a chart handed
* REAL rows whose measure is all `0`, all `null`, all negative, or unparseable
* built no links and returned a bare `<div className={className} />`.
*
* Measured in Chromium against a populated control before the fix, at
* `origin/main` e8e4c4df5: the control drew 1 `<svg>` / 7 `<path>` /
* 26 descendants; each of the four blank tiles rendered `descendantCount: 1`,
* `svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to
* one another. Nothing was on the page — no marks, no text, no `role` — so the
* tile was indistinguishable from a widget that had crashed, which is the one
* distinction every other refusal in that file exists to make.
*
* The two boundaries this pins are the ones that make the message TRUE rather
* than merely present:
*
* - **no rows at all still returns the bare div.** That is the empty-RESULT
* question (objectui#7130), answered upstream in `ObjectChart` where the
* query outcome is known. "No row's measure is above zero" would be a false
* sentence about a dataset with no rows in it.
* - **one positive row among zeros still DRAWS.** The refusal fires on an
* empty link set, never on a thin one; a sankey that can draw anything is
* never replaced by prose.
*/
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 480, height: 320 }),
};
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
render(
<AdvancedChartImpl
chartType="sankey"
xAxisKey="stage"
series={SERIES as any}
data={data as any}
/>,
);

const refusalOf = (container: HTMLElement) =>
container.querySelector('[data-chart-error="no-positive-flow"]');

/**
* The four row shapes that reach the empty link set. They are NOT the same
* situation — a genuinely all-zero flow, values a flow cannot represent because
* they are negative, and measures `Number(…) || 0` folds to zero — but the
* predicate the filter applies is the one thing true of all of them, so one
* message serves all four without saying anything false about any of them.
*/
const NO_POSITIVE_ROWS: Array<[string, Array<Record<string, unknown>>]> = [
['every measure is 0', [{ stage: 'Prospecting', amount: 0 }, { stage: 'Won', amount: 0 }]],
['every measure is null', [{ stage: 'Prospecting', amount: null }, { stage: 'Won', amount: null }]],
['every measure is negative', [{ stage: 'Refunds', amount: -40 }, { stage: 'Credits', amount: -12 }]],
['every measure is unparseable', [{ stage: 'A', amount: 'n/a' }, { stage: 'B', amount: 'n/a' }]],
];

describe('AdvancedChartImpl — sankey with no positive flow (objectui#7140)', () => {
it.each(NO_POSITIVE_ROWS)('says so instead of rendering nothing when %s', (_label, rows) => {
const { container } = renderSankey(rows);

const refusal = refusalOf(container);
expect(refusal, 'renders the explanatory placeholder').not.toBeNull();
// A refusal is a STATE, not an alert — the shell the other two refusals in
// this file render through.
expect(refusal?.getAttribute('role')).toBe('status');
// Names the measure it tested, so the author knows WHICH column was all
// zero rather than being told the chart is empty.
expect(refusal?.textContent).toContain('amount');
expect(refusal?.textContent).toContain('above zero');
// The old behaviour was a bare `<div>` with nothing in it. Anything that
// paints a plot here would be a sankey drawn from links that do not exist.
expect(container.querySelector('svg')).toBeNull();
});

it('leaves the no-rows case alone — that is the empty-result question, answered upstream', () => {
const { container } = renderSankey([]);

expect(refusalOf(container), 'no refusal without rows to refuse over').toBeNull();
expect(container.querySelector('svg')).toBeNull();
// Byte-for-byte what this arm returned before objectui#7140: an empty div,
// carrying only the className it was handed.
expect(container.textContent).toBe('');
});

it('still draws when ONE row is positive among zeros', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 0 },
{ stage: 'Proposal', amount: 7 },
{ stage: 'Won', amount: 0 },
]);

expect(refusalOf(container), 'a drawable sankey is never replaced by prose').toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('CONTROL — an all-positive sankey draws, and carries no refusal', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 40 },
{ stage: 'Proposal', amount: 25 },
{ stage: 'Won', amount: 12 },
]);

expect(refusalOf(container)).toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('does not fire for other chart families handed the same all-zero rows', () => {
// The guard lives inside the sankey arm and reads the sankey filter's own
// result, so it cannot reach a family that has no such filter. Pinned
// because a hoisted copy of the predicate is the obvious refactor and would
// blank four working charts: bar/pie/funnel/treemap all render an all-zero
// dataset today (measured in Chromium — axes, labels and legend).
for (const chartType of ['bar', 'pie', 'funnel', 'treemap'] as const) {
const { container } = render(
<AdvancedChartImpl
chartType={chartType}
xAxisKey="stage"
series={SERIES as any}
data={[{ stage: 'A', amount: 0 }, { stage: 'B', amount: 0 }] as any}
/>,
);
expect(refusalOf(container), `${chartType} must be untouched`).toBeNull();
cleanup();
}
});
});
40 changes: 39 additions & 1 deletion packages/plugin-charts/src/AdvancedChartImpl.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1048,7 +1048,45 @@ function AdvancedChartImplInner({
const nodes = [{ name: rootName }, ...rows.map((r) => ({ name: String(r?.[xAxisKey] ?? '') }))];
const links = rows.map((r, i) => ({ source: 0, target: i + 1, value: Number(r?.[dataKey]) || 0 }));
if (links.length === 0) {
return <div className={className} />;
// Rows ARRIVED and the filter above kept none of them, so there is no
// flow to draw. This used to return a bare `<div>` — objectui#7140.
//
// Measured in Chromium before it was changed, against a populated
// control that drew 1 `<svg>` / 7 `<path>`: the all-zero, all-null,
// all-negative and unparseable-measure tiles each rendered ONE element
// and nothing else (`descendantCount: 1`, `svg: 0`, `textContent: ''`),
// and their screenshots were byte-identical to each other. No marks, no
// text, no `role` — the one path in this file that put nothing at all on
// the page, and pixel-identical to a render that crashed. A reader could
// not tell a genuinely all-zero flow from a broken widget, which is the
// distinction every other refusal here exists to make.
//
// Gated on rows being present for the same reason `hasNoCategoryKey` and
// `hasNoPlottableSeries` are: handed NO rows the sentence below would be
// false — there is no row whose measure could be anything. That is the
// empty-RESULT question (objectui#7130), answered upstream in
// `ObjectChart` where the query outcome is known, so this arm leaves the
// no-rows case byte-for-byte as it was.
//
// ONE code and ONE sentence, for the reason `hasNoPlottableSeries`'
// docstring gives: three causes reach here — a genuinely all-zero flow,
// values a flow cannot represent because they are negative, and
// unparseable measures that `Number(…) || 0` folds to zero — and naming
// any ONE of them is a sentence that is false for the other two. The
// predicate the filter actually applies is true for all three, so the
// copy names THAT. No console warning either, unlike the two refusals
// below: those carry a diagnostic pair that does not fit on screen,
// whereas this message already names the key and the exact test it
// failed.
if (data.length === 0) {
return <div className={className} />;
}
return (
<ChartRefusal code="no-positive-flow" className={className}>
This chart has no flow to draw: no row&apos;s{' '}
<code className="font-mono">{dataKey}</code> is above zero.
</ChartRefusal>
);
}
return (
<ChartContainer config={config} className={className} {...containerProps}>
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .changeset/7140-sankey-no-positive-flow.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
---
'@object-ui/plugin-charts': minor
---

A sankey with no positive flow says so, instead of rendering an empty div
(objectui#7140).

`AdvancedChartImpl`'s sankey arm keeps only strictly positive measures, so a
chart handed **real rows** whose measure is all `0`, all `null`, all negative,
or unparseable built no links and returned a bare `<div>`. Measured in Chromium
against a populated control: the control drew 1 `<svg>` / 7 `<path>` /
26 descendants; each of those four tiles rendered `descendantCount: 1`,
`svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to one
another. No marks, no text, no `role` — a tile indistinguishable from a widget
that had crashed, which is the one distinction the file's other refusals exist
to make.

It now renders through the `ChartRefusal` shell those refusals already use —
same box, same `role="status"`, and a new `data-chart-error="no-positive-flow"`
— reading *"This chart has no flow to draw: no row's `<measure>` is above
zero."*

Two boundaries are deliberate and pinned:

- **No rows at all is untouched.** That is the empty-result question, answered
upstream in `ObjectChart` where the query outcome is known; a sentence about
what the rows contain would be false about a dataset with no rows in it.
- **One positive row among zeros still draws.** The refusal fires on an empty
link set, never on a thin one.

One code and one sentence for three causes (a genuinely all-zero flow, values a
flow cannot represent because they are negative, and measures `Number(…) || 0`
folds to zero): naming any single cause would be false for the other two, so
the copy names the predicate the filter actually applies, which is true for all
three. No recovery is promised. Every other chart family is byte-identical —
eight of the twelve tiles in the browser sweep hashed unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* Sankey — rows arrived, none of them is a positive number (objectui#7140).
*
* The sankey arm keeps only strictly positive measures
* (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a chart handed
* REAL rows whose measure is all `0`, all `null`, all negative, or unparseable
* built no links and returned a bare `<div className={className} />`.
*
* Measured in Chromium against a populated control before the fix, at
* `origin/main` e8e4c4df5: the control drew 1 `<svg>` / 7 `<path>` /
* 26 descendants; each of the four blank tiles rendered `descendantCount: 1`,
* `svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to
* one another. Nothing was on the page — no marks, no text, no `role` — so the
* tile was indistinguishable from a widget that had crashed, which is the one
* distinction every other refusal in that file exists to make.
*
* The two boundaries this pins are the ones that make the message TRUE rather
* than merely present:
*
* - **no rows at all still returns the bare div.** That is the empty-RESULT
* question (objectui#7130), answered upstream in `ObjectChart` where the
* query outcome is known. "No row's measure is above zero" would be a false
* sentence about a dataset with no rows in it.
* - **one positive row among zeros still DRAWS.** The refusal fires on an
* empty link set, never on a thin one; a sankey that can draw anything is
* never replaced by prose.
*/
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 480, height: 320 }),
};
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
render(
<AdvancedChartImpl
chartType="sankey"
xAxisKey="stage"
series={SERIES as any}
data={data as any}
/>,
);

const refusalOf = (container: HTMLElement) =>
container.querySelector('[data-chart-error="no-positive-flow"]');

/**
* The four row shapes that reach the empty link set. They are NOT the same
* situation — a genuinely all-zero flow, values a flow cannot represent because
* they are negative, and measures `Number(…) || 0` folds to zero — but the
* predicate the filter applies is the one thing true of all of them, so one
* message serves all four without saying anything false about any of them.
*/
const NO_POSITIVE_ROWS: Array<[string, Array<Record<string, unknown>>]> = [
['every measure is 0', [{ stage: 'Prospecting', amount: 0 }, { stage: 'Won', amount: 0 }]],
['every measure is null', [{ stage: 'Prospecting', amount: null }, { stage: 'Won', amount: null }]],
['every measure is negative', [{ stage: 'Refunds', amount: -40 }, { stage: 'Credits', amount: -12 }]],
['every measure is unparseable', [{ stage: 'A', amount: 'n/a' }, { stage: 'B', amount: 'n/a' }]],
];

describe('AdvancedChartImpl — sankey with no positive flow (objectui#7140)', () => {
it.each(NO_POSITIVE_ROWS)('says so instead of rendering nothing when %s', (_label, rows) => {
const { container } = renderSankey(rows);

const refusal = refusalOf(container);
expect(refusal, 'renders the explanatory placeholder').not.toBeNull();
// A refusal is a STATE, not an alert — the shell the other two refusals in
// this file render through.
expect(refusal?.getAttribute('role')).toBe('status');
// Names the measure it tested, so the author knows WHICH column was all
// zero rather than being told the chart is empty.
expect(refusal?.textContent).toContain('amount');
expect(refusal?.textContent).toContain('above zero');
// The old behaviour was a bare `<div>` with nothing in it. Anything that
// paints a plot here would be a sankey drawn from links that do not exist.
expect(container.querySelector('svg')).toBeNull();
});

it('leaves the no-rows case alone — that is the empty-result question, answered upstream', () => {
const { container } = renderSankey([]);

expect(refusalOf(container), 'no refusal without rows to refuse over').toBeNull();
expect(container.querySelector('svg')).toBeNull();
// Byte-for-byte what this arm returned before objectui#7140: an empty div,
// carrying only the className it was handed.
expect(container.textContent).toBe('');
});

it('still draws when ONE row is positive among zeros', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 0 },
{ stage: 'Proposal', amount: 7 },
{ stage: 'Won', amount: 0 },
]);

expect(refusalOf(container), 'a drawable sankey is never replaced by prose').toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('CONTROL — an all-positive sankey draws, and carries no refusal', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 40 },
{ stage: 'Proposal', amount: 25 },
{ stage: 'Won', amount: 12 },
]);

expect(refusalOf(container)).toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('does not fire for other chart families handed the same all-zero rows', () => {
// The guard lives inside the sankey arm and reads the sankey filter's own
// result, so it cannot reach a family that has no such filter. Pinned
// because a hoisted copy of the predicate is the obvious refactor and would
// blank four working charts: bar/pie/funnel/treemap all render an all-zero
// dataset today (measured in Chromium — axes, labels and legend).
for (const chartType of ['bar', 'pie', 'funnel', 'treemap'] as const) {
const { container } = render(
<AdvancedChartImpl
chartType={chartType}
xAxisKey="stage"
series={SERIES as any}
data={[{ stage: 'A', amount: 0 }, { stage: 'B', amount: 0 }] as any}
/>,
);
expect(refusalOf(container), `${chartType} must be untouched`).toBeNull();
cleanup();
}
});
});
40 changes: 39 additions & 1 deletion packages/plugin-charts/src/AdvancedChartImpl.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1048,7 +1048,45 @@ function AdvancedChartImplInner({
const nodes = [{ name: rootName }, ...rows.map((r) => ({ name: String(r?.[xAxisKey] ?? '') }))];
const links = rows.map((r, i) => ({ source: 0, target: i + 1, value: Number(r?.[dataKey]) || 0 }));
if (links.length === 0) {
return <div className={className} />;
// Rows ARRIVED and the filter above kept none of them, so there is no
// flow to draw. This used to return a bare `<div>` — objectui#7140.
//
// Measured in Chromium before it was changed, against a populated
// control that drew 1 `<svg>` / 7 `<path>`: the all-zero, all-null,
// all-negative and unparseable-measure tiles each rendered ONE element
// and nothing else (`descendantCount: 1`, `svg: 0`, `textContent: ''`),
// and their screenshots were byte-identical to each other. No marks, no
// text, no `role` — the one path in this file that put nothing at all on
// the page, and pixel-identical to a render that crashed. A reader could
// not tell a genuinely all-zero flow from a broken widget, which is the
// distinction every other refusal here exists to make.
//
// Gated on rows being present for the same reason `hasNoCategoryKey` and
// `hasNoPlottableSeries` are: handed NO rows the sentence below would be
// false — there is no row whose measure could be anything. That is the
// empty-RESULT question (objectui#7130), answered upstream in
// `ObjectChart` where the query outcome is known, so this arm leaves the
// no-rows case byte-for-byte as it was.
//
// ONE code and ONE sentence, for the reason `hasNoPlottableSeries`'
// docstring gives: three causes reach here — a genuinely all-zero flow,
// values a flow cannot represent because they are negative, and
// unparseable measures that `Number(…) || 0` folds to zero — and naming
// any ONE of them is a sentence that is false for the other two. The
// predicate the filter actually applies is true for all three, so the
// copy names THAT. No console warning either, unlike the two refusals
// below: those carry a diagnostic pair that does not fit on screen,
// whereas this message already names the key and the exact test it
// failed.
if (data.length === 0) {
return <div className={className} />;
}
return (
<ChartRefusal code="no-positive-flow" className={className}>
This chart has no flow to draw: no row&apos;s{' '}
<code className="font-mono">{dataKey}</code> is above zero.
</ChartRefusal>
);
}
return (
<ChartContainer config={config} className={className} {...containerProps}>
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions .changeset/7140-sankey-no-positive-flow.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
---
'@object-ui/plugin-charts': minor
---

A sankey with no positive flow says so, instead of rendering an empty div
(objectui#7140).

`AdvancedChartImpl`'s sankey arm keeps only strictly positive measures, so a
chart handed **real rows** whose measure is all `0`, all `null`, all negative,
or unparseable built no links and returned a bare `<div>`. Measured in Chromium
against a populated control: the control drew 1 `<svg>` / 7 `<path>` /
26 descendants; each of those four tiles rendered `descendantCount: 1`,
`svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to one
another. No marks, no text, no `role` — a tile indistinguishable from a widget
that had crashed, which is the one distinction the file's other refusals exist
to make.

It now renders through the `ChartRefusal` shell those refusals already use —
same box, same `role="status"`, and a new `data-chart-error="no-positive-flow"`
— reading *"This chart has no flow to draw: no row's `<measure>` is above
zero."*

Two boundaries are deliberate and pinned:

- **No rows at all is untouched.** That is the empty-result question, answered
upstream in `ObjectChart` where the query outcome is known; a sentence about
what the rows contain would be false about a dataset with no rows in it.
- **One positive row among zeros still draws.** The refusal fires on an empty
link set, never on a thin one.

One code and one sentence for three causes (a genuinely all-zero flow, values a
flow cannot represent because they are negative, and measures `Number(…) || 0`
folds to zero): naming any single cause would be false for the other two, so
the copy names the predicate the filter actually applies, which is true for all
three. No recovery is promised. Every other chart family is byte-identical —
eight of the twelve tiles in the browser sweep hashed unchanged.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* Sankey — rows arrived, none of them is a positive number (objectui#7140).
*
* The sankey arm keeps only strictly positive measures
* (`data.filter((r) => (Number(r?.[dataKey]) || 0) > 0)`), so a chart handed
* REAL rows whose measure is all `0`, all `null`, all negative, or unparseable
* built no links and returned a bare `<div className={className} />`.
*
* Measured in Chromium against a populated control before the fix, at
* `origin/main` e8e4c4df5: the control drew 1 `<svg>` / 7 `<path>` /
* 26 descendants; each of the four blank tiles rendered `descendantCount: 1`,
* `svgCount: 0`, `textContent: ''`, and their screenshots hashed identical to
* one another. Nothing was on the page — no marks, no text, no `role` — so the
* tile was indistinguishable from a widget that had crashed, which is the one
* distinction every other refusal in that file exists to make.
*
* The two boundaries this pins are the ones that make the message TRUE rather
* than merely present:
*
* - **no rows at all still returns the bare div.** That is the empty-RESULT
* question (objectui#7130), answered upstream in `ObjectChart` where the
* query outcome is known. "No row's measure is above zero" would be a false
* sentence about a dataset with no rows in it.
* - **one positive row among zeros still DRAWS.** The refusal fires on an
* empty link set, never on a thin one; a sankey that can draw anything is
* never replaced by prose.
*/
import React from 'react';
import { describe, it, expect, afterEach, vi } from 'vitest';
import { render, cleanup } from '@testing-library/react';

vi.mock('recharts', async () => {
const actual = await vi.importActual<any>('recharts');
return {
...actual,
ResponsiveContainer: ({ children }: any) =>
React.cloneElement(children, { width: 480, height: 320 }),
};
});

import AdvancedChartImpl from './AdvancedChartImpl';

afterEach(cleanup);

const SERIES = [{ dataKey: 'amount', label: 'Amount' }];

const renderSankey = (data: Array<Record<string, unknown>>) =>
render(
<AdvancedChartImpl
chartType="sankey"
xAxisKey="stage"
series={SERIES as any}
data={data as any}
/>,
);

const refusalOf = (container: HTMLElement) =>
container.querySelector('[data-chart-error="no-positive-flow"]');

/**
* The four row shapes that reach the empty link set. They are NOT the same
* situation — a genuinely all-zero flow, values a flow cannot represent because
* they are negative, and measures `Number(…) || 0` folds to zero — but the
* predicate the filter applies is the one thing true of all of them, so one
* message serves all four without saying anything false about any of them.
*/
const NO_POSITIVE_ROWS: Array<[string, Array<Record<string, unknown>>]> = [
['every measure is 0', [{ stage: 'Prospecting', amount: 0 }, { stage: 'Won', amount: 0 }]],
['every measure is null', [{ stage: 'Prospecting', amount: null }, { stage: 'Won', amount: null }]],
['every measure is negative', [{ stage: 'Refunds', amount: -40 }, { stage: 'Credits', amount: -12 }]],
['every measure is unparseable', [{ stage: 'A', amount: 'n/a' }, { stage: 'B', amount: 'n/a' }]],
];

describe('AdvancedChartImpl — sankey with no positive flow (objectui#7140)', () => {
it.each(NO_POSITIVE_ROWS)('says so instead of rendering nothing when %s', (_label, rows) => {
const { container } = renderSankey(rows);

const refusal = refusalOf(container);
expect(refusal, 'renders the explanatory placeholder').not.toBeNull();
// A refusal is a STATE, not an alert — the shell the other two refusals in
// this file render through.
expect(refusal?.getAttribute('role')).toBe('status');
// Names the measure it tested, so the author knows WHICH column was all
// zero rather than being told the chart is empty.
expect(refusal?.textContent).toContain('amount');
expect(refusal?.textContent).toContain('above zero');
// The old behaviour was a bare `<div>` with nothing in it. Anything that
// paints a plot here would be a sankey drawn from links that do not exist.
expect(container.querySelector('svg')).toBeNull();
});

it('leaves the no-rows case alone — that is the empty-result question, answered upstream', () => {
const { container } = renderSankey([]);

expect(refusalOf(container), 'no refusal without rows to refuse over').toBeNull();
expect(container.querySelector('svg')).toBeNull();
// Byte-for-byte what this arm returned before objectui#7140: an empty div,
// carrying only the className it was handed.
expect(container.textContent).toBe('');
});

it('still draws when ONE row is positive among zeros', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 0 },
{ stage: 'Proposal', amount: 7 },
{ stage: 'Won', amount: 0 },
]);

expect(refusalOf(container), 'a drawable sankey is never replaced by prose').toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('CONTROL — an all-positive sankey draws, and carries no refusal', () => {
const { container } = renderSankey([
{ stage: 'Prospecting', amount: 40 },
{ stage: 'Proposal', amount: 25 },
{ stage: 'Won', amount: 12 },
]);

expect(refusalOf(container)).toBeNull();
expect(container.querySelector('svg')).not.toBeNull();
});

it('does not fire for other chart families handed the same all-zero rows', () => {
// The guard lives inside the sankey arm and reads the sankey filter's own
// result, so it cannot reach a family that has no such filter. Pinned
// because a hoisted copy of the predicate is the obvious refactor and would
// blank four working charts: bar/pie/funnel/treemap all render an all-zero
// dataset today (measured in Chromium — axes, labels and legend).
for (const chartType of ['bar', 'pie', 'funnel', 'treemap'] as const) {
const { container } = render(
<AdvancedChartImpl
chartType={chartType}
xAxisKey="stage"
series={SERIES as any}
data={[{ stage: 'A', amount: 0 }, { stage: 'B', amount: 0 }] as any}
/>,
);
expect(refusalOf(container), `${chartType} must be untouched`).toBeNull();
cleanup();
}
});
});
40 changes: 39 additions & 1 deletion packages/plugin-charts/src/AdvancedChartImpl.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -1048,7 +1048,45 @@ function AdvancedChartImplInner({
const nodes = [{ name: rootName }, ...rows.map((r) => ({ name: String(r?.[xAxisKey] ?? '') }))];
const links = rows.map((r, i) => ({ source: 0, target: i + 1, value: Number(r?.[dataKey]) || 0 }));
if (links.length === 0) {
return <div className={className} />;
// Rows ARRIVED and the filter above kept none of them, so there is no
// flow to draw. This used to return a bare `<div>` — objectui#7140.
//
// Measured in Chromium before it was changed, against a populated
// control that drew 1 `<svg>` / 7 `<path>`: the all-zero, all-null,
// all-negative and unparseable-measure tiles each rendered ONE element
// and nothing else (`descendantCount: 1`, `svg: 0`, `textContent: ''`),
// and their screenshots were byte-identical to each other. No marks, no
// text, no `role` — the one path in this file that put nothing at all on
// the page, and pixel-identical to a render that crashed. A reader could
// not tell a genuinely all-zero flow from a broken widget, which is the
// distinction every other refusal here exists to make.
//
// Gated on rows being present for the same reason `hasNoCategoryKey` and
// `hasNoPlottableSeries` are: handed NO rows the sentence below would be
// false — there is no row whose measure could be anything. That is the
// empty-RESULT question (objectui#7130), answered upstream in
// `ObjectChart` where the query outcome is known, so this arm leaves the
// no-rows case byte-for-byte as it was.
//
// ONE code and ONE sentence, for the reason `hasNoPlottableSeries`'
// docstring gives: three causes reach here — a genuinely all-zero flow,
// values a flow cannot represent because they are negative, and
// unparseable measures that `Number(…) || 0` folds to zero — and naming
// any ONE of them is a sentence that is false for the other two. The
// predicate the filter actually applies is true for all three, so the
// copy names THAT. No console warning either, unlike the two refusals
// below: those carry a diagnostic pair that does not fit on screen,
// whereas this message already names the key and the exact test it
// failed.
if (data.length === 0) {
return <div className={className} />;
}
return (
<ChartRefusal code="no-positive-flow" className={className}>
This chart has no flow to draw: no row&apos;s{' '}
<code className="font-mono">{dataKey}</code> is above zero.
</ChartRefusal>
);
}
return (
<ChartContainer config={config} className={className} {...containerProps}>
Expand Down
Loading