[Lens] Allow user to drag and select a subset of the timeline in the chart (aka brush interaction) #62636

mbondyra · 2020-04-06T16:02:55Z

Summary

Closes #58513

types
check edgecases
add tests

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

elasticmachine · 2020-04-06T16:03:38Z

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

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

mbondyra · 2020-04-08T12:21:17Z

After talking to @ppisljar I've created this issue with potential refactoring we could benefit from: #62936

mbondyra · 2020-04-08T12:30:02Z

@elasticmachine merge upstream

wylieconlon

Haven't tested this yet, but I left a few comments about the structure of the code.

wylieconlon · 2020-04-08T18:27:52Z

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

+          }
+
+          const table = data.tables[layers[0].layerId];
+          const xAxisFieldName: string | undefined = xAxisColumn?.meta?.aggConfigParams?.field;


By accessing the aggConfigParams property of the table, you are breaking one of the rules that we decided to stick with in Lens: visualizations aren't allowed to write code that is specific to a single datasource. The way we usually would write this code as a result is that instead of checking for aggConfigParams or the date_histogram function, you would check for xScaleType === 'time' or dataType === 'date'

@wylieconlon but this is the information we need to pass timeFieldName, so the filter will be applied to time picker and not as a regular filter. I don't see any other way to get it. We do the same thing onClick. Do you think we should extend configuration and add xAxisFieldName?

as discussed in the meeting this should be fine as we will be moving field information off the aggConfigParams directly to meta and is something that can be provided by most datasources.

wylieconlon · 2020-04-08T18:28:33Z

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

+          // TODO: simplify the context structure: https://github.com/elastic/kibana/issues/62936
+          const context: EmbeddableVisTriggerContext = {
+            data: {
+              range: [moment(min).toISOString(), moment(max).toISOString()],


I think you need to set the timezone explicitly here, right?

I actually checked for different timezones and it works correctly right now. How would the potential timezone setting look like?

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

wylieconlon · 2020-04-08T18:33:05Z

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

+            },
+          },
+          formatHint: { id: 'date', params: { pattern: 'HH:mm' } },
+        },


Left this comment in a few places, but it will hopefully simplify this test too: we should not use the table metadata, because it's specific to only one data source.

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

wylieconlon · 2020-04-13T19:37:34Z

@elasticmachine merge upstream

wylieconlon · 2020-04-14T21:03:52Z

@mbondyra I just pushed an update to this PR because I had already made some tweaks locally, and couldn't find a way to submit a PR to your fork. Here is the reasoning behind what I've changed:

The behavior in your PR was only correct for the primary time field of the index pattern. While we do have some issues with secondary time fields, it's important that we don't make them worse.
The mechanism you were using in the PR was to expose the name of the sourceField to visualization: we decided early that this was not something visualizations should have access to, and I don't think it's needed to resolve this behavior.

The new behavior that I've pushed up has each datasource pass the primaryTimeFieldName, a concept that all of our index patterns have. If the X axis is using a different field, then a range filter is added to the filter bar instead of changing the time picker.

GIF of new behavior:

wylieconlon

Behavior now LGTM with the tweaks I made. Tested in scenarios including multiple layers and non-primary time fields. The best dataset for this is actually metricbeat run locally, because it can track the start time of processes.

…-on-lens # Conflicts: # x-pack/legacy/plugins/lens/public/services.ts # x-pack/legacy/plugins/lens/public/xy_visualization/services.ts # x-pack/plugins/lens/public/xy_visualization/index.ts # x-pack/plugins/lens/public/xy_visualization/services.ts # x-pack/plugins/lens/public/xy_visualization/xy_expression.tsx

wylieconlon

The behavior does something that we don't currently support, which is that the time picker is always updated for time fields. I'm approving with the understanding that we will bind all time fields to the time picker in Lens in the near future, which means this behavior is ready for that.

wylieconlon · 2020-04-17T19:06:31Z

x-pack/plugins/lens/public/editor_frame_service/embeddable/embeddable.tsx

-
+        return [VIS_EVENT_TO_TRIGGER.filter, VIS_EVENT_TO_TRIGGER.brush];
+      // case 'lnsDatatable':
+      //   return [VIS_EVENT_TO_TRIGGER.filter];


Commented code?

