De-angularize vis tooltips

[nickofthyme]

Summary

• Remove angular from vis/tooltip
• Move tooltip from vis to vis_type_vislib
• Fix errors from vislib and tests due to changes

Fix existing tooltip display 0 value issue (38a0fd7)

Before

Notice the Flight delay minutes value is 0 and therefore is filtered out.

After

Flight delay minutes value of 0 now shows in tooltip

Checklist

• This was checked for cross-browser compatibility, including a check against IE11
• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for keyboard-only and screenreader accessibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[flash1293]

Looks good go to me once green, left a few comments to look into. I didn't look into the test failures, but seems like something isn't mocked correctly there.

[flash1293]

Just realized these visualization types depend on vislib (at least for static code) - we should put them in a central place so visualizations don't depend on each other (not necessarily in this PR though). I probably missed this in the vislib shimming PR.

I saw RangeOption, SwitchOption, ColorModes, Labels, Styles and NumberInputOption.

Looks like RangeOption, SwitchOption and NumberInputOption are a thin layer on top of eui form components. @streamich Would they fit into kibana_react? They are not really chart specific.

ColorModes, Labels and Styles are types - I would say they don't really have to be shared, but if they are shared I think they should go into visualizations @ppisljar what do you think?

[nickofthyme]

Ya I wasn't sure what to do with these either.

[ppisljar]

ColorModes is a setting specific to metric visualization ... not sure about Labels and Styles, but i am guessing its also settings limited to just a few visualizations ?

[flash1293]

Let's duplicate the types and make them internal to the plugins they are used in.

For the components it seems like they do not really fit into kibana_react - let's make them a part of the new "charts" plugin for now (doesn't have to happen in this PR) - it's a bit of a different beast, but these forms are also useful for all chart types to make the forms consistent, so it's the best home for them now - they are slightly too complex to just duplicate them IMHO.

[nickofthyme]

Joe I'll update the types and components is another pr

[flash1293]

Why is this commented out? Seems like this is used.

[nickofthyme]

Yeah I couldn't tell where it was coming from. It was in the angular template but there was no metricCol property that was set on the $scope. Nor was there any other reference to metricCol in all of kibana. Is it possible to be coming from $rootScope?

kibana/src/legacy/ui/public/vis/components/tooltip/_hierarchical_tooltip_formatter.js

Lines 29 to 64 in 56482ab

	export function HierarchicalTooltipFormatterProvider($rootScope, $compile, $sce) {
	const $tooltip = $(template);
	const $tooltipScope = $rootScope.$new();
	$compile($tooltip)($tooltipScope);
	return function(metricFieldFormatter) {
	return function(event) {
	const datum = event.datum;
	// Collect the current leaf and parents into an array of values
	$tooltipScope.rows = collectBranch(datum);
	// Map those values to what the tooltipSource.rows format.
	_.forEachRight($tooltipScope.rows, function(row) {
	row.spacer = $sce.trustAsHtml(_.repeat(' ', row.depth));
	let percent;
	if (row.item.percentOfGroup != null) {
	percent = row.item.percentOfGroup;
	}
	row.metric = metricFieldFormatter ? metricFieldFormatter.convert(row.metric) : row.metric;
	if (percent != null) {
	row.metric += ' (' + numeral(percent).format('0.[00]%') + ')';
	}
	return row;
	});
	$tooltipScope.$apply();
	return $tooltip[0].outerHTML;
	};
	};
	}

[flash1293]

Seems like a leftover to me - it should be safe to remove it completely

[flash1293]

Can we do a proper check here?

      let percent = undefined;
// ...
if (percent !== null) {

This makes me nervous :)

[nickofthyme]

Same -- I was just replicating the existing logic. I'll update it.

[andreadelrio]

Sass changes LGTM

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: ebf8d00

History

• 💔 Build #21155 failed b8af4c6
• 💔 Build #21136 failed 3eb71dc
• 💔 Build #20939 failed d84a9ca
• 💔 Build #20840 failed 5facffd
• 💔 Build #20742 failed 6818f49

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream