Set unified hovermode via template

[archmoj]

Addressing #4667 i.e. to set unified hovermode via template.
Commit 2c76d79 fixes the bug.
Other commits make helper functions to check hovermode.

@plotly/plotly_js

[alexcjohnson]

Does it not work with just layoutOut.hovermode? Do we need the layoutIn fallback for some specific case?

[archmoj]

Some tests failed without the layoutIn fall back.

[alexcjohnson]

If this is just unit tests (ie we're not running the entire supplyDefaults pipeline so earlier steps in that should have set layoutOut.hovermode were not called, then I'd rather change the test than include this fallback. If there's a case where the full pipeline does not behave correctly then this worries me, as that case presumably would not work with templates.

[archmoj]

hovermode coerce with auto defaults depending on cartesian is

plotly.js/src/components/fx/layout_defaults.js

Lines 36 to 44 in 6de4c75

	varhoverMode=coerce('hovermode',hovermodeDflt);
	if(hoverMode){
	vardflt;
	if(['x unified','y unified'].indexOf(hoverMode)!==-1){
	dflt=-1;
	}
	coerce('hoverdistance');
	coerce('spikedistance',dflt);
	}

called at the end of pipeline here:

plotly.js/src/plots/plots.js

Lines 1672 to 1678 in 6de4c75

	for(componentincomponentsRegistry){
	_module=componentsRegistry[component];
	if(_module.supplyLayoutDefaults){
	_module.supplyLayoutDefaults(layoutIn,layoutOut,fullData);
	}
	}

I am not sure how to proceed if we want to remove layoutIn.hovermode fallback.

[archmoj]

On another note - here is test which is failing, if we remove layoutIn fallback:

plotly.js/test/jasmine/tests/hover_label_test.js

Lines 4037 to 4052 in 6de4c75

	it('set smart defaults for spikeline in x unified',function(done){
	Plotly.newPlot(gd,[{y: [4,6,5]}],{'hovermode': 'x unified','xaxis': {'color': 'red'}})
	.then(function(gd){
	expect(gd._fullLayout.hovermode).toBe('x unified');
	varax=gd._fullLayout.xaxis;
	expect(ax.showspike).toBeTrue;
	expect(ax.spikemode).toBe('across');
	expect(ax.spikethickness).toBe(1.5);
	expect(ax.spikedash).toBe('dot');
	expect(ax.spikecolor).toBe('red');
	expect(ax.spikesnap).toBe('hovered data');
	expect(gd._fullLayout.yaxis.showspike).toBeFalse;
	})
	.catch(failTest)
	.then(done);
	});

[alexcjohnson]

If that test is failing without the layoutIn fallback, it means that spikes will not be properly enabled by hovermode set in the template, right?

So I guess the code here is called by the basePlotModules loop:

plotly.js/src/plots/plots.js

Lines 1639 to 1647 in 6de4c75

	// base plot module layout defaults
	for(i=0;i<basePlotModules.length;i++){
	_module=basePlotModules[i];
	// e.g. pie does not have a layout-defaults step
	if(_module.supplyLayoutDefaults){
	_module.supplyLayoutDefaults(layoutIn,layoutOut,fullData);
	}
	}

I think the easiest thing to do would be to move hovermode coercion here, right at the top of this function. That will involve making a special coerce function using fx/layout_attributes, but it will ensure the ordering is correct.

[archmoj]

@alexcjohnson Good call. Revised in the commits below.

[alexcjohnson]

nonblocking, but at one point these names were useful in stack traces. That may or may not not be the case anymore, based on changes to our build and sourcemap processes... but historically that was the reason for names like this that are otherwise unused.

[alexcjohnson]

We talked about short-circuiting this one, right? something like:

varhoverMode=layoutOut.hovermode;if(hoverMode===undefined){hoverMode=handleHoverModeDefaults(layoutIn,layoutOut,fullData);}

Is that a possibility? Or would that not work for some reason?

[archmoj]

hovermode and clickmode use special coerce function which returns previously coerced value.

plotly.js/src/components/fx/hovermode_defaults.js

Lines 14 to 20 in abf3a55

	module.exports=functionhandleHoverModeDefaults(layoutIn,layoutOut,fullData){
	functioncoerce(attr,dflt){
	// don't coerce if it is already coerced in other place e.g. in cartesian defaults
	if(layoutOut[attr]!==undefined)returnlayoutOut[attr];
	returnLib.coerce(layoutIn,layoutOut,layoutAttributes,attr,dflt);
	}

[alexcjohnson]

Looks great. Just one perf comment #4669 (comment) but that's not blocking. Let's do it! 💃