Fixup weekly and quarterly period label positioning and with rangebreaks

[archmoj]

Fixes #5062.
cc #4911 and #4993

Before vs After

@plotly/plotly_js

[alexcjohnson]

I forget, did we already shift the ticks to look like they're right at the end of the rangebreak? If so (please confirm) this seems like it'll make sense for 5-day work weeks regardless of which two days were removed and whether weeks start on Sunday or Monday. But there are still a few cases I'm worried about:

• removing just one weekend day (eg Saturday is still included, just Sunday is removed)
• weekend AND nightly breaks - the shift wouldn't be correct, and could even result in a broken tick because it lands within another rangebreak
• holidays that shorten to a 4-day work week

The only thing I can think of is perhaps figure out the actual range of the week between A and B, go to the midpoint of THAT, and then check if the result is in a break and needs further shifting. Is that possible?

[archmoj]

I forget, did we already shift the ticks to look like they're right at the end of the rangebreak? If so (please confirm)

Yes. The logic for setting rangebreaks labels is just above this part:

plotly.js/src/plots/cartesian/axes.js

Lines 640 to 682 in 8928851

	if(ax.rangebreaks) {
	// replace ticks inside breaks that would get a tick
	// and reduce ticks
	var len = tickVals.length;
	if(len) {
	var tf = 0;
	if(ax.tickmode === 'auto') {
	tf =
	(ax._id.charAt(0) === 'y' ? 2 : 6) *
	(ax.tickfont ? ax.tickfont.size : 12);
	}
	var newTickVals = [];
	var prevPos;
	var dir = axrev ? 1 : -1;
	var first = axrev ? 0 : len - 1;
	var last = axrev ? len - 1 : 0;
	for(var q = first; dir * q <= dir * last; q += dir) {
	var tickVal = tickVals[q];
	if(ax.maskBreaks(tickVal.value) === BADNUM) {
	tickVal.value = moveOutsideBreak(tickVal.value, ax);
	if(ax._rl && (
	ax._rl[0] === tickVal.value ||
	ax._rl[1] === tickVal.value
	)) continue;
	}
	var pos = ax.c2p(tickVal.value);
	if(pos === prevPos) {
	if(newTickVals[newTickVals.length - 1].value < tickVal.value) {
	newTickVals[newTickVals.length - 1] = tickVal;
	}
	} else if(prevPos === undefined || Math.abs(pos - prevPos) > tf) {
	prevPos = pos;
	newTickVals.push(tickVal);
	}
	}
	tickVals = newTickVals.reverse();
	}
	}

[archmoj]

there are still a few cases I'm worried about:

• removing just one weekend day (eg Saturday is still included, just Sunday is removed)
• weekend AND nightly breaks - the shift wouldn't be correct, and could even result in a broken tick because it lands within another rangebreak
• holidays that shorten to a 4-day work week

The only thing I can think of is perhaps figure out the actual range of the week between A and B, go to the midpoint of THAT, and then check if the result is in a break and needs further shifting. Is that possible?

Good call. I am working on a commit to help fix these cases.

[archmoj]

Addressed in f907f2a.

[alexcjohnson]

Weekend AND nightly breaks is still a problem: https://codepen.io/alexcjohnson/pen/yLOVjJy?editors=0010

If you want to make this a separate issue and address it later, that's fine by me. If you want to go that way please file the issue and then I'll resolve this comment.

[archmoj]

Weekend AND nightly breaks is still a problem: https://codepen.io/alexcjohnson/pen/yLOVjJy?editors=0010

Good catch. This one addressed in 50a3451.
Here is a codepen.

[alexcjohnson]

Do we need an else periodLength = ONEAVGQUARTER here, to cover the case of quarter tickformat but longer dtick?

[archmoj]

That's the case I thought it could help.

[archmoj]

At first I thought we need an else blocks for weeks and quarters to make them similar to others.
But ( as you mentioned) since we don't generate auto labels for weeks and quarters that could lead to the problem of moving daily/monthly labels by a week/quarter. So those else blocks were removed in beed352 commit.

