Skip to content

Commit

Permalink
fix(callbacks): Fix triggering in lazy rendering
Browse files Browse the repository at this point in the history
- Correct onafterinit callback to not be triggered when chart isn't fully rendered.
- Fix .resize() to not rendere when the chart isn't fully rendered

Fix #1254
  • Loading branch information
netil authored Feb 27, 2020
1 parent 7090fa9 commit 3e73fdf
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 6 deletions.
66 changes: 66 additions & 0 deletions spec/internals/bb-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ describe("Interface & initialization", () => {
});

describe("check for lazy rendering", () => {
const spy = {};
const args = {
data: {
columns: [
Expand All @@ -362,6 +363,16 @@ describe("Interface & initialization", () => {
}
};

["afterinit", "rendered", "resize", "resized"].forEach(v => {
args[`on${v}`] = spy[v] = sinon.spy();
});

afterEach(() => {
for (let x in spy) {
spy[x].resetHistory();
}
});

it("check lazy rendering & mutation observer: style attribute", done => {
const el = document.body.querySelector("#chart");

Expand All @@ -372,11 +383,18 @@ describe("Interface & initialization", () => {

expect(el.innerHTML).to.be.empty;

for (let x in spy) {
expect(spy[x].called).to.be.false;
}

el.style.display = "block";

setTimeout(() => {
expect(el.innerHTML).to.be.not.empty;
el.style.display = "";

expect(spy.afterinit.called).to.be.true;
expect(spy.rendered.called).to.be.true;
done();
}, 500);
});
Expand All @@ -391,14 +409,56 @@ describe("Interface & initialization", () => {

expect(el.innerHTML).to.be.empty;

for (let x in spy) {
expect(spy[x].called).to.be.false;
}

el.classList.remove("hide");

setTimeout(() => {
expect(el.innerHTML).to.be.not.empty;
expect(spy.afterinit.called).to.be.true;
expect(spy.rendered.called).to.be.true;
done();
}, 500);
});

it("check lazy rendering on callbacks", done => {
const el = document.body.querySelector("#chart");

// hide to lazy render
el.style.display = "none";

chart = util.generate(args);

expect(el.innerHTML).to.be.empty;

// onresize, resized shouldn't be called on resize
chart.resize({width: 500});

for (let x in spy) {
expect(spy[x].called).to.be.false;
}

el.style.display = "block";

setTimeout(() => {
expect(el.innerHTML).to.be.not.empty;
el.style.display = "";

expect(spy.afterinit.called).to.be.true;
expect(spy.rendered.called).to.be.true;

chart.resize({width: 500});

setTimeout(() => {
expect(spy.resize.called).to.be.true;
expect(spy.resized.called).to.be.true;
done();
}, 300);
}, 500);
});

it("check lazy rendering via option", done => {
const el = document.body.querySelector("#chart");

Expand All @@ -412,11 +472,17 @@ describe("Interface & initialization", () => {
// chart shouldn't be rendered
expect(el.innerHTML).to.be.empty;

for (let x in spy) {
expect(spy[x].called).to.be.false;
}

// call to render
chart.flush();

setTimeout(() => {
expect(el.innerHTML).to.be.not.empty;
expect(spy.afterinit.called).to.be.true;
expect(spy.rendered.called).to.be.true;
done();
}, 500);
});
Expand Down
12 changes: 7 additions & 5 deletions src/api/api.chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,15 @@ extend(Chart.prototype, {
*/
resize(size) {
const $$ = this.internal;
const {config, resizeFunction} = $$;
const {config} = $$;

config.size_width = size ? size.width : null;
config.size_height = size ? size.height : null;
if ($$.rendered) {
config.size_width = size ? size.width : null;
config.size_height = size ? size.height : null;

this.flush(false, true);
resizeFunction();
this.flush(false, true);
$$.resizeFunction();
}
},

/**
Expand Down
1 change: 0 additions & 1 deletion src/internals/Chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ export default class Chart {
$$.loadConfig(config);
$$.beforeInit(config);
$$.init();
$$.afterInit(config);

// bind "this" to nested API
(function bindThis(fn, target, argThis) {
Expand Down
1 change: 1 addition & 0 deletions src/internals/ChartInternal.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ export default class ChartInternal {
const convertedData = $$.convertData(config, $$.initWithData);

convertedData && $$.initWithData(convertedData);
$$.afterInit();
}
}

Expand Down

0 comments on commit 3e73fdf

Please sign in to comment.