Skip to content

Commit

Permalink
fix(grid): fix redundant <rect> appends
Browse files Browse the repository at this point in the history
When grid API is called, empty <rect> elements are appended.
Fix to not append unnecessary nodes.

Fix #3147
  • Loading branch information
netil committed Apr 10, 2023
1 parent 65b3c1c commit 1039363
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 11 deletions.
18 changes: 9 additions & 9 deletions src/ChartInternal/internals/region.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,26 @@ export default {

// hide if arc type
region.main.style("visibility", $$.hasArcType() ? "hidden" : null);
// select <g> element

let list = region.main
// select <g> element
const regions = region.main
.selectAll(`.${$REGION.region}`)
.data(config.regions);

$T(list.exit())
$T(regions.exit())
.style("opacity", "0")
.remove();

list = list.enter()
.append("g")
.merge(list)
.attr("class", $$.classRegion.bind($$));
const regionsEnter = regions.enter()
.append("g");

list
regionsEnter
.append("rect")
.style("fill-opacity", "0");

region.list = list;
region.list = regionsEnter
.merge(regions)
.attr("class", $$.classRegion.bind($$));
},

redrawRegion(withTransition) {
Expand Down
4 changes: 2 additions & 2 deletions test/api/chart-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe("API chart", () => {
});

it("should update groups correctly", function(done) {
this.timeout(1000);
this.timeout(2000);

const main = chart.$.main;
const path = main.select(`.${$BAR.bars}-data1 path`);
Expand All @@ -70,7 +70,7 @@ describe("API chart", () => {
done();
}, 300);

setTimeout(done, 500);
setTimeout(done, 1000);
});
});

Expand Down
26 changes: 26 additions & 0 deletions test/api/region-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,32 @@ describe("API region", function() {
done();
}, 1000);
});

it("check for <rect> element generation", () => {
// when
chart.regions.add({
axis: "y",
start: 150,
end: 200,
class: "a",
});

chart.regions.add({
axis: "y",
start: 200,
end: 220,
class: "b",
});

const regionList = chart.internal.$el.region.list;

expect(regionList.size()).to.be.equal(4);

// regions <rect> element should be 1
regionList.each(function(d, i) {
expect(this.querySelectorAll("rect").length).to.be.equal(1);
});
});
});

describe("Add regions using regions()", () => {
Expand Down

0 comments on commit 1039363

Please sign in to comment.