Skip to content

Commit

Permalink
feat(grid): Intent to ship grid.front option (#385)
Browse files Browse the repository at this point in the history
New option to handle grid element position on DOM hierarchy

Fix #384
Close #385
  • Loading branch information
netil authored Apr 20, 2018
1 parent 23ff1df commit 58621a0
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 64 deletions.
101 changes: 67 additions & 34 deletions spec/internals/grid-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* billboard.js project is licensed under the MIT license
*/
/* eslint-disable */
import CLASS from "../../src/config/classes";
import util from "../assets/util";

describe("GRID", function() {
Expand All @@ -23,8 +24,7 @@ describe("GRID", function() {
},
axis: {
y: {
tick: {
}
tick: {}
}
},
grid: {
Expand All @@ -36,53 +36,47 @@ describe("GRID", function() {
});

it("should not show y grids", () => {
expect(chart.internal.main.select(".bb-ygrids").size()).to.be.equal(0);
expect(chart.internal.main.select(`.${CLASS.ygrids}`).size()).to.be.equal(0);
});

it("should update args to show y grids", () => {
it("set options grid.y.show=true", () => {
args.grid.y.show = true;

expect(true).to.be.ok;
});

it("should show y grids", function() {
const ygrids = chart.internal.main.select(".bb-ygrids");
const ygrids = chart.internal.main.select(`.${CLASS.ygrids}`);

expect(ygrids.size()).to.be.equal(1);
expect(ygrids.selectAll(".bb-ygrid").size()).to.be.equal(9);
expect(ygrids.selectAll(`.${CLASS.ygrid}`).size()).to.be.equal(9);
});

it("should update args to show only 3 y grids", () => {
it("set options grid.y.ticks=3", () => {
args.grid.y.ticks = 3;

expect(true).to.be.ok;
});

it("should show only 3 y grids", () => {
const ygrids = chart.internal.main.select(".bb-ygrids");
const ygrids = chart.internal.main.select(`.${CLASS.ygrids}`);

expect(ygrids.size()).to.be.equal(1);
expect(ygrids.selectAll(".bb-ygrid").size()).to.be.equal(3);
expect(ygrids.selectAll(`.${CLASS.ygrid}`).size()).to.be.equal(3);
});

it("should update args to show y grids depending on y axis ticks", () => {
it("set options axis.y.tick.count=5", () => {
args.axis.y.tick.count = 5;

expect(true).to.be.ok;
});

it("should show grids depending on y axis ticks", () => {
const ygrids = chart.internal.main.select(".bb-ygrids");
const ygrids = chart.internal.main.select(`.${CLASS.ygrids}`);
const expectedYs = [];

ygrids.selectAll(".bb-ygrid").each(function(d, i) {
ygrids.selectAll(`.${CLASS.ygrid}`).each(function(d, i) {
expectedYs[i] = +d3.select(this).attr("y1");
});

expect(ygrids.size()).to.be.equal(1);
expect(ygrids.selectAll(".bb-ygrid").size()).to.be.equal(5);
expect(ygrids.selectAll(`.${CLASS.ygrid}`).size()).to.be.equal(5);

chart.internal.main.select(".bb-axis-y").selectAll(".tick").each(function(d, i) {
chart.internal.main.select(`.${CLASS.axisY}`).selectAll(".tick").each(function(d, i) {
let y = d3.select(this).attr("transform").match(/\d+\)/);

if (y.length >= 1) {
Expand All @@ -94,6 +88,26 @@ describe("GRID", function() {
});
});

describe("front option", () => {
it("grid element should positioned before chart element", () => {
const grid = chart.internal.main.select(`.${CLASS.grid}`).node();
const nextSiblingClassName = grid.nextSibling.getAttribute("class");

expect(nextSiblingClassName).to.be.equal(CLASS.chart);
});

it("set options grid.front=true", () => {
args.grid.front = true;
});

it("grid element should positioned after gridLines element", () => {
const grid = chart.internal.main.select(`.${CLASS.xgridFocus}`).node().parentNode;
const previousSiblingClassName = grid.previousSibling.getAttribute("class");

expect(previousSiblingClassName).to.be.equal(`${CLASS.grid} ${CLASS.gridLines}`);
});
});

describe("y grid lines", () => {
describe("position #1", () => {
before(() => {
Expand All @@ -116,12 +130,12 @@ describe("GRID", function() {
});

it("should show 3 grid lines", () => {
expect(chart.internal.main.selectAll(".bb-ygrid-lines .bb-ygrid-line").size()).to.be.equal(3);
expect(chart.internal.main.selectAll(`.${CLASS.ygrid}-lines .${CLASS.ygrid}-line`).size()).to.be.equal(3);
});

it("should locate grid lines properly", done => {
setTimeout(() => {
const lines = chart.internal.main.selectAll(".bb-ygrid-lines .bb-ygrid-line line");
const lines = chart.internal.main.selectAll(`.${CLASS.ygrid}-lines .${CLASS.ygrid}-line line`);
const expectedY1s = [373, 268, 196];

lines.each(function (d, i) {
Expand All @@ -136,7 +150,7 @@ describe("GRID", function() {
});

it("should locate grid texts properly", () => {
const lines = chart.internal.main.selectAll(".bb-ygrid-lines .bb-ygrid-line");
const lines = chart.internal.main.selectAll(`.${CLASS.ygrid}-lines .${CLASS.ygrid}-line`);
const expectedPositions = ["start", "middle", "end"];
const expectedDxs = [4, 0, -4];

Expand Down Expand Up @@ -175,12 +189,12 @@ describe("GRID", function() {
});

it("should show 3 grid lines", () => {
expect(chart.internal.main.selectAll(".bb-ygrid-lines .bb-ygrid-line").size()).to.be.equal(3);
expect(chart.internal.main.selectAll(`.${CLASS.ygrid}-lines .${CLASS.ygrid}-line`).size()).to.be.equal(3);
});

it("should locate grid lines properly", done => {
setTimeout(() => {
const lines = chart.internal.main.selectAll(".bb-ygrid-lines .bb-ygrid-line line");
const lines = chart.internal.main.selectAll(`.${CLASS.ygrid}-lines .${CLASS.ygrid}-line line`);
const expectedX1s = [75, 220, 321];

lines.each(function(d, i) {
Expand All @@ -195,7 +209,7 @@ describe("GRID", function() {
});

it("should locate grid texts properly", () => {
const lines = chart.internal.main.selectAll(".bb-ygrid-lines .bb-ygrid-line");
const lines = chart.internal.main.selectAll(`.${CLASS.ygrid}-lines .${CLASS.ygrid}-line`);
const expectedPositions = ["start", "middle", "end"];
const expectedDxs = [4, 0, -4];

Expand All @@ -208,7 +222,26 @@ describe("GRID", function() {
expect(+dx).to.be.equal(expectedDxs[i]);
});
});
});

describe("lines.front option", () => {
it("grid lines should positioned after chart element", () => {
const gridLines = chart.internal.main.select(`.${CLASS.grid}-lines`).node();
const previousSiblingClassName = gridLines.previousSibling.getAttribute("class");

expect(previousSiblingClassName).to.be.equal(CLASS.chart);
});

it("set options grid.lines.front=false", () => {
args.grid.lines = {front: false};
});

it("grid lines should positioned before grid element", () => {
const gridLines = chart.internal.main.select(`.${CLASS.grid}-lines`).node();
const nextSiblingClassName = gridLines.nextSibling.getAttribute("class");

expect(nextSiblingClassName).to.be.equal(CLASS.grid);
});
});
});

Expand All @@ -234,11 +267,11 @@ describe("GRID", function() {
});

it("should show 3 grid lines", () => {
expect(chart.internal.main.selectAll(".bb-xgrid-lines .bb-xgrid-line").size()).to.be.equal(3);
expect(chart.internal.main.selectAll(`.${CLASS.xgrid}-lines .${CLASS.xgrid}-line`).size()).to.be.equal(3);
});

it("should locate grid lines properly", () => {
const lines = chart.internal.main.selectAll(".bb-xgrid-lines .bb-xgrid-line");
const lines = chart.internal.main.selectAll(`.${CLASS.xgrid}-lines .${CLASS.xgrid}-line`);
const expectedX1s = [202, 397, 593];

lines.each(function (d, i) {
Expand All @@ -249,7 +282,7 @@ describe("GRID", function() {
});

it("should locate grid texts properly", () => {
const lines = chart.internal.main.selectAll(".bb-xgrid-lines .bb-xgrid-line");
const lines = chart.internal.main.selectAll(`.${CLASS.xgrid}-lines .${CLASS.xgrid}-line`);
const expectedPositions = ["start", "middle", "end"];
const expectedDxs = [4, 0, -4];

Expand Down Expand Up @@ -288,11 +321,11 @@ describe("GRID", function() {
});

it("should show 3 grid lines", () => {
expect(chart.internal.main.selectAll(".bb-xgrid-lines .bb-xgrid-line").size()).to.be.equal(3);
expect(chart.internal.main.selectAll(`.${CLASS.xgrid}-lines .${CLASS.xgrid}-line`).size()).to.be.equal(3);
});

it("should locate grid lines properly", () => {
const lines = chart.internal.main.selectAll(".bb-xgrid-lines .bb-xgrid-line");
const lines = chart.internal.main.selectAll(`.${CLASS.xgrid}-lines .${CLASS.xgrid}-line`);
const expectedY1s = [144, 283, 421];

lines.each(function(d, i) {
Expand All @@ -303,7 +336,7 @@ describe("GRID", function() {
});

it("should locate grid texts properly", () => {
const lines = chart.internal.main.selectAll(".bb-xgrid-lines .bb-xgrid-line");
const lines = chart.internal.main.selectAll(`.${CLASS.xgrid}-lines .${CLASS.xgrid}-line`);
const expectedPositions = ["start", "middle", "end"];
const expectedDxs = [4, 0, -4];

Expand Down Expand Up @@ -341,7 +374,7 @@ describe("GRID", function() {
});

it("should show x grid lines", () => {
const lines = chart.internal.main.select(".bb-xgrid-lines .bb-xgrid-line");
const lines = chart.internal.main.select(`.${CLASS.xgrid}-lines .${CLASS.xgrid}-line`);
const expectedX1 = 593;
const expectedText = ["Label 3"];

Expand Down Expand Up @@ -384,7 +417,7 @@ describe("GRID", function() {
});

it("should show x grid lines", () => {
const lines = chart.internal.main.selectAll(".bb-xgrid-lines .bb-xgrid-line");
const lines = chart.internal.main.selectAll(`.${CLASS.xgrid}-lines .${CLASS.xgrid}-line`);
const expectedX1 = [524, 75];
const expectedText = ["Label 3", "Label a"];

Expand Down
7 changes: 5 additions & 2 deletions src/config/Options.js
Original file line number Diff line number Diff line change
Expand Up @@ -2104,16 +2104,17 @@ export default class Options {
* @name grid
* @memberOf Options
* @type {Object}
* @property {Boolean} [front=false] Set 'grid & focus lines' to be positioned over grid lines and chart elements.
* @property {Boolean} [x.show=false] Show grids along x axis.
* @property {Boolean} [x.lines=[]] Show additional grid lines along x axis.<br>
* This option accepts array including object that has value, text, position and class. text, position and class are optional. For position, start, middle and end (default) are available.
* If x axis is category axis, value can be category name. If x axis is timeseries axis, value can be date string, Date object and unixtime integer.
* @property {Boolean} [y.show=false] Show grids along x axis.
* @property {Boolean} [y.lines=[]] Show additional grid lines along y axis.<br>
* This option accepts array including object that has value, text, position and class.
* @property {Boolean} [y.ticks=10]
* @property {Boolean} [y.ticks=10] Number of y grids to be shown.
* @property {Boolean} [focus.show=true] Show grids when focus.
* @property {Boolean} [lines.front=true]
* @property {Boolean} [lines.front=true] Set grid lines to be positioned over chart elements.
* @default undefined
* @example
* grid: {
Expand All @@ -2134,6 +2135,7 @@ export default class Options {
* ],
* ticks: 5
* },
* front: true,
* focus: {
* show: false
* },
Expand All @@ -2149,6 +2151,7 @@ export default class Options {
grid_y_lines: [],
grid_y_ticks: 10,
grid_focus_show: true,
grid_front: false,
grid_lines_front: true,

/**
Expand Down
1 change: 1 addition & 0 deletions src/internals/ChartInternal.js
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ export default class ChartInternal {

// Grid lines
config.grid_lines_front && $$.initGridLines();
config.grid_front && $$.initXYFocusGrid();

// Cover whole with rects for events
$$.initEventRect();
Expand Down
57 changes: 29 additions & 28 deletions src/internals/grid.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,10 @@ extend(ChartInternal.prototype, {
const $$ = this;
const config = $$.config;

$$.grid = $$.main.append("g")
.attr("clip-path", $$.clipPathForGrid)
.attr("class", CLASS.grid);

if (config.grid_x_show) {
$$.grid.append("g").attr("class", CLASS.xgrids);
}

if (config.grid_y_show) {
$$.grid.append("g").attr("class", CLASS.ygrids);
}

if (config.grid_focus_show) {
$$.grid.append("g")
.attr("class", CLASS.xgridFocus)
.append("line")
.attr("class", CLASS.xgridFocus);
}

$$.xgrid = d3SelectAll([]);

if (!config.grid_lines_front) {
$$.initGridLines();
}
!config.grid_lines_front && $$.initGridLines();
!config.grid_front && $$.initXYFocusGrid();
},

initGridLines() {
Expand Down Expand Up @@ -150,9 +130,7 @@ extend(ChartInternal.prototype, {
main.select(`line.${CLASS.xgridFocus}`)
.style("visibility", "hidden");

if (config.grid_x_show) {
$$.updateXGrid();
}
config.grid_x_show && $$.updateXGrid();

$$.xgridLines = main.select(`.${CLASS.xgridLines}`)
.selectAll(`.${CLASS.xgridLine}`)
Expand Down Expand Up @@ -181,9 +159,8 @@ extend(ChartInternal.prototype, {
$$.xgridLines = xgridLine.merge($$.xgridLines);

// Y-Grid
if (config.grid_y_show) {
$$.updateYGrid();
}
config.grid_y_show && $$.updateYGrid();

$$.ygridLines = main.select(`.${CLASS.ygridLines}`)
.selectAll(`.${CLASS.ygridLine}`)
.data(config.grid_y_lines);
Expand Down Expand Up @@ -259,6 +236,30 @@ extend(ChartInternal.prototype, {
];
},

initXYFocusGrid() {
const $$ = this;
const config = $$.config;

$$.grid = $$.main.append("g")
.attr("clip-path", $$.clipPathForGrid)
.attr("class", CLASS.grid);

if (config.grid_x_show) {
$$.grid.append("g").attr("class", CLASS.xgrids);
}

if (config.grid_y_show) {
$$.grid.append("g").attr("class", CLASS.ygrids);
}

if (config.grid_focus_show) {
$$.grid.append("g")
.attr("class", CLASS.xgridFocus)
.append("line")
.attr("class", CLASS.xgridFocus);
}
},

showXGridFocus(selectedData) {
const $$ = this;
const config = $$.config;
Expand Down

0 comments on commit 58621a0

Please sign in to comment.