From 74c2ab8543ec66e47e1e1302054ae3cd242df4e9 Mon Sep 17 00:00:00 2001 From: "Maarten A. Breddels" Date: Tue, 29 Nov 2022 15:00:06 +0100 Subject: [PATCH] fix: axis formatting depended on initial values or changes The codepath to to axis formatting for the initial values was different from when properties such as tick_values are updated afterwards. This was probably due to some technical debt/circular dependency in the code. If did the minimal amount of change to make sure the behaviour is the same, while having a consistent codepath. --- js/src/Axis.ts | 67 ++++++++++++++++++--------------------------- js/src/ColorAxis.ts | 12 ++++++-- 2 files changed, 37 insertions(+), 42 deletions(-) diff --git a/js/src/Axis.ts b/js/src/Axis.ts index 39471f779..c01bf6e67 100644 --- a/js/src/Axis.ts +++ b/js/src/Axis.ts @@ -59,12 +59,14 @@ export class Axis extends WidgetView { Promise.all([scale_promise, offset_promise]).then(() => { this.create_listeners(); - this.tick_format = this.generate_tick_formatter(); + this.create_axis(); this.set_scales_range(); + this.update_scales(); + this.set_tick_values(); + this.tickformat_changed(); this.append_axis(); }); } - create_listeners() { // Creates all event listeners @@ -125,7 +127,10 @@ export class Axis extends WidgetView { update_display() { this.g_axisline.remove(); + this.create_axis(); + this.set_tick_values(); this.set_scales_range(); + this.tickformat_changed(); this.append_axis(); } @@ -194,16 +199,6 @@ export class Axis extends WidgetView { this.axis.tickValues(this.axis_scale.scale.ticks()); } } - if ( - this.model.get('tick_format') === null || - this.model.get('tick_format') === undefined - ) { - if (!isOrdinalScale(this.axis_scale)) { - this.tick_format = this.guess_tick_format(this.axis.tickValues()); - } - } - this.axis.tickFormat(this.tick_format); - if (this.g_axisline) { this.g_axisline .transition('set_tick_values') @@ -231,9 +226,11 @@ export class Axis extends WidgetView { apply_tick_styling() { // Applies current tick styling to all displayed ticks - const tickText = this.g_axisline.selectAll('.tick text'); - applyStyles(tickText, this.model.get('tick_style')); - tickText.attr('transform', this.get_tick_transforms()); + if (this.g_axisline) { + const tickText = this.g_axisline.selectAll('.tick text'); + applyStyles(tickText, this.model.get('tick_style')); + tickText.attr('transform', this.get_tick_transforms()); + } } get_tick_transforms() { @@ -367,7 +364,6 @@ export class Axis extends WidgetView { } append_axis() { - this.create_axis(); this.update_scales(); // Create initial SVG element @@ -386,7 +382,6 @@ export class Axis extends WidgetView { applyAttrs(lineText, this.get_label_attributes()); // Apply custom settings - this.set_tick_values(); this.update_grid_lines(); this.update_color(); this.apply_tick_styling(); @@ -682,6 +677,7 @@ export class Axis extends WidgetView { //animate axis and grid lines on domain changes const animate = true; this.set_tick_values(animate); + this.tickformat_changed(); this.update_grid_lines(animate); } @@ -860,14 +856,11 @@ export class Axis extends WidgetView { }; } - _linear_scale_precision(ticks?: any[]): number { + _linear_scale_precision(): number { if (!(isLinearScale(this.axis_scale) || isColorScale(this.axis_scale))) { return -1; } - ticks = - ticks === undefined || ticks === null - ? this.axis_scale.scale.ticks() - : ticks; + let ticks: any[] = this.axis.tickValues(); // Case where all data is concentrated into one point. if (ticks.length === 1) { return 1; @@ -898,19 +891,16 @@ export class Axis extends WidgetView { } } - linear_sc_format(ticks?: any[]) { - return this.get_format_func(this._linear_scale_precision(ticks)); + linear_sc_format() { + return this.get_format_func(this._linear_scale_precision()); } - date_sc_format(ticks?: any[]) { + date_sc_format() { // assumes that scale is a linear date scale if (!isDateScale(this.axis_scale)) { return; } - ticks = - ticks === undefined || ticks === null - ? this.axis_scale.scale.ticks() - : ticks; + let ticks: any[] = this.axis.tickValues(); // diff is the difference between ticks in milliseconds const diff = Math.abs(ticks[1] - ticks[0]); @@ -981,18 +971,15 @@ export class Axis extends WidgetView { }; } - log_sc_format(ticks?: any[]) { - return this.get_format_func(this._log_sc_precision(ticks)); + log_sc_format() { + return this.get_format_func(this._log_sc_precision()); } - _log_sc_precision(ticks?: any[]): number { + _log_sc_precision(): number { if (!isLogScale(this.axis_scale)) { return -1; } - ticks = - ticks === undefined || ticks === null - ? this.axis_scale.scale.ticks() - : ticks; + let ticks: any[] = this.axis.tickValues(); const ratio = Math.abs(Math.log10(ticks[1] / ticks[0])); if (ratio >= 0.301) { @@ -1004,16 +991,16 @@ export class Axis extends WidgetView { } } - guess_tick_format(ticks?: any[]) { + guess_tick_format() { if (isDateScale(this.axis_scale) || isDateColorScale(this.axis_scale)) { - return this.date_sc_format(ticks); + return this.date_sc_format(); } else if ( isLinearScale(this.axis_scale) || isColorScale(this.axis_scale) ) { - return this.linear_sc_format(ticks); + return this.linear_sc_format(); } else if (isLogScale(this.axis_scale)) { - return this.log_sc_format(ticks); + return this.log_sc_format(); } } diff --git a/js/src/ColorAxis.ts b/js/src/ColorAxis.ts index 6117e78ef..70caad7c9 100644 --- a/js/src/ColorAxis.ts +++ b/js/src/ColorAxis.ts @@ -40,6 +40,8 @@ class ColorBar extends Axis { const that = this; return scale_promise.then(() => { that.create_listeners(); + this.create_axis(); + this.set_tick_values(); that.tick_format = that.generate_tick_formatter(); that.set_scales_range(); that.append_axis(); @@ -70,7 +72,7 @@ class ColorBar extends Axis { ); } - update_display() { + create_axis(): void { this.side = this.model.get('side'); this.vertical = this.model.get('orientation') === 'vertical'; if (this.vertical) { @@ -104,7 +106,11 @@ class ColorBar extends Axis { > as d3.AxisScale ); } + } + + update_display() { this.g_axisline.remove(); + this.create_axis(); this.g_axisline = this.d3el .select('#colorBarG' + this.cid) .append('g') @@ -455,7 +461,9 @@ class ColorBar extends Axis { transform = 'translate(0, ' + (this.side === 'top' ? 0 : this.bar_height) + ')'; } - this.g_axisline.attr('transform', transform).call(this.axis); + if (this.g_axisline) { + this.g_axisline.attr('transform', transform).call(this.axis); + } } }