removing commented code

mbondyra · 2020-04-20T10:16:58Z

@elasticmachine merge upstream

AlonaNadler · 2020-04-20T15:46:45Z

Hey great job!!
few things:

when I have two layers and using both layers time stamp, I brush the chart to zoom in and them using the time picker I move to a longer time range. It works oddly. it added a filter pill, the filter pill still focuses on the brush time range so changes I make in the time picker don't apply. I don't think it should add a filter pill.
Sometimes a filter pill is added sometimes it doesn't

Other than that works very nice ++

…-on-lens # Conflicts: # src/plugins/embeddable/public/lib/triggers/triggers.ts # x-pack/plugins/lens/public/xy_visualization/xy_expression.tsx

mbondyra · 2020-04-24T11:37:54Z

@elasticmachine merge upstream

mbondyra · 2020-04-27T13:56:48Z

@elasticmachine merge upstream

ppisljar

code LGTM

…-on-lens # Conflicts: # x-pack/plugins/lens/public/xy_visualization/xy_expression.tsx

kibanamachine · 2020-04-30T18:09:25Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 674564a

History

💔 Build #44400 failed 6c94c86
💔 Build #44382 failed a3491ee
💔 Build #44360 failed 19ae66f
💚 Build #43381 succeeded 793aaa1
💚 Build #43050 succeeded 0919adc

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

