Skip to content

Commit

Permalink
fix(chart): new Chart() do not set default view key (#4925)
Browse files Browse the repository at this point in the history
  • Loading branch information
@pearmini
pearmini committed Apr 28, 2023
1 parent f3632f5 commit 99538a6
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 44 deletions.
58 changes: 27 additions & 31 deletions __tests__/unit/api/chart.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Canvas, DisplayObject } from '@antv/g';
import { Chart, createLibrary, VIEW_CLASS_NAME } from '../../../src';
import { Canvas } from '@antv/g';
import { Chart, createLibrary } from '../../../src';
import { G2_CHART_KEY } from '../../../src/api/chart';
import {
View,
Expand Down Expand Up @@ -41,36 +41,36 @@ import {
} from '../../../src/api/mark/mark';

describe('Chart', () => {
it('Chart() should have expected defaults', () => {
it('Chart() should have expected defaults.', () => {
const chart = new Chart({ theme: 'classic' });
expect(chart.type).toBe('view');
expect(chart.parentNode).toBeNull();
expect(chart.value).toEqual({ key: G2_CHART_KEY, theme: 'classic' });
expect(chart.value).toEqual({ theme: 'classic', key: undefined });
expect(chart['_container'].nodeName).toBe('DIV');
});

it('Chart({...}) should support HTML container', () => {
it('Chart({...}) should support HTML container.', () => {
const container = document.createElement('div');
const chart = new Chart({ theme: 'classic', container });
expect(chart['_container']).toBe(container);
});

it('Chart({...}) should support id container', () => {
it('Chart({...}) should support id container.', () => {
const div = document.createElement('div');
div.setAttribute('id', 'root');
document.body.appendChild(div);
const chart = new Chart({ theme: 'classic', container: 'root' });
expect(chart['_container']).toBe(div);
});

it('Chart({...}) should support undefined container', () => {
it('Chart({...}) should support undefined container.', () => {
const chart = new Chart({ theme: 'classic' });
const defaultContainer = chart['_container'];
expect(defaultContainer.nodeName).toBe('DIV');
expect(defaultContainer.parentNode).toBeNull();
});

it('Chart({...}) should override default value', () => {
it('Chart({...}) should override default value.', () => {
const chart = new Chart({
theme: 'classic',
data: [1, 2, 3],
Expand All @@ -83,13 +83,13 @@ describe('Chart', () => {
});
});

it('chart.getContainer() should return container', () => {
it('chart.getContainer() should return container.', () => {
const container = document.createElement('div');
const chart = new Chart({ theme: 'classic', container });
expect(chart.getContainer()).toBe(container);
});

it('chart.[attr](...) should specify options by API', () => {
it('chart.[attr](...) should specify options by API.', () => {
const chart = new Chart({ theme: 'classic' });
chart
.data([1, 2, 3])
Expand All @@ -114,7 +114,7 @@ describe('Chart', () => {
});
});

it('chart.nodeName() should return expected node ', () => {
it('chart.nodeName() should return expected node.', () => {
const chart = new Chart({ theme: 'classic' });
expect(chart.interval()).toBeInstanceOf(Interval);
expect(chart.rect()).toBeInstanceOf(Rect);
Expand Down Expand Up @@ -174,14 +174,14 @@ describe('Chart', () => {
]);
});

it('chart.container() should use last node as root node', () => {
it('chart.container() should use last node as root node.', () => {
const chart = new Chart({ theme: 'classic' });
chart.view();
chart.spaceLayer();
expect(chart.spaceLayer()).toBeInstanceOf(SpaceLayer);
});

it('chart.container() should set layout options for root node', () => {
it('chart.container() should set layout options for root node.', () => {
const chart = new Chart({
theme: 'classic',
width: 100,
Expand Down Expand Up @@ -228,7 +228,7 @@ describe('Chart', () => {
});
});

it('chart.container() should return expected container', () => {
it('chart.container() should return expected container.', () => {
const chart = new Chart({ theme: 'classic' });
expect(chart.view()).toBeInstanceOf(View);
expect(chart.options()).toEqual({ type: 'view', theme: 'classic' });
Expand All @@ -249,36 +249,34 @@ describe('Chart', () => {
});
});

it('chart.options() should return view tree', () => {
it('chart.options() should return view tree.', () => {
const chart = new Chart({ theme: 'classic' });
chart.interval();
chart.point();
expect(chart.options()).toEqual({
type: 'view',
key: G2_CHART_KEY,
theme: 'classic',
children: [{ type: 'interval' }, { type: 'point' }],
});
});

it('chart.options(options) should handle date object', () => {
it('chart.options(options) should handle date object.', () => {
const chart = new Chart({ theme: 'classic' });
const date = new Date();
chart.cell().data([{ date }]);
expect(chart.options()).toEqual({
type: 'view',
key: G2_CHART_KEY,
theme: 'classic',
children: [{ type: 'cell', data: [{ date }] }],
});
});

it('chart.options(options) should return this chart instance', () => {
it('chart.options(options) should return this chart instance.', () => {
const chart = new Chart({ theme: 'classic' });
expect(chart.options({ width: 800 })).toBe(chart);
});

it('chart.title() should set title options', () => {
it('chart.title() should set title options.', () => {
const chart = new Chart({ theme: 'classic' });

chart.title('This is a title.');
Expand All @@ -294,30 +292,28 @@ describe('Chart', () => {
});
});

it('chart.nodeName() should build view tree', () => {
it('chart.nodeName() should build view tree.', () => {
const chart = new Chart({ theme: 'classic' });
chart.interval();
chart.point();
expect(chart.options()).toEqual({
type: 'view',
key: G2_CHART_KEY,
theme: 'classic',
children: [{ type: 'interval' }, { type: 'point' }],
});
});

it('chart.call(chart => chart.nodeName()) should build view tree', () => {
it('chart.call(chart => chart.nodeName()) should build view tree.', () => {
const chart = new Chart({ theme: 'classic' });
chart.call((chart) => chart.interval()).call((chart) => chart.point());
expect(chart.options()).toEqual({
type: 'view',
key: G2_CHART_KEY,
theme: 'classic',
children: [{ type: 'interval' }, { type: 'point' }],
});
});

it('chart.nodeName() should build nested view tree', () => {
it('chart.nodeName() should build nested view tree.', () => {
const chart = new Chart({ theme: 'classic' });
chart
.spaceFlex()
Expand All @@ -338,7 +334,7 @@ describe('Chart', () => {
});
});

it('chart.getContext() should return rendering context', () => {
it('chart.getContext() should return rendering context.', () => {
const chart = new Chart({ theme: 'classic' });

chart.data([
Expand All @@ -362,7 +358,7 @@ describe('Chart', () => {
expect(context.canvas).toBeInstanceOf(Canvas);
});

it('chart.render() should return promise', (done) => {
it('chart.render() should return promise.', (done) => {
const chart = new Chart({ theme: 'classic' });

chart.data([
Expand Down Expand Up @@ -509,12 +505,12 @@ describe('Chart', () => {
await chart.render();

const context = chart.getContext();
const view = context.views?.find((v) => v.key === chart.attr('key'));
const view = context.views?.find((v) => v.key === chart['_key']);

expect(chart.getView()).toEqual(view);
expect(chart.getCoordinate()).toEqual(view?.coordinate);
expect(chart.getTheme()).toEqual(view?.theme);
expect(chart.getGroup().id).toEqual(chart.attr('key'));
expect(chart.getGroup().id).toEqual(chart['_key']);
expect(chart.getScale()).toEqual(view?.scale);
expect(chart.getScaleByChannel('color')).toEqual(view?.scale.color);
expect(chart.getScaleByChannel('shape')).not.toBeDefined();
Expand Down Expand Up @@ -544,7 +540,7 @@ describe('Chart', () => {
.encode('color', 'genre');

await chart.render();
expect(chart.getGroup().id).toEqual(chart.attr('key'));
expect(chart.getGroup().id).toEqual(chart['_key']);
chart.destroy();
expect(chart.getGroup()).toEqual(null);
});
Expand All @@ -566,7 +562,7 @@ describe('Chart', () => {
.encode('color', 'genre');

await chart.render();
expect(chart.getGroup().id).toEqual(chart.attr('key'));
expect(chart.getGroup().id).toEqual(chart['_key']);
chart.clear();
expect(chart.getGroup()).toEqual(null);
});
Expand Down
27 changes: 16 additions & 11 deletions src/api/chart.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,12 @@ export class Chart extends View<ChartOptions> {
private _height: number;

constructor(options: ChartOptions) {
const { container, canvas, key = G2_CHART_KEY, ...rest } = options || {};
const { container, canvas, ...rest } = options || {};
super(rest, 'view');
this.attr('key', key);
this._container = normalizeContainer(container);
this._emitter = new EventEmitter();
this._context = { library, emitter: this._emitter, canvas };
this.bindAutoFit();
this._bindAutoFit();
}

render(): Promise<Chart> {
Expand All @@ -206,11 +205,17 @@ export class Chart extends View<ChartOptions> {

return new Promise((resolve, reject) => {
try {
const { width, height } = sizeOf(this.options(), this._container);
const options = this.options();
const { key = G2_CHART_KEY } = options;
const { width, height } = sizeOf(options, this._container);

// Update actual size and key.
this._width = width;
this._height = height;
this._key = key;

render(
{ ...this.options(), width, height },
{ key: this._key, ...options, width, height },
this._context,
() => resolve(this),
reject,
Expand Down Expand Up @@ -287,7 +292,7 @@ export class Chart extends View<ChartOptions> {
destroy() {
const options = this.options();
this.emit(ChartEvent.BEFORE_DESTROY);
this.unbindAutoFit();
this._unbindAutoFit();
this._options = {};
destroy(options, this._context, true);
removeContainer(this._container);
Expand Down Expand Up @@ -320,23 +325,23 @@ export class Chart extends View<ChartOptions> {
return finished;
}

private onResize = debounce(() => {
private _onResize = debounce(() => {
this.forceFit();
}, 300);

private bindAutoFit() {
private _bindAutoFit() {
const options = this.options();
const { autoFit } = options;
if (autoFit) {
window.addEventListener('resize', this.onResize);
window.addEventListener('resize', this._onResize);
}
}

private unbindAutoFit() {
private _unbindAutoFit() {
const options = this.options();
const { autoFit } = options;
if (autoFit) {
window.removeEventListener('resize', this.onResize);
window.removeEventListener('resize', this._onResize);
}
}
}
6 changes: 4 additions & 2 deletions src/api/composition/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ export class CompositionNode<
ParentValue extends Record<string, any> = Record<string, any>,
ChildValue extends Record<string, any> = Record<string, any>,
> extends Node<Value, ParentValue, ChildValue> {
protected _key: string;

/**
* Change current node data and its children data.
*/
Expand All @@ -31,7 +33,7 @@ export class CompositionNode<
const chart = this.getRoot();
const { views } = chart.getContext();
if (!views?.length) return undefined;
return views.find((view) => view.key === this.attr('key'));
return views.find((view) => view.key === this._key);
}

getScale(): Record<string, Scale> {
Expand All @@ -53,7 +55,7 @@ export class CompositionNode<
}

getGroup(): DisplayObject {
const key = this.attr('key');
const key = this._key;
if (!key) return undefined;
const chart = this.getRoot();
const chartGroup = chart.getContext().canvas.getRoot();
Expand Down

0 comments on commit 99538a6

Please sign in to comment.