Skip to content

Commit

Permalink
fix(axis): Avoid unnecessary computing char dimension
Browse files Browse the repository at this point in the history
Moved and updated condition on getSizeFor1Char() call to avoid multiple calls.

Ref #399
Close #407
  • Loading branch information
netil authored May 11, 2018
1 parent cd7c6b9 commit 3f52827
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 31 deletions.
18 changes: 18 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
"eslint-config-naver": "^2.0.0",
"eslint-loader": "^2.0.0",
"eslint-plugin-import": "^2.11.0",
"exports-loader": "^0.7.0",
"hammer-simulator": "0.0.1",
"husky": "^0.14.3",
"inject-loader": "^4.0.1",
Expand Down
9 changes: 8 additions & 1 deletion spec/internals/axis-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import util from "../assets/util";
import bb from "../../src/core";
import CLASS from "../../src/config/classes";
import getSizeFor1Char from "exports-loader?getSizeFor1Char!../../src/axis/bb.axis";

describe("AXIS", function() {
let chart;
Expand Down Expand Up @@ -82,6 +83,13 @@ describe("AXIS", function() {
args.axis.y.tick.values = values;
});

it("should compute char dimension", () => {
const size = getSizeFor1Char(d3.select(".tick"));

expect(size.w && size.h).to.be.ok;
expect(getSizeFor1Char.size).to.be.equal(size);
});

it("should have only 2 tick on y axis", () => {
const ticksSize = chart.internal.main.select(`.${CLASS.axisY}`)
.selectAll("g.tick").size();
Expand Down Expand Up @@ -1006,7 +1014,6 @@ describe("AXIS", function() {
});
});


describe("axis text on 'binary floating point'", () => {
before(() => {
args = {
Expand Down
61 changes: 32 additions & 29 deletions src/axis/bb.axis.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,37 @@ import {isArray, toArray, isDefined, isFunction, isString} from "../internals/ut
// 2. ceil values of translate/x/y to int for half pixel anti-aliasing
// 3. multiline tick text

/**
* Compute a character dimension
* @param {d3.selection} node
* @return {{w: number, h: number}}
* @private
*/
const getSizeFor1Char = node => {
// default size for one character
const size = {
w: 5.5,
h: 11.5
};

!node.empty() && node.select("text")
.text("0")
.call(el => {
const box = el.node().getBBox();
const h = box.height;
const w = box.width;

if (h && w) {
size.h = h;
size.w = w;
}

el.text("");
});

return (getSizeFor1Char.size = size);
};

export default function(params = {}) {
let scale = d3ScaleLinear();
let orient = "bottom";
Expand Down Expand Up @@ -89,34 +120,6 @@ export default function(params = {}) {
return isDefined(formatted) ? formatted : "";
}

let getSizeFor1Char = tick => {
// default size for one character
const size = {
h: 11.5,
w: 5.5
};

!tick.empty() && tick.select("text")
.text("0")
.call(el => {
const box = el.node().getBBox();
const h = box.height;
const w = box.width;

if (h && w) {
size.h = h;
size.w = w;
}

el.text("");
});

// overwrite to return calculated size
getSizeFor1Char = () => size;

return size;
};

function transitionise(selection) {
return params.withoutTransition ?
selection.interrupt() : selection.transition(transition);
Expand Down Expand Up @@ -189,7 +192,7 @@ export default function(params = {}) {
}

let tspan;
const sizeFor1Char = getSizeFor1Char(g.select(".tick"));
const sizeFor1Char = getSizeFor1Char.size || getSizeFor1Char(g.select(".tick"));
const counts = [];
const tickLength = Math.max(innerTickSize, 0) + tickPadding;
const isVertical = orient === "left" || orient === "right";
Expand Down
3 changes: 2 additions & 1 deletion src/internals/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ const isEmpty = o => (
(isString(o) && o.length === 0) ||
(isObjectType(o) && Object.keys(o).length === 0)
);
const notEmpty = o => !isEmpty(o);

/**
* Check if is array
* @param {Array} arr
Expand All @@ -38,7 +40,6 @@ const isArray = arr => arr && arr.constructor === Array;
*/
const isObject = obj => obj && !obj.nodeType && isObjectType(obj) && !isArray(obj);

const notEmpty = o => !isEmpty(o);
const getOption = (options, key, defaultValue) => (
isDefined(options[key]) ? options[key] : defaultValue
);
Expand Down
11 changes: 11 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3176,6 +3176,13 @@ expand-tilde@^2.0.0, expand-tilde@^2.0.2:
dependencies:
homedir-polyfill "^1.0.1"

exports-loader@^0.7.0:
version "0.7.0"
resolved "https://registry.yarnpkg.com/exports-loader/-/exports-loader-0.7.0.tgz#84881c784dea6036b8e1cd1dac3da9b6409e21a5"
dependencies:
loader-utils "^1.1.0"
source-map "0.5.0"

express@^4.15.5, express@^4.16.2:
version "4.16.3"
resolved "https://registry.yarnpkg.com/express/-/express-4.16.3.tgz#6af8a502350db3246ecc4becf6b5a34d22f7ed53"
Expand Down Expand Up @@ -7835,6 +7842,10 @@ source-map-url@~0.3.0:
version "0.3.0"
resolved "https://registry.yarnpkg.com/source-map-url/-/source-map-url-0.3.0.tgz#7ecaf13b57bcd09da8a40c5d269db33799d4aaf9"

[email protected]:
version "0.5.0"
resolved "https://registry.yarnpkg.com/source-map/-/source-map-0.5.0.tgz#0fe96503ac86a5adb5de63f4e412ae4872cdbe86"

[email protected], source-map@^0.5.3, source-map@^0.5.6, source-map@^0.5.7, source-map@~0.5.1:
version "0.5.7"
resolved "https://registry.yarnpkg.com/source-map/-/source-map-0.5.7.tgz#8a039d2d1021d22d1ea14c80d8ea468ba2ef3fcc"
Expand Down

0 comments on commit 3f52827

Please sign in to comment.