[archmoj]

Do we need an else periodLength = ONEAVGQUARTER here, to cover the case of quarter tickformat but longer dtick?

No. Adding the else condition would result in wrong label positioning for the case of quarters.
So I think we could resolve this.

[alexcjohnson]

Here's the case I think this else would address: quarter period ticklabels, and dtick of 6 months or 1 year https://codepen.io/alexcjohnson/pen/abNBYxZ?editors=0010
Currently this puts the quarter label right on the tick:

but just like when we have monthly labels and 3-month dtick and we shift the label half a month over anyway, we should still be shifting the label half a quarter in this case. If, in this codepen, you change dtick to "M3", exactly a quarter, then it does shift the label correctly:

Note: if you turn rangebreaks back on in that codepen (notice I renamed to rangebreaksx to disable them) and disable the explicit dtick, then all manner of bad things happen as you zoom and pan - unpredictable and nonuniform tick spacing, strange jumps...

[archmoj]

Here's the case I think this else would address: quarter period ticklabels, and dtick of 6 months or 1 year https://codepen.io/alexcjohnson/pen/abNBYxZ?editors=0010
Currently this puts the quarter label right on the tick:

but just like when we have monthly labels and 3-month dtick and we shift the label half a month over anyway, we should still be shifting the label half a quarter in this case.

Good catch. This one fixed by 350a10a.
Here is an updated codepen.

[archmoj]

Note: if you turn rangebreaks back on in that codepen (notice I renamed to rangebreaksx to disable them) and disable the explicit dtick, then all manner of bad things happen as you zoom and pan - unpredictable and nonuniform tick spacing, strange jumps...

Now I also put rangebreaks back in the above pen; and it looks OK.

[alexcjohnson]

Good progress here! You're right, I like the use of hours as periods rather than instants - and I'd totally forgotten about AM/PM, that's also pretty cool!

I'm still seeing a couple of oddities. First, sometimes the tick label right before a rangebreak seems to not get the appropriate half-period shift (in this case 2015-12-18 PM, but I've seen this with day and hour tickformats too):
https://codepen.io/alexcjohnson/pen/yLOzLbO?editors=0010

Second, when I zoom out a bit on your original codepen, at some point just the week-53 tick disappears even though dtick didn't change.
https://codepen.io/alexcjohnson/pen/KKzXKXV?editors=0010

The first one I'd call a blocker; the second would be nice to fix but could be deferred for a later bugfix.

[archmoj]

I'm still seeing a couple of oddities. First, sometimes the tick label right before a rangebreak seems to not get the appropriate half-period shift (in this case 2015-12-18 PM, but I've seen this with day and hour tickformats too):
https://codepen.io/alexcjohnson/pen/yLOzLbO?editors=0010

The first one I'd call a blocker; the second would be nice to fix but could be deferred for a later bugfix.

@alexcjohnson
Good call. Addressed in c080bc5.
Here is the codepen.

[alexcjohnson]

Addressed in c080bc5.

That's good for the case when dtick matches the period, but it doesn't work right when dtick is a multiple of the period - ie if I zoom out that pen to 1-day ticks - then 2015-12-18 AM is shifted TOO far.
https://codepen.io/alexcjohnson/pen/MWyEYXX?editors=0010

[archmoj]

Addressed in c080bc5.

That's good for the case when dtick matches the period, but it doesn't work right when dtick is a multiple of the period - ie if I zoom out that pen to 1-day ticks - then 2015-12-18 AM is shifted TOO far.
https://codepen.io/alexcjohnson/pen/MWyEYXX?editors=0010

@alexcjohnson
Good catch. Addressed in 4c10ddd.
updated codepen

[alexcjohnson]

💃 Still some things that don't quite line up right, but I don't think there's anything plainly broken or misleading anymore. After we merge I'll make a new bug report collecting the remaining issues.