[Lens] Multiple y axes #69911

flash1293 · 2020-06-25T12:06:17Z

This PR adds the ability to configure two separate y axes in XY charts.

These changes are limited to the xy visualization, expect for one small fix in x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx - there is a flag enableDimensionEditor on the dimension group whether to show a second visualization-controlled tab in the dimension editor, but it wasn't used to actually determine whether to show the editor or not.

Axis assignment

The process of assigning metric to axes follows this algorithm:

Split up all metrics by their type (auto/left/right) - this results in three lists
Move all "auto" metrics to either left or right:
- If a metrics formatter is compatible with all formatters of other left axis metrics, it becomes a left axis metric
- If a metrics formatter is compatible with all formatters of other right axis metrics, it becomes a right axis metric
- Otherwise it will be added to whatever axis has fewer metrics assigned
If the left/right axis list is not empty, take the formatter of the first metric in the list as the formatter for the axis
Assign all metrics of the left/right axis list to the same group

A formatter is compatible if it has the same type, e.g. number or bytes.

There are other ways of handling this (e.g. displaying multiple axes on a single side if the formatters don't match), but all approaches (including the chosen one) have non-optimal edge cases. These can only be really fixed by providing a more sophisticated way of editing axes which would be out of scope for this PR. We can add this in a later iteration by providing a separate axis management menu in the toolbar.

Subtle changes

Prior to this, an axis with non-matching formatters would simply fall back to a plain number formatter. This is confusing in a lot of cases (e.g. [Lens] Field Formatters not working for multiple layers #68260), so this PR changes the logic by just using the first one if the case can't be solved by using two y axes
There is a separate yTitle and xTitle argument on the expression to label the axes - so far xTitle was only used when there was no column name provided by the data source (never in practice). As we can have multiple y axes now as well, by default the names of the participating columns are used, with the yTitle as fallback (like it's happening for xTitle).

Expression integration

Currently y axis series are specified by listing out the columns in the accessors argument. To stay compatible with previous versions and not introduce too much new structure, this PR simply adds an yAxisConfiguration argument that can be specified multiple times, setting the axis mode to either left, right or auto. As long as the user didn't select anything, no state is written and "auto" mode is assumed. If the user manually picks an axis mode, a lens_xy_yAxisConfiguration sub-expression is added:

lens_xy_chart {...} yConfig={lens_xy_yConfig forAccessor="a" axisMode="right"}

Related to that: In x-pack/plugins/lens/public/xy_visualization/types.ts there were expression functions for YState and XConfig which were unused, I removed them as part of this PR.

lens_xy_yConfig can be extended later to add things like metric color overrides.

`elastic-charts` integration

To use multiple axes, they have to be defined with a groupId, then series can be mapped to them by specifying the same groupId. Currently there is a single series element in the spec for each layer, but it's possible to have a single layer mapped to different axes. This PR makes this possible, by splitting up each layer for each y accessor, keeping all other properties the same except for the group id which is mapped for each accessor individually. That changes existing specs with more than one metric (e.g. turning one LineSeries element into multiple ones).

elasticmachine · 2020-06-26T17:57:54Z

Pinging @elastic/kibana-app (Team:KibanaApp)

wylieconlon

So far I've tested the behavior, and it's looking promising. Did something change with the way we do axis titles in this PR? I'm noticing the comma-separated titles more.

Will review code next week.

mbondyra · 2020-06-29T08:53:03Z

@elasticmachine merge upstream

…to lens/multiple-y-axes

flash1293 · 2020-06-29T12:14:47Z

@wylieconlon I added this logic, but as you are mentioning it I remembered we chose to only use the first series as label here to avoid overly long axis labels. Rolled back that change

@mbondyra Renamed the expression functions so we can add colors in there as well. Also, I fixed the bug with the unformatted value in the tooltip.

kibanamachine · 2020-06-29T16:51:31Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: c8b8ec8

Build metrics

✅ unchanged

History

💚 Build #57104 succeeded 338e0ec
💚 Build #57059 succeeded 970b08a
💔 Build #57037 failed 6550d20
💚 Build #56749 succeeded 5b48a51
💔 Build #56656 failed 9fdb807

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

mbondyra

tested on Firefox, it works great! Code looks good to me!

wylieconlon

Overall LGTM, thanks for making those changes!

wylieconlon · 2020-06-30T22:01:13Z

x-pack/plugins/lens/public/xy_visualization/xy_expression.tsx

@@ -276,6 +276,9 @@ export function XYChart({
        legendPosition={legend.position}
        showLegendExtra={false}
        theme={chartTheme}
+        tooltip={{
+          headerFormatter: (d) => xAxisFormatter.convert(d.value),


This does of course fail if there are non-matching types on the X axis, but I think we can't fix that case.

…-based-rbac * upstream/master: (38 commits) Move logger configuration integration test to jest (elastic#70378) Changes observability plugin codeowner (elastic#70439) update (elastic#70424) [Logs UI] Avoid CCS-incompatible index name resolution (elastic#70179) Enable "Explore underlying data" actions for Lens visualizations (elastic#70047) Initial work on uptime homepage API (elastic#70135) expressions indexPattern function (elastic#70315) [Discover] Deangularization context error message refactoring (elastic#70090) [Lens] Add "no data" popover (elastic#69147) [Lens] Move chart switcher over (elastic#70182) chore: add missing mjs extension (elastic#70326) [Lens] Multiple y axes (elastic#69911) skip flaky suite (elastic#70386) fix bug to add timeline to case (elastic#70343) [QA][Code Coverage] Drop catchError and use try / catch instead, (elastic#69198) [QA] [Code Coverage] Integrate with Team Assignment Pipeline and Add Research and Development Indexes and Cluster (elastic#69348) [Metrics UI] Add context.reason and alertOnNoData to Inventory alerts (elastic#70260) Resolver refactoring (elastic#70312) [Ingest Manager] Fix agent ack after input format change (elastic#70335) [eslint][ts] Enable prefer-ts-expect-error (elastic#70022) ...

flash1293 added 7 commits June 25, 2020 14:02

start implementing multiple y axes

22568a2

implement multiple y axes

199ed81

Merge remote-tracking branch 'upstream/master' into lens/multiple-y-axes

41ede6e

add unit tests for multi y axis behavior

992f930

fix some things

9fdb807

Merge remote-tracking branch 'upstream/master' into lens/multiple-y-axes

3987124

fix tests

5b48a51

flash1293 added Feature:Lens release_note:enhancement v7.9.0 v8.0.0 labels Jun 26, 2020

flash1293 marked this pull request as ready for review June 26, 2020 17:57

flash1293 requested a review from a team June 26, 2020 17:57

flash1293 added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Jun 26, 2020

flash1293 requested review from mbondyra and wylieconlon June 26, 2020 17:58

wylieconlon reviewed Jun 26, 2020

View reviewed changes

elasticmachine and others added 4 commits June 29, 2020 03:53

Merge branch 'master' into lens/multiple-y-axes

6550d20

Merge remote-tracking branch 'upstream/master' into lens/multiple-y-axes

c6801f6

rename expression functions and fix bugs

204a240

Merge branch 'lens/multiple-y-axes' of github.com:flash1293/kibana in…

970b08a

…to lens/multiple-y-axes

flash1293 added 4 commits June 29, 2020 15:10

Merge remote-tracking branch 'upstream/master' into lens/multiple-y-axes

7238f1f

remove hack and use global setting

338e0ec

Merge remote-tracking branch 'upstream/master' into lens/multiple-y-axes

6febf5d

fix expression

c8b8ec8

mbondyra approved these changes Jun 30, 2020

View reviewed changes

wylieconlon approved these changes Jun 30, 2020

View reviewed changes

flash1293 merged commit e1665e8 into elastic:master Jul 1, 2020

flash1293 mentioned this pull request Jul 1, 2020

[7.x] [Lens] Multiple y axes (#69911) #70400

Merged

flash1293 added a commit to flash1293/kibana that referenced this pull request Jul 1, 2020

[Lens] Multiple y axes (elastic#69911)

0577d49

flash1293 added a commit that referenced this pull request Jul 1, 2020

[Lens] Multiple y axes (#69911) (#70400)

4b06ef9

Bamieh pushed a commit to Bamieh/kibana that referenced this pull request Jul 1, 2020

[Lens] Multiple y axes (elastic#69911)

88210f5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Multiple y axes #69911

[Lens] Multiple y axes #69911

flash1293 commented Jun 25, 2020 •

edited

Loading

elasticmachine commented Jun 26, 2020

wylieconlon left a comment

mbondyra commented Jun 29, 2020

flash1293 commented Jun 29, 2020

kibanamachine commented Jun 29, 2020

mbondyra left a comment

wylieconlon left a comment

wylieconlon Jun 30, 2020

[Lens] Multiple y axes #69911

[Lens] Multiple y axes #69911

Conversation

flash1293 commented Jun 25, 2020 • edited Loading

Axis assignment

Subtle changes

Expression integration

elastic-charts integration

elasticmachine commented Jun 26, 2020

wylieconlon left a comment

Choose a reason for hiding this comment

mbondyra commented Jun 29, 2020

flash1293 commented Jun 29, 2020

kibanamachine commented Jun 29, 2020

💚 Build Succeeded

Build metrics

History

mbondyra left a comment

Choose a reason for hiding this comment

wylieconlon left a comment

Choose a reason for hiding this comment

wylieconlon Jun 30, 2020

Choose a reason for hiding this comment

flash1293 commented Jun 25, 2020 •

edited

Loading

`elastic-charts` integration