[Lens] Index pattern selection using EuiSelectable by cchaos · Pull Request #46350 · elastic/kibana

cchaos · 2019-09-23T15:59:38Z

Created a shared component for changing index pattern

There were two different implementations used in the fields panel and the layer panel. They both used EuiComboBox and swapped out the UI from text to the input. This wasn't ideal as I wanted the selection to occur in a popover with a straight list and search capability.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

This was checked for cross-browser compatibility, including a check against IE11
~~[ ] 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~~

cchaos · 2019-09-23T16:08:04Z

@flash1293 Because I see that you were the one who created these components, can you help me fix a few things?

Looks like I ran into the same EUI bug but I just closed the popover on selection of a new index pattern.
Tests need to be updated
The switcher in the layer config seems to be working, but I can't get the fields panel one to update its parent. https://d.pr/free/i/8Gpaz2

elasticmachine · 2019-09-23T17:46:15Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

flash1293 · 2019-09-24T11:41:05Z

x-pack/legacy/plugins/lens/public/indexpattern_plugin/change_indexpattern.tsx

I will open an EUI issue for this, we had the same problem with the combo box

flash1293 · 2019-09-24T12:06:23Z

@cchaos

Looks like I ran into the same EUI bug but I just closed the popover on selection of a new index pattern.

I think this is the best option at the moment. Can we push the issue a bit? Seems like quite a few people ran into it.

Tests need to be updated

Selectables in popovers are super awkward to test with our setup because of you can't mount them because enzyme doesn't support portals and you can't shallow render because then you won't get the rendered options. You can sort-of do both by shallow rendering the container, then use dive() to get the shallow contents of popover, then mount the shallow contents again in a separate enzyme wrapper to get the actually rendered options. I will take care of that tomorrow.

The switcher in the layer config seems to be working, but I can't get the fields panel one to update its parent. https://d.pr/free/i/8Gpaz2

I fixed that, This was caused by a now obsolete performance optimization that made it necessary to pass the open state in from the data panel. I just removed the optimization.

elasticmachine · 2019-09-24T13:41:49Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

flash1293 · 2019-09-24T16:12:58Z

@cchaos tests should work now

elasticmachine · 2019-09-24T18:03:06Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cchaos · 2019-09-24T18:18:36Z

Awesome, thank you @flash1293 ! I'll rebase the PR and do the browser checks. I think it's ready for review then.

elasticmachine · 2019-09-24T18:19:07Z

Pinging @elastic/kibana-app

elasticmachine · 2019-09-24T20:47:07Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

x-pack/legacy/plugins/lens/public/indexpattern_plugin/change_indexpattern.tsx

x-pack/legacy/plugins/lens/public/indexpattern_plugin/datapanel.tsx

elasticmachine · 2019-09-25T00:32:27Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-25T18:19:20Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

cchaos · 2019-09-25T19:30:46Z

This PR's ready for a final review.

cchaos · 2019-09-27T01:56:13Z

@elasticmachine merge upstream

elasticmachine · 2019-09-27T03:05:17Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

cchaos · 2019-09-27T13:58:39Z

Random unrelated failing jest test. Gonna merge with upstream manually to see if it fixes it.

nickofthyme

LGTM

Just a thought on the css. Would it be good for us to set a max-width on the popover? To avoid this 👇

cchaos · 2019-09-27T14:05:26Z

Good catch @nickofthyme ! I will add that in

flash1293

LGTM, tested and works as expected 👍

elasticmachine · 2019-09-27T15:13:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-27T15:21:07Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-09-27T16:35:45Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

flash1293 reviewed Sep 24, 2019

View reviewed changes

cchaos marked this pull request as ready for review September 24, 2019 18:17

cchaos requested a review from a team September 24, 2019 18:17

cchaos added Feature:Lens Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.5.0 v8.0.0 labels Sep 24, 2019

nickofthyme reviewed Sep 24, 2019

View reviewed changes

cchaos requested a review from a team as a code owner September 24, 2019 22:48

cchaos added the release_note:skip Skip the PR/issue when compiling release notes label Sep 25, 2019

cchaos and others added 5 commits September 27, 2019 09:59

Use EuiSelectable for LayerPanelChooser

da99223

Fixing layout of index switcher in config panel

76cf70c

Some fixes but not all

568f2cf

fix index pattern picker in data panel

3bca87b

fix tests

0322b1c

cchaos added 3 commits September 27, 2019 09:59

Fix truncation

575af86

Fix bad merge

dc597ac

find instead of filter

93567a1

cchaos force-pushed the lens-index-pattern-select branch from 6afd876 to 93567a1 Compare September 27, 2019 13:59

nickofthyme approved these changes Sep 27, 2019

View reviewed changes

Fix bad merge from IP support PR

4929e77

Fix width of popover and class names

12d7fe3

flash1293 approved these changes Sep 27, 2019

View reviewed changes

cchaos mentioned this pull request Sep 27, 2019

[discover] De-angularize index pattern selection #46347

Merged

5 tasks

Fix class names from bad merge in master

82e55d4

nickofthyme approved these changes Sep 27, 2019

View reviewed changes

cchaos merged commit 6494f1e into elastic:master Sep 27, 2019

cchaos deleted the lens-index-pattern-select branch September 27, 2019 16:55

cchaos mentioned this pull request Sep 27, 2019

[7.x] [Lens] Index pattern selection using EuiSelectable (#46350) #46822

Merged

cchaos added a commit that referenced this pull request Sep 27, 2019

[Lens] Index pattern selection using EuiSelectable (#46350) (#46822)

57bf384

Conversation

cchaos commented Sep 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Created a shared component for changing index pattern

Checklist

Uh oh!

cchaos commented Sep 23, 2019

Uh oh!

elasticmachine commented Sep 23, 2019

💔 Build Failed

Uh oh!

flash1293 Sep 24, 2019

Choose a reason for hiding this comment

Uh oh!

flash1293 commented Sep 24, 2019

Uh oh!

elasticmachine commented Sep 24, 2019

💔 Build Failed

Uh oh!

flash1293 commented Sep 24, 2019

Uh oh!

elasticmachine commented Sep 24, 2019

💚 Build Succeeded

Uh oh!

cchaos commented Sep 24, 2019

Uh oh!

elasticmachine commented Sep 24, 2019

Uh oh!

elasticmachine commented Sep 24, 2019

💚 Build Succeeded

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Sep 25, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 25, 2019

💚 Build Succeeded

Uh oh!

cchaos commented Sep 25, 2019

Uh oh!

cchaos commented Sep 27, 2019

Uh oh!

elasticmachine commented Sep 27, 2019

💔 Build Failed

Uh oh!

cchaos commented Sep 27, 2019

Uh oh!

nickofthyme left a comment

Choose a reason for hiding this comment

Uh oh!

cchaos commented Sep 27, 2019

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Sep 27, 2019

💚 Build Succeeded

Uh oh!

elasticmachine commented Sep 27, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Sep 27, 2019

💚 Build Succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cchaos commented Sep 23, 2019 •

edited

Loading