From d560f9dbadbf7bf6af64350831d3ffb7b11639c3 Mon Sep 17 00:00:00 2001
From: Ben McCann <322311+benmccann@users.noreply.github.com>
Date: Fri, 3 Apr 2020 15:12:34 -0700
Subject: [PATCH] Fix overlapping ticks on log scale (#7242)

---
 docs/getting-started/v3-migration.md  |   2 +-
 src/core/core.ticks.js                | 125 +++++++++++++++-----------
 src/scales/scale.logarithmic.js       |   2 +-
 test/specs/scale.logarithmic.tests.js |   2 +-
 4 files changed, 75 insertions(+), 56 deletions(-)

diff --git a/docs/getting-started/v3-migration.md b/docs/getting-started/v3-migration.md
index 7a78f38fe..dddab3924 100644
--- a/docs/getting-started/v3-migration.md
+++ b/docs/getting-started/v3-migration.md
@@ -155,7 +155,7 @@ Animation system was completely rewritten in Chart.js v3. Each property can now
 #### Ticks
 
 * `options.ticks.major` and `options.ticks.minor` were replaced with scriptable options for tick fonts.
-* `Chart.Ticks.formatters.linear` and `Chart.Ticks.formatters.logarithmic` were replaced with `Chart.Ticks.formatters.numeric`.
+* `Chart.Ticks.formatters.linear` was renamed to `Chart.Ticks.formatters.numeric`.
 
 #### Tooltip
 
diff --git a/src/core/core.ticks.js b/src/core/core.ticks.js
index 1b82e4477..46bfc75bf 100644
--- a/src/core/core.ticks.js
+++ b/src/core/core.ticks.js
@@ -2,62 +2,81 @@ import {isArray} from '../helpers/helpers.core';
 import {log10} from '../helpers/helpers.math';
 
 /**
- * Namespace to hold static tick generation functions
- * @namespace Chart.Ticks
+ * Namespace to hold formatters for different types of ticks
+ * @namespace Chart.Ticks.formatters
  */
-export default {
+const formatters = {
+	/**
+	 * Formatter for value labels
+	 * @method Chart.Ticks.formatters.values
+	 * @param value the value to display
+	 * @return {string|string[]} the label to display
+	 */
+	values(value) {
+		return isArray(value) ? value : '' + value;
+	},
+
 	/**
-	 * Namespace to hold formatters for different types of ticks
-	 * @namespace Chart.Ticks.formatters
+	 * Formatter for numeric ticks
+	 * @method Chart.Ticks.formatters.numeric
+	 * @param tickValue {number} the value to be formatted
+	 * @param index {number} the position of the tickValue parameter in the ticks array
+	 * @param ticks {object[]} the list of ticks being converted
+	 * @return {string} string representation of the tickValue parameter
 	 */
-	formatters: {
-		/**
-		 * Formatter for value labels
-		 * @method Chart.Ticks.formatters.values
-		 * @param value the value to display
-		 * @return {string|string[]} the label to display
-		 */
-		values(value) {
-			return isArray(value) ? value : '' + value;
-		},
-
-		/**
-		 * Formatter for numeric ticks
-		 * @method Chart.Ticks.formatters.numeric
-		 * @param tickValue {number} the value to be formatted
-		 * @param index {number} the position of the tickValue parameter in the ticks array
-		 * @param ticks {object[]} the list of ticks being converted
-		 * @return {string} string representation of the tickValue parameter
-		 */
-		numeric(tickValue, index, ticks) {
-			if (tickValue === 0) {
-				return '0'; // never show decimal places for 0
-			}
-
-			// If we have lots of ticks, don't use the ones
-			let delta = ticks.length > 3 ? ticks[2].value - ticks[1].value : ticks[1].value - ticks[0].value;
-
-			// If we have a number like 2.5 as the delta, figure out how many decimal places we need
-			if (Math.abs(delta) > 1 && tickValue !== Math.floor(tickValue)) {
-				// not an integer
-				delta = tickValue - Math.floor(tickValue);
-			}
-
-			const logDelta = log10(Math.abs(delta));
-
-			const maxTick = Math.max(Math.abs(ticks[0].value), Math.abs(ticks[ticks.length - 1].value));
-			const minTick = Math.min(Math.abs(ticks[0].value), Math.abs(ticks[ticks.length - 1].value));
-			const locale = this.chart.options.locale;
-			if (maxTick < 1e-4 || minTick > 1e+7) { // all ticks are small or big numbers; use scientific notation
-				const logTick = log10(Math.abs(tickValue));
-				let numExponential = Math.floor(logTick) - Math.floor(logDelta);
-				numExponential = Math.max(Math.min(numExponential, 20), 0);
-				return tickValue.toExponential(numExponential);
-			}
-
-			let numDecimal = -1 * Math.floor(logDelta);
-			numDecimal = Math.max(Math.min(numDecimal, 20), 0); // toFixed has a max of 20 decimal places
-			return new Intl.NumberFormat(locale, {minimumFractionDigits: numDecimal, maximumFractionDigits: numDecimal}).format(tickValue);
+	numeric(tickValue, index, ticks) {
+		if (tickValue === 0) {
+			return '0'; // never show decimal places for 0
+		}
+
+		// If we have lots of ticks, don't use the ones
+		let delta = ticks.length > 3 ? ticks[2].value - ticks[1].value : ticks[1].value - ticks[0].value;
+
+		// If we have a number like 2.5 as the delta, figure out how many decimal places we need
+		if (Math.abs(delta) > 1 && tickValue !== Math.floor(tickValue)) {
+			// not an integer
+			delta = tickValue - Math.floor(tickValue);
+		}
+
+		const logDelta = log10(Math.abs(delta));
+
+		const maxTick = Math.max(Math.abs(ticks[0].value), Math.abs(ticks[ticks.length - 1].value));
+		const minTick = Math.min(Math.abs(ticks[0].value), Math.abs(ticks[ticks.length - 1].value));
+		const locale = this.chart.options.locale;
+		if (maxTick < 1e-4 || minTick > 1e+7) { // all ticks are small or big numbers; use scientific notation
+			const logTick = log10(Math.abs(tickValue));
+			let numExponential = Math.floor(logTick) - Math.floor(logDelta);
+			numExponential = Math.max(Math.min(numExponential, 20), 0);
+			return tickValue.toExponential(numExponential);
 		}
+
+		let numDecimal = -1 * Math.floor(logDelta);
+		numDecimal = Math.max(Math.min(numDecimal, 20), 0); // toFixed has a max of 20 decimal places
+		return new Intl.NumberFormat(locale, {minimumFractionDigits: numDecimal, maximumFractionDigits: numDecimal}).format(tickValue);
 	}
 };
+
+/**
+ * Formatter for logarithmic ticks
+ * @method Chart.Ticks.formatters.logarithmic
+ * @param tickValue {number} the value to be formatted
+ * @param index {number} the position of the tickValue parameter in the ticks array
+ * @param ticks {object[]} the list of ticks being converted
+ * @return {string} string representation of the tickValue parameter
+ */
+formatters.logarithmic = function(tickValue, index, ticks) {
+	if (tickValue === 0) {
+		return '0';
+	}
+	const remain = tickValue / (Math.pow(10, Math.floor(log10(tickValue))));
+	if (remain === 1 || remain === 2 || remain === 5) {
+		return formatters.numeric.call(this, tickValue, index, ticks);
+	}
+	return '';
+};
+
+/**
+ * Namespace to hold static tick generation functions
+ * @namespace Chart.Ticks
+ */
+export default {formatters};
diff --git a/src/scales/scale.logarithmic.js b/src/scales/scale.logarithmic.js
index 2c750c059..26ca37f7b 100644
--- a/src/scales/scale.logarithmic.js
+++ b/src/scales/scale.logarithmic.js
@@ -50,7 +50,7 @@ function generateTicks(generationOptions, dataRange) {
 const defaultConfig = {
 	// label settings
 	ticks: {
-		callback: Ticks.formatters.numeric,
+		callback: Ticks.formatters.logarithmic,
 		major: {
 			enabled: true
 		}
diff --git a/test/specs/scale.logarithmic.tests.js b/test/specs/scale.logarithmic.tests.js
index 323084a9e..c4ce945cc 100644
--- a/test/specs/scale.logarithmic.tests.js
+++ b/test/specs/scale.logarithmic.tests.js
@@ -679,7 +679,7 @@ describe('Logarithmic Scale tests', function() {
 			}
 		});
 
-		expect(getLabels(chart.scales.y)).toEqual(['80', '70', '60', '50', '40', '30', '20', '10', '9', '8', '7', '6', '5', '4', '3', '2', '1']);
+		expect(getLabels(chart.scales.y)).toEqual(['', '', '', '50', '', '', '20', '10', '', '', '', '', '5', '', '', '2', '1']);
 	});
 
 	it('should build labels using the user supplied callback', function() {
-- 
2.47.2