From 6e4b7d316e855399af7419b5826beb4b2329219e Mon Sep 17 00:00:00 2001 From: bqxbqx Date: Thu, 5 Dec 2024 11:33:46 +0800 Subject: [PATCH 1/3] feat(label): implement label color inheritance from dependent elements Implement the ability for labels to inherit colors from their associated graphical elements. This allows labels to automatically match the color of the elements they are describing. - Add style inheritance in label creation - Pass element style information to label callbacks - Add test case to verify the implementation --- __tests__/integration/issue-5474.spec.ts | 35 + .../snapshots/bugfix/issue5474.svg | 1602 +++++++++++++++++ __tests__/plots/bugfix/index.ts | 1 + __tests__/plots/bugfix/issue-5474.ts | 39 + src/runtime/plot.ts | 21 +- 5 files changed, 1695 insertions(+), 3 deletions(-) create mode 100644 __tests__/integration/issue-5474.spec.ts create mode 100644 __tests__/integration/snapshots/bugfix/issue5474.svg create mode 100644 __tests__/plots/bugfix/issue-5474.ts diff --git a/__tests__/integration/issue-5474.spec.ts b/__tests__/integration/issue-5474.spec.ts new file mode 100644 index 0000000000..d51366e32c --- /dev/null +++ b/__tests__/integration/issue-5474.spec.ts @@ -0,0 +1,35 @@ +import { issue5474 as render } from '../plots/bugfix/issue-5474'; +import { createNodeGCanvas } from './utils/createNodeGCanvas'; +import { sleep } from './utils/sleep'; +import './utils/useSnapshotMatchers'; + +describe('issue5474', () => { + const canvas = createNodeGCanvas(800, 500); + + it('issue5474.render() should render chart with labels matching element colors', async () => { + const { chart } = render({ + canvas, + container: document.createElement('div'), + }); + + await chart.render(); + await sleep(20); + + const labels = canvas.document.getElementsByClassName('label'); + expect(labels.length).toBeGreaterThan(0); + + labels.forEach((label) => { + expect(label.style.fill).toBe( + // @ts-ignore + label.__data__.dependentElement.attributes.fill, + ); + }); + + const dir = `${__dirname}/snapshots/bugfix`; + await expect(canvas).toMatchDOMSnapshot(dir, render.name); + }); + + afterAll(() => { + canvas?.destroy(); + }); +}); diff --git a/__tests__/integration/snapshots/bugfix/issue5474.svg b/__tests__/integration/snapshots/bugfix/issue5474.svg new file mode 100644 index 0000000000..9c7a411610 --- /dev/null +++ b/__tests__/integration/snapshots/bugfix/issue5474.svg @@ -0,0 +1,1602 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Sports + + + + + + + + + + + + + + + + + + + + Strategy + + + + + + + + + + + + + + + + + + + + Action + + + + + + + + + + + + + + + + + + + + Shooter + + + + + + + + + + + + + + + + + + + + Other + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Sports + + + + + + + Strategy + + + + + + + Action + + + + + + + Shooter + + + + + + + Other + + + + + + + + + genre + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + 0 + + + + + + + 50 + + + + + + + 100 + + + + + + + 150 + + + + + + + 200 + + + + + + + 250 + + + + + + + 300 + + + + + + + 350 + + + + + + + + + sold + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Sports + + + + + + + + + + + + + Strategy + + + + + + + + + + + + + Action + + + + + + + + + + + + + Shooter + + + + + + + + + + + + + Other + + + + + + + + + + + + \ No newline at end of file diff --git a/__tests__/plots/bugfix/index.ts b/__tests__/plots/bugfix/index.ts index 8e2caa72e2..5fb5c75b2f 100644 --- a/__tests__/plots/bugfix/index.ts +++ b/__tests__/plots/bugfix/index.ts @@ -2,3 +2,4 @@ export { issue6396 } from './issue-6396'; export { issue6399 } from './issue-6399'; export { issueChart2719 } from './issue-chart-2719'; export { issue6020 } from './issue-6020'; +export { issue5474 } from './issue-5474'; diff --git a/__tests__/plots/bugfix/issue-5474.ts b/__tests__/plots/bugfix/issue-5474.ts new file mode 100644 index 0000000000..3a1b278661 --- /dev/null +++ b/__tests__/plots/bugfix/issue-5474.ts @@ -0,0 +1,39 @@ +import { Chart } from '../../../src'; + +export function issue5474(context) { + const { container, canvas, callback } = context; + + const chart = new Chart({ + container: container, + autoFit: true, + insetRight: 10, + canvas, + }); + + if (callback) { + callback(chart); + } else { + chart + .interval() + .data([ + { genre: 'Sports', sold: 0 }, + { genre: 'Strategy', sold: 115 }, + { genre: 'Action', sold: 120 }, + { genre: 'Shooter', sold: 350 }, + { genre: 'Other', sold: 150 }, + ]) + .encode('x', 'genre') + .encode('y', 'sold') + .encode('color', 'genre') + .label({ + text: 'genre', + fill: (_, i, array, d) => { + return d.style.fill; + }, + }); + } + + chart.render(); + + return { chart }; +} diff --git a/src/runtime/plot.ts b/src/runtime/plot.ts index a5ee20e6be..4f161aa3dc 100644 --- a/src/runtime/plot.ts +++ b/src/runtime/plot.ts @@ -1197,7 +1197,9 @@ function plotLabel( return elements.flatMap((e) => { const L = getLabels(options, i, e); L.forEach((l) => { - labelShapeFunction.set(l, shapeFunction); + labelShapeFunction.set(l, (data) => + shapeFunction({ ...data, elementStyle: e.style }), + ); labelDescriptor.set(l, labelOption); }); return L; @@ -1365,11 +1367,24 @@ function createLabelShapeFunction( style: abstractStyle, render, selector, + elementStyle, ...abstractOptions } = options; + const visualOptions = mapObject( { ...abstractOptions, ...abstractStyle } as Record, - (d) => valueOf(d, datum, index, abstractData, { channel }), + (d) => + valueOf(d, datum, index, abstractData, { + channel: { ...channel }, + style: new Proxy(elementStyle, { + get: (target, prop) => { + if (typeof prop === 'string') { + return target.getPropertyValue(prop); + } + return undefined; + }, + }), + }), ); const { shape = defaultLabelShape, text, ...style } = visualOptions; const f = typeof formatter === 'string' ? format(formatter) : formatter; @@ -1393,7 +1408,7 @@ function valueOf( datum: Record, i: number, data: Record, - options: { channel: Record }, + options: { channel: Record; style?: Record }, ) { if (typeof value === 'function') return value(datum, i, data, options); if (typeof value !== 'string') return value; From ee9c83790516a74078a23e30c6f3d40ca483e692 Mon Sep 17 00:00:00 2001 From: bqxbqx Date: Thu, 5 Dec 2024 17:08:52 +0800 Subject: [PATCH 2/3] refactor(label): improve style callback compatibility and clarity - Remove Proxy usage in label style callback to prevent browser compatibility issues - Rename style parameter to elementStyle for better semantic clarity - Update documentation to clarify elementStyle parameter usage in label callbacks --- __tests__/plots/bugfix/issue-5474.ts | 4 +--- site/docs/spec/label/overview.zh.md | 2 +- src/runtime/plot.ts | 13 +++---------- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/__tests__/plots/bugfix/issue-5474.ts b/__tests__/plots/bugfix/issue-5474.ts index 3a1b278661..70d505fa35 100644 --- a/__tests__/plots/bugfix/issue-5474.ts +++ b/__tests__/plots/bugfix/issue-5474.ts @@ -27,9 +27,7 @@ export function issue5474(context) { .encode('color', 'genre') .label({ text: 'genre', - fill: (_, i, array, d) => { - return d.style.fill; - }, + fill: (_, i, array, d) => d.elementStyle.fill, }); } diff --git a/site/docs/spec/label/overview.zh.md b/site/docs/spec/label/overview.zh.md index c7408b5bd0..e327ef3c85 100644 --- a/site/docs/spec/label/overview.zh.md +++ b/site/docs/spec/label/overview.zh.md @@ -127,7 +127,7 @@ chart d, i, data, - { channel }, // 聚合图形的样式 + { channel, elementStyle }, // 聚合图形的样式 & label依赖元素的样式 ) => (channel.y[i] < 11700 ? '#E49361' : '#4787F7'), ); ``` diff --git a/src/runtime/plot.ts b/src/runtime/plot.ts index 4f161aa3dc..05eae06e7a 100644 --- a/src/runtime/plot.ts +++ b/src/runtime/plot.ts @@ -1198,7 +1198,7 @@ function plotLabel( const L = getLabels(options, i, e); L.forEach((l) => { labelShapeFunction.set(l, (data) => - shapeFunction({ ...data, elementStyle: e.style }), + shapeFunction({ ...data, elementStyle: e.attributes }), ); labelDescriptor.set(l, labelOption); }); @@ -1376,14 +1376,7 @@ function createLabelShapeFunction( (d) => valueOf(d, datum, index, abstractData, { channel: { ...channel }, - style: new Proxy(elementStyle, { - get: (target, prop) => { - if (typeof prop === 'string') { - return target.getPropertyValue(prop); - } - return undefined; - }, - }), + elementStyle: elementStyle, }), ); const { shape = defaultLabelShape, text, ...style } = visualOptions; @@ -1408,7 +1401,7 @@ function valueOf( datum: Record, i: number, data: Record, - options: { channel: Record; style?: Record }, + options: { channel: Record; elementStyle?: Record }, ) { if (typeof value === 'function') return value(datum, i, data, options); if (typeof value !== 'string') return value; From 565c519447042e101745b6be1408787a2a58959f Mon Sep 17 00:00:00 2001 From: bqxbqx Date: Thu, 5 Dec 2024 17:42:01 +0800 Subject: [PATCH 3/3] refactor: simplify object property using shorthand notation --- src/runtime/plot.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/runtime/plot.ts b/src/runtime/plot.ts index 05eae06e7a..ae1725b8c5 100644 --- a/src/runtime/plot.ts +++ b/src/runtime/plot.ts @@ -1376,7 +1376,7 @@ function createLabelShapeFunction( (d) => valueOf(d, datum, index, abstractData, { channel: { ...channel }, - elementStyle: elementStyle, + elementStyle, }), ); const { shape = defaultLabelShape, text, ...style } = visualOptions;