Skip to content

Commit

Permalink
fix(axis): Correct axis transition
Browse files Browse the repository at this point in the history
- Fixed to not applying transiton when transition.duration is falsy
- Avoid computing max tick width value if domain hasn't been changed
- Simplified .getXAxis()/.getYAxis() arguments numbers

Fix #759
  • Loading branch information
netil authored Feb 8, 2019
1 parent 51ee8ce commit e2bec90
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 67 deletions.
86 changes: 47 additions & 39 deletions src/axis/Axis.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,20 +134,24 @@ export default class Axis {
});
}

getXAxis(axisName, scale, orient, tickFormat,
tickValues, withOuterTick, withoutTransition, withoutRotateTickText) {
// called from : updateScales() & getMaxTickWidth()
getXAxis(name, scale, outerTick, noTransition, noTickTextRotate) {
const $$ = this.owner;
const config = $$.config;
const isCategory = $$.isCategorized();
const orient = $$[`${name}Orient`];
const tickFormat = $$.xAxisTickFormat;
const tickValues = $$.xAxisTickValues;

const axisParams = {
isCategory,
withOuterTick,
withoutTransition,
outerTick,
noTransition,
config,
axisName,
name,
tickMultiline: config.axis_x_tick_multiline,
tickWidth: config.axis_x_tick_width,
tickTextRotate: withoutRotateTickText ? 0 : config.axis_x_tick_rotate,
tickTextRotate: noTickTextRotate ? 0 : config.axis_x_tick_rotate,
tickTitle: isCategory && config.axis_x_tick_tooltip && $$.api.categories(),
orgXScale: $$.x
};
Expand Down Expand Up @@ -176,16 +180,20 @@ export default class Axis {
return axis;
}

getYAxis(axisName, scale, orient, tickFormat, tickValues,
withOuterTick, withoutTransition, withoutRotateTickText) {
// called from : updateScales() & getMaxTickWidth()
getYAxis(name, scale, outerTick, noTransition, noTickTextRotate) {
const $$ = this.owner;
const config = $$.config;
const orient = $$[`${name}Orient`];
const tickFormat = config[`axis_${name}_tick_format`];
const tickValues = $$[`${name}AxisTickValues`];

const axisParams = {
withOuterTick,
withoutTransition,
outerTick,
noTransition,
config,
axisName,
tickTextRotate: withoutRotateTickText ? 0 : config.axis_y_tick_rotate
name,
tickTextRotate: noTickTextRotate ? 0 : config.axis_y_tick_rotate
};

const axis = new AxisRenderer(axisParams)
Expand Down Expand Up @@ -260,10 +268,10 @@ export default class Axis {
) : format;
}

getTickValues(type) {
getTickValues(id) {
const $$ = this.owner;
const tickValues = $$.config[`axis_${type}_tick_values`];
const axis = $$[`${type}Axis`];
const tickValues = $$.config[`axis_${id}_tick_values`];
const axis = $$[`${id}Axis`];

return tickValues || (axis ? axis.tickValues() : undefined);
}
Expand All @@ -280,33 +288,33 @@ export default class Axis {
return this.getTickValues("y2");
}

getLabelOptionByAxisId(axisId) {
return this.owner.config[`axis_${axisId}_label`];
getLabelOptionByAxisId(id) {
return this.owner.config[`axis_${id}_label`];
}

getLabelText(axisId) {
const option = this.getLabelOptionByAxisId(axisId);
getLabelText(id) {
const option = this.getLabelOptionByAxisId(id);

return isString(option) ? option : (
option ? option.text : null
);
}

setLabelText(axisId, text) {
setLabelText(id, text) {
const $$ = this.owner;
const config = $$.config;
const option = this.getLabelOptionByAxisId(axisId);
const option = this.getLabelOptionByAxisId(id);

if (isString(option)) {
config[`axis_${axisId}_label`] = text;
config[`axis_${id}_label`] = text;
} else if (option) {
option.text = text;
}
}

getLabelPosition(axisId, defaultPosition) {
getLabelPosition(id, defaultPosition) {
const isRotated = this.owner.config.axis_rotated;
const option = this.getLabelOptionByAxisId(axisId);
const option = this.getLabelOptionByAxisId(id);
const position = (isObjectType(option) && option.position) ?
option.position : defaultPosition[+!isRotated];

Expand Down Expand Up @@ -475,11 +483,11 @@ export default class Axis {
getMaxTickWidth(id, withoutRecompute) {
const $$ = this.owner;
const config = $$.config;
const currentTickMax = $$.currentMaxTickWidths;
const currentTickMax = $$.currentMaxTickWidths[id];
let maxWidth = 0;

if ((withoutRecompute && currentTickMax[id]) || !config[`axis_${id}_show`]) {
return currentTickMax[id];
if (withoutRecompute || !config[`axis_${id}_show`]) {
return currentTickMax.size;
}

if ($$.svg) {
Expand All @@ -488,16 +496,16 @@ export default class Axis {
const getFrom = isYAxis ? "getY" : "getX";

const scale = $$[id].copy().domain($$[`${getFrom}Domain`](targetsToShow, id));
const axis = this[`${getFrom}Axis`](
id,
scale,
$$[`${id}Orient`],
isYAxis ? config[`axis_${id}_tick_format`] : $$.xAxisTickFormat,
null,
false,
true,
true
);
const domain = scale.domain().toString();

// do not compute if domain is same
if (currentTickMax.domain === domain) {
return currentTickMax.size;
} else {
currentTickMax.domain = domain;
}

const axis = this[`${getFrom}Axis`](id, scale, false, true, true);

!isYAxis && this.updateXAxisTickValues(targetsToShow, axis);

Expand All @@ -518,10 +526,10 @@ export default class Axis {
}

if (maxWidth > 0) {
currentTickMax[id] = maxWidth;
currentTickMax.size = maxWidth;
}

return currentTickMax[id];
return currentTickMax.size;
}

updateLabels(withTransition) {
Expand Down
14 changes: 7 additions & 7 deletions src/axis/AxisRenderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export default class AxisRenderer {
constructor(params = {}) {
const config = {
innerTickSize: 6,
outerTickSize: params.withOuterTick ? 6 : 0,
outerTickSize: params.outerTick ? 6 : 0,
orient: "bottom",
range: [],
tickArguments: null,
Expand All @@ -23,7 +23,7 @@ export default class AxisRenderer {
tickPadding: 3,
tickValues: null,
transition: null,
withoutTransition: params.withoutTransition
noTransition: params.noTransition
};

config.tickLength = Math.max(config.innerTickSize, 0) + config.tickPadding;
Expand Down Expand Up @@ -65,13 +65,13 @@ export default class AxisRenderer {

const {innerTickSize, tickLength, range} = config;

// get the axis' tick position configuration
const axisName = params.axisName;
const tickTextPos = axisName && /^(x|y|y2)$/.test(axisName) ?
params.config[`axis_${axisName}_tick_text_position`] : {x: 0, y: 0};
// // get the axis' tick position configuration
const name = params.name;
const tickTextPos = name && /^(x|y|y2)$/.test(name) ?
params.config[`axis_${name}_tick_text_position`] : {x: 0, y: 0};

// tick visiblity
const prefix = axisName === "subx" ? `subchart_axis_x` : `axis_${axisName}`;
const prefix = name === "subX" ? `subchart_axis_x` : `axis_${name}`;
const axisShow = params.config[`${prefix}_show`];
const tickShow = {
tick: axisShow ? params.config[`${prefix}_tick_show`] : false,
Expand Down
6 changes: 5 additions & 1 deletion src/axis/AxisRendererHelper.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ export default class AxisRendererHelper {
this.config = config;
this.scale = scale;

if (!params.config.transition_duration) {
config.noTransition = true;
}

// set range
config.range = scale.rangeExtent ?
scale.rangeExtent() :
Expand Down Expand Up @@ -121,7 +125,7 @@ export default class AxisRendererHelper {
transitionise(selection) {
const config = this.config;

return config.withoutTransition ?
return config.noTransition ?
selection.interrupt() : selection.transition(config.transition);
}
}
4 changes: 2 additions & 2 deletions src/config/Options.js
Original file line number Diff line number Diff line change
Expand Up @@ -680,8 +680,8 @@ export default class Options {
* data: {
* colors: {
* data1: "#ff0000",
* data2: function(d) {
* return "#000";
* data2: function(d) {
* return "#000";
* }
* ...
* }
Expand Down
4 changes: 3 additions & 1 deletion src/internals/ChartInternal.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ export default class ChartInternal {
$$.legendItemHeight = 0;

$$.currentMaxTickWidths = {
x: 0, y: 0, y2: 0
x: {size: 0, domain: ""},
y: {size: 0, domain: ""},
y2: {size: 0, domain: ""}
};

$$.rotated_padding_left = 30;
Expand Down
29 changes: 12 additions & 17 deletions src/internals/scale.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,12 @@ extend(ChartInternal.prototype, {
/**
* Update scale
* @private
* @param {Boolean} withoutTransitionAtInit - param is given at the init rendering
* @param {Boolean} isInit - param is given at the init rendering
*/
updateScales(withoutTransitionAtInit) {
updateScales(isInit) {
const $$ = this;
const config = $$.config;
const isRotated = config.axis_rotated;
const isInit = !$$.x;

// update edges
$$.xMin = isRotated ? 1 : 0;
Expand All @@ -126,41 +125,37 @@ extend(ChartInternal.prototype, {

// update scales
// x Axis
$$.x = $$.getX($$.xMin, $$.xMax, isInit ? undefined : $$.x.orgDomain(),
() => $$.xAxis.tickOffset());
$$.x = $$.getX($$.xMin, $$.xMax, $$.x && $$.x.orgDomain(), () => $$.xAxis.tickOffset());
$$.subX = $$.getX($$.xMin, $$.xMax, $$.orgXDomain, d => (d % 1 ? 0 : $$.subXAxis.tickOffset()));

$$.xAxisTickFormat = $$.axis.getXAxisTickFormat();
$$.xAxisTickValues = $$.axis.getXAxisTickValues();

$$.xAxis = $$.axis
.getXAxis("x", $$.x, $$.xOrient, $$.xAxisTickFormat,
$$.xAxisTickValues, config.axis_x_tick_outer, withoutTransitionAtInit);
.getXAxis("x", $$.x, config.axis_x_tick_outer, isInit);

$$.subXAxis = $$.axis
.getXAxis("subx", $$.subX, $$.subXOrient, $$.xAxisTickFormat,
$$.xAxisTickValues, config.axis_x_tick_outer, withoutTransitionAtInit);
.getXAxis("subX", $$.subX, config.axis_x_tick_outer, isInit);

// y Axis
$$.y = $$.getY($$.yMin, $$.yMax, isInit ? config.axis_y_default : $$.y.domain());
$$.subY = $$.getY($$.subYMin, $$.subYMax, isInit ? config.axis_y_default : $$.subY.domain());
$$.y = $$.getY($$.yMin, $$.yMax, $$.y ? $$.y.domain() : config.axis_y_default);
$$.subY = $$.getY($$.subYMin, $$.subYMax, $$.subY ? $$.subY.domain() : config.axis_y_default);

$$.yAxisTickValues = $$.axis.getYAxisTickValues();

$$.yAxis = $$.axis
.getYAxis("y", $$.y, $$.yOrient, config.axis_y_tick_format,
$$.yAxisTickValues, config.axis_y_tick_outer);
.getYAxis("y", $$.y, config.axis_y_tick_outer, isInit);

// y2 Axis
if (config.axis_y2_show) {
$$.y2 = $$.getY($$.yMin, $$.yMax, isInit ? config.axis_y2_default : $$.y2.domain());
$$.subY2 = $$.getY($$.subYMin, $$.subYMax, isInit ? config.axis_y2_default : $$.subY2.domain());
$$.y2 = $$.getY($$.yMin, $$.yMax, $$.y2 ? $$.y2.domain() : config.axis_y2_default);
$$.subY2 = $$.getY($$.subYMin, $$.subYMax,
$$.subY2 ? $$.subY2.domain() : config.axis_y2_default);

$$.y2AxisTickValues = $$.axis.getY2AxisTickValues();

$$.y2Axis = $$.axis
.getYAxis("y2", $$.y2, $$.y2Orient, config.axis_y2_tick_format,
$$.y2AxisTickValues, config.axis_y2_tick_outer);
.getYAxis("y2", $$.y2, config.axis_y2_tick_outer, isInit);
}

// update for arc
Expand Down

0 comments on commit e2bec90

Please sign in to comment.