Introduce formatter -options for tooltips and tick labels #6037

kurkle · 2019-01-31T22:59:08Z

Couple of things going on here:

Fix scatter sample to use callback instead of legacy userCallback
Alias helpers.callback for minimization
Propose a _configure step to scale for resolving options
Propose a fallback path for formatter options in _configure
Propose _formatTick and _formatTooltip method names
Format tick labels if a callback is provided
Format value/label in TooltipItem
Add a test for multiline labels (I broke it and nothing failed)

Todo if merging is plausible:

Tests for formatter's
Docs

For testing: CodePen

src/core/core.scale.js

etimberg · 2019-01-31T23:44:15Z

src/core/core.tooltip.js

@@ -217,8 +217,8 @@ function createTooltipItem(element) {
 	return {
 		xLabel: xScale ? xScale.getLabelForIndex(index, datasetIndex) : '',
 		yLabel: yScale ? yScale.getLabelForIndex(index, datasetIndex) : '',
-		label: indexScale ? indexScale.getLabelForIndex(index, datasetIndex) : '',
-		value: valueScale ? valueScale.getLabelForIndex(index, datasetIndex) : '',
+		label: indexScale ? indexScale._formatTooltip(index, datasetIndex) : '',


Not sure about a private function here

etimberg · 2019-01-31T23:49:58Z

src/core/core.scale.js

+	/**
+	 * @private
+	 */
+	_configure: function() {


I think this is an interesting concept for supporting these options

Only things that should need an update to change can be resolved here though.

benmccann · 2019-01-31T23:53:15Z

Docs would have to be added as well

src/core/core.scale.js

benmccann · 2019-02-02T01:11:31Z

I'm wondering what the tick is when it's passed to the user-supplied formatter function. Is it a number or an object {tick: number, major: boolean}. I think we have both in different places. The latter would be useful to allow users to more easily do more advanced formatting on their own

kurkle · 2019-02-02T09:49:28Z

how hard can it be to rebase 😄

changes

core.scale:

alias globalDefaults globally
pass tick object to formatter

scale.time:

pass tick object to tickFormatFunction
use already figured tick.major in tickFormatFunction
call _formatTick in tickFormatFunction

src/core/core.scale.js

src/scales/scale.time.js

- alias globalDefaults globally - pass tick object to formatter time scale: - pass tick object to tickFormatFunction - use already figured `tick.major` in tickFormatFunction - use _formatTick in tickFormatFunction

benmccann · 2019-02-13T04:02:15Z

test/specs/core.scale.tests.js

@@ -73,6 +73,22 @@ describe('Core.scale', function() {

 			expect(lastTick(chart).label).toBeUndefined();
 		});
+
+		it('should display multiline-labels correctly', function() {


nit: "multiline labels" or "multi-line labels" would both be fine. current hyphen location is unexpected

benmccann · 2019-02-13T04:02:31Z

test/specs/core.scale.tests.js

+			});
+
+			expect(lastTick(chart).label).toEqual(['September', '2018']);
+


nit: I'd probably remove this blank line

benmccann · 2019-02-13T04:04:16Z

This looks pretty good to me. I'm wondering if there are any other tests we need? Maybe one for the new _formatTooltip method?

benmccann · 2019-02-19T15:44:40Z

src/core/core.scale.js

+		var labels = [];
+		var i, ilen, tick;
+
+		if (helpers.isArray(ticks) && ticks.length) {


It might be nicer to make whether the ticks are being passed in and whether it's the new format be independent. Can we check if the tick is an object instead?

nishantprajapti · 2019-04-10T11:55:38Z

@benmccann just want to know why have you referenced #2442 at the bottom here. Any reasons ??

benmccann · 2019-04-10T15:03:28Z

Ah, I guess this only handles textual formatting and not styling

benmccann reviewed Jan 31, 2019

View reviewed changes

src/core/core.scale.js Show resolved Hide resolved

benmccann reviewed Jan 31, 2019

View reviewed changes

src/core/core.scale.js Outdated Show resolved Hide resolved

etimberg reviewed Jan 31, 2019

View reviewed changes

benmccann reviewed Feb 2, 2019

View reviewed changes

src/core/core.scale.js Outdated Show resolved Hide resolved

kurkle force-pushed the formatter branch 4 times, most recently from 64795ee to c832c17 Compare February 2, 2019 09:42

benmccann reviewed Feb 2, 2019

View reviewed changes

src/core/core.scale.js Show resolved Hide resolved

benmccann reviewed Feb 2, 2019

View reviewed changes

src/core/core.scale.js Show resolved Hide resolved

benmccann reviewed Feb 2, 2019

View reviewed changes

src/scales/scale.time.js Outdated Show resolved Hide resolved

kurkle added 2 commits February 4, 2019 13:18

Introduce formatter -options

c69cd5d

core.scale:

d19651f

- alias globalDefaults globally - pass tick object to formatter time scale: - pass tick object to tickFormatFunction - use already figured `tick.major` in tickFormatFunction - use _formatTick in tickFormatFunction

kurkle force-pushed the formatter branch from c832c17 to d19651f Compare February 4, 2019 11:19

mark ticks as major even if major is not enabled

11a49df

benmccann reviewed Feb 13, 2019

View reviewed changes

nits

060735e

benmccann reviewed Feb 19, 2019

View reviewed changes

benmccann mentioned this pull request Feb 19, 2019

Add possibility to change style for each of the ticks #2442

Closed

kurkle closed this Apr 1, 2019

kurkle deleted the formatter branch June 12, 2020 05:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce formatter -options for tooltips and tick labels #6037

Introduce formatter -options for tooltips and tick labels #6037

kurkle commented Jan 31, 2019 •

edited

Loading

etimberg Jan 31, 2019

etimberg Jan 31, 2019

kurkle Feb 1, 2019

benmccann commented Jan 31, 2019

benmccann commented Feb 2, 2019

kurkle commented Feb 2, 2019

benmccann Feb 13, 2019

benmccann Feb 13, 2019

benmccann commented Feb 13, 2019

benmccann Feb 19, 2019

nishantprajapti commented Apr 10, 2019

benmccann commented Apr 10, 2019 •

edited

Loading

		});

		expect(lastTick(chart).label).toEqual(['September', '2018']);

Introduce formatter -options for tooltips and tick labels #6037

Introduce formatter -options for tooltips and tick labels #6037

Conversation

kurkle commented Jan 31, 2019 • edited Loading

Couple of things going on here:

Todo if merging is plausible:

For testing: CodePen

etimberg Jan 31, 2019

Choose a reason for hiding this comment

etimberg Jan 31, 2019

Choose a reason for hiding this comment

kurkle Feb 1, 2019

Choose a reason for hiding this comment

benmccann commented Jan 31, 2019

benmccann commented Feb 2, 2019

kurkle commented Feb 2, 2019

changes

benmccann Feb 13, 2019

Choose a reason for hiding this comment

benmccann Feb 13, 2019

Choose a reason for hiding this comment

benmccann commented Feb 13, 2019

benmccann Feb 19, 2019

Choose a reason for hiding this comment

nishantprajapti commented Apr 10, 2019

benmccann commented Apr 10, 2019 • edited Loading

kurkle commented Jan 31, 2019 •

edited

Loading

benmccann commented Apr 10, 2019 •

edited

Loading