…chart (aka brush interaction) (elastic#62636) * feat: brushing basic example for time histogram * test: added * refactor: simplify the structure * refactor: move to inline function * refactor * refactor * Always use time field from index pattern * types * use the meta.aggConfigParams for timefieldName * fix: test snapshot update * Update embeddable.tsx removing commented code * fix: moment remov * fix: corrections for adapting to timepicker on every timefield * fix: fix single bar condition * types Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Wylie Conlon <wylieconlon@gmail.com>

…chart (aka brush interaction) (#62636) (#64992) * feat: brushing basic example for time histogram * test: added * refactor: simplify the structure * refactor: move to inline function * refactor * refactor * Always use time field from index pattern * types * use the meta.aggConfigParams for timefieldName * fix: test snapshot update * Update embeddable.tsx removing commented code * fix: moment remov * fix: corrections for adapting to timepicker on every timefield * fix: fix single bar condition * types Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Wylie Conlon <wylieconlon@gmail.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Wylie Conlon <wylieconlon@gmail.com>

feat: brushing basic example for time histogram

c213932

mbondyra requested review from wylieconlon and timroes April 6, 2020 16:02

mbondyra added Feature:Lens Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.8 v7.8.0 v8.0.0 labels Apr 6, 2020

mbondyra changed the title ~~[Lens] Brushing filter~~ [Lens] Allow user to drag and select a subset of the timeline in the chart (aka brush interaction) Apr 6, 2020

rayafratkina removed the v7.8 label Apr 6, 2020

test: added

8c99c2d

botelastic bot added the Feature:Embedding Embedding content via iFrame label Apr 7, 2020

timroes reviewed Apr 7, 2020

View reviewed changes

x-pack/legacy/plugins/lens/public/xy_visualization/xy_expression.tsx Outdated Show resolved Hide resolved

refactor: simplify the structure

0e83e13

mbondyra marked this pull request as ready for review April 8, 2020 12:02

mbondyra requested a review from a team April 8, 2020 12:02

mbondyra requested a review from a team as a code owner April 8, 2020 12:02

mbondyra mentioned this pull request Apr 8, 2020

Simplify context needed for SELECT_RANGE_TRIGGER action #62936

Closed

Merge branch 'master' into brushing-on-lens

e868030

mbondyra added the release_note:enhancement label Apr 8, 2020

refactor: move to inline function

4ff7e63

wylieconlon suggested changes Apr 8, 2020

View reviewed changes

refactor

e984281

elasticmachine and others added 2 commits April 13, 2020 15:37

Merge branch 'master' into brushing-on-lens

5806bd6

refactor

8e23141

wylieconlon approved these changes Apr 14, 2020

View reviewed changes

mbondyra added 4 commits April 15, 2020 10:36

types

b162d60

use the meta.aggConfigParams for timefieldName

f6228cb

fix: test snapshot update

3b59c4b

wylieconlon approved these changes Apr 17, 2020

View reviewed changes

Update embeddable.tsx

61695ad

removing commented code

Merge branch 'master' into brushing-on-lens

789baf1

mbondyra added 2 commits April 22, 2020 13:08

Merge commit 'ffc8741da2ffcf4d31ae3ccc143fd483a61cbb1b' into brushing…

006c9cf

…-on-lens # Conflicts: # src/plugins/embeddable/public/lib/triggers/triggers.ts # x-pack/plugins/lens/public/xy_visualization/xy_expression.tsx

fix: moment remov

7ce6088

Merge branch 'master' into brushing-on-lens

0919adc

mbondyra requested a review from flash1293 April 24, 2020 11:38

Merge branch 'master' into brushing-on-lens

793aaa1

ppisljar approved these changes Apr 29, 2020

View reviewed changes

mbondyra added 4 commits April 30, 2020 10:45

Merge commit '6e2691358fdb59af717a9aaa2b32887b9cdf2d2a' into brushing…

19ae66f

…-on-lens # Conflicts: # x-pack/plugins/lens/public/xy_visualization/xy_expression.tsx

fix: corrections for adapting to timepicker on every timefield

a3491ee

fix: fix single bar condition

6c94c86

types

674564a

mbondyra merged commit 5887c97 into elastic:master Apr 30, 2020

wylieconlon mentioned this pull request Apr 30, 2020

Update dependency @elastic/charts to v19.1.2 #64759

Merged

mbondyra mentioned this pull request May 1, 2020

[7.x] [Lens] Allow user to drag and select a subset of the timeline in the chart (aka brush interaction) (#62636) #64992

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Lens] Allow user to drag and select a subset of the timeline in the chart (aka brush interaction) #62636

[Lens] Allow user to drag and select a subset of the timeline in the chart (aka brush interaction) #62636

mbondyra commented Apr 6, 2020 •

edited

Loading

elasticmachine commented Apr 6, 2020

mbondyra commented Apr 8, 2020

mbondyra commented Apr 8, 2020

wylieconlon left a comment

wylieconlon Apr 8, 2020

mbondyra Apr 9, 2020 •

edited

Loading

ppisljar Apr 29, 2020

wylieconlon Apr 8, 2020

mbondyra Apr 9, 2020

wylieconlon Apr 8, 2020

wylieconlon commented Apr 13, 2020

wylieconlon commented Apr 14, 2020

wylieconlon left a comment

wylieconlon left a comment

wylieconlon Apr 17, 2020

mbondyra commented Apr 20, 2020

AlonaNadler commented Apr 20, 2020

mbondyra commented Apr 24, 2020

mbondyra commented Apr 27, 2020

ppisljar left a comment

kibanamachine commented Apr 30, 2020

[Lens] Allow user to drag and select a subset of the timeline in the chart (aka brush interaction) #62636

[Lens] Allow user to drag and select a subset of the timeline in the chart (aka brush interaction) #62636

Conversation

mbondyra commented Apr 6, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Apr 6, 2020

mbondyra commented Apr 8, 2020

mbondyra commented Apr 8, 2020

wylieconlon left a comment

Choose a reason for hiding this comment

wylieconlon Apr 8, 2020

Choose a reason for hiding this comment

mbondyra Apr 9, 2020 • edited Loading

Choose a reason for hiding this comment

ppisljar Apr 29, 2020

Choose a reason for hiding this comment

wylieconlon Apr 8, 2020

Choose a reason for hiding this comment

mbondyra Apr 9, 2020

Choose a reason for hiding this comment

wylieconlon Apr 8, 2020

Choose a reason for hiding this comment

wylieconlon commented Apr 13, 2020

wylieconlon commented Apr 14, 2020

wylieconlon left a comment

Choose a reason for hiding this comment

wylieconlon left a comment

Choose a reason for hiding this comment

wylieconlon Apr 17, 2020

Choose a reason for hiding this comment

mbondyra commented Apr 20, 2020

AlonaNadler commented Apr 20, 2020

mbondyra commented Apr 24, 2020

mbondyra commented Apr 27, 2020

ppisljar left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 30, 2020

💚 Build Succeeded

History

mbondyra commented Apr 6, 2020 •

edited

Loading

mbondyra Apr 9, 2020 •

edited

Loading