Skip to content

[Lens] Decouple visualizations from specific operations#75703

Merged
wylieconlon merged 3 commits intoelastic:masterfrom
wylieconlon:lens/decouple-visualizations
Aug 26, 2020
Merged

[Lens] Decouple visualizations from specific operations#75703
wylieconlon merged 3 commits intoelastic:masterfrom
wylieconlon:lens/decouple-visualizations

Conversation

@wylieconlon
Copy link
Contributor

This is both cleaning up tech debt in Lens and setting us up for the date ranges and a combined number histogram/range function. The main issue is that we are coupling the data type and expected display behavior. The nice thing is that we already have a second parameter on OperationMetadata which should be used instead:

export interface OperationMetadata {
  dataType: DataType;
  scale?: 'ordinal' | 'interval' | 'ratio';

The main difference is that histograms are interval type, and other bucket aggregations are ordinal type. Here are the places that I have identified as needing to change:

  • The XY suggestion logic looks at dataType === 'date' to determine the X axis. It should instead use operationMetadata.scaleType === 'interval', so that number histograms are placed on the X axis.
  • Pie charts reject dataType === 'date' in suggestions, but instead they should reject scaleType === 'interval'
  • The XY title logic uses special logic for dataType === 'date' when building titles. It probably should
  • The handleFilterClick function on datatables is looking up the time field only for date histograms, but not date functions in general.

Closes #75702

Checklist

@wylieconlon wylieconlon added Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// Feature:Lens labels Aug 21, 2020
@wylieconlon wylieconlon requested a review from a team August 21, 2020 21:26
@elasticmachine
Copy link
Contributor

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

@wylieconlon wylieconlon added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v8.0.0 labels Aug 21, 2020
Copy link
Contributor

@mbondyra mbondyra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on Firefox, all works as before.
Code lgtm - I double-checked if there are any other places but didn't find anything more than what you've targetted already. 👌

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

page load bundle size

id value diff baseline
lens 867.2KB +90.0B 867.1KB

History

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

@wylieconlon wylieconlon merged commit 8364d8d into elastic:master Aug 26, 2020
@wylieconlon wylieconlon deleted the lens/decouple-visualizations branch August 26, 2020 22:27
wylieconlon pushed a commit that referenced this pull request Aug 27, 2020
)

* [Lens] Decouple visualizations from specific operations

* Remove unused mock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Lens release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.10.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Lens] Visualizations are tightly coupled to specific functions based on operation.dataType

4 participants