[Discover] EUI-fication of the filters by majagrubic · Pull Request #48452 · elastic/kibana

majagrubic · 2019-10-16T18:47:00Z

Summary

Related:
#47357

Filters selectors are now EUI components:

The logic is still tied to Angular. Need to add tests, so opening this as a draft.

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

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

elasticmachine · 2019-10-16T20:12:32Z

💔 Build Failed

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

elasticmachine · 2019-10-16T21:32:42Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 4c86921

kertal

Great that you already applied to Lens style, tested in Chrome, works 👍. Added comments about details

src/legacy/core_plugins/kibana/public/discover/components/field_chooser/field_selector.tsx

...egacy/core_plugins/kibana/public/discover/components/field_chooser/discover_field_search.tsx

.../core_plugins/kibana/public/discover/components/field_chooser/discover_field_search.test.tsx

...egacy/core_plugins/kibana/public/discover/components/field_chooser/discover_field_search.tsx

elasticmachine · 2019-10-17T15:14:35Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 9f2939b

elasticmachine · 2019-10-17T16:37:59Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 61502e2

elasticmachine · 2019-10-17T19:12:04Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 37efd6e

majagrubic · 2019-10-17T21:54:28Z

@elasticmachine merge upstream

elasticmachine · 2019-10-17T23:13:00Z

💚 Build Succeeded

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

ryankeairns · 2019-10-18T13:09:05Z

Nice!

I'll take a look at that the design side as it appears there might be a panel inside of the popover menu.

ryankeairns · 2019-10-29T15:55:18Z

@majagrubic This is looking great, I've got two additional requests:

Let's use the compressed style form inputs. I missed this the first time around, so here is another PR with the desired changes: use compressed style inputs majagrubic/kibana#2

Since the first two select inputs have three finite options, it would be preferable to use a compressed EuiButtonGroup instead of an EuiSelect. If you'd like to do this as a follow-up, that is fine as well. Here is an example of this would look afterwards:

Thanks,
Ryan

use compressed style inputs

majagrubic · 2019-10-29T22:34:28Z

@ryankeairns thanks for the comment, I merged your PR. regarding button group over select: is this how it is expected to look like then?

elasticmachine · 2019-10-29T22:36:28Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9b0f109

ryankeairns · 2019-10-30T00:03:42Z

@majagrubic that looks great, thanks for fitting this in!

elasticmachine · 2019-10-30T14:32:25Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9753c3c

ryankeairns

@majagrubic checked this out locally and it looks great, thank you!

How much work would it be to have the popover close when clicking outside of it? Currently, it is only possible to close it by clicking on the filter button again. Clicking outside to close a popover is common pattern we've used elsewhere, see Lens as an example.

elasticmachine · 2019-10-30T15:15:45Z

💚 Build Succeeded

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

majagrubic · 2019-10-31T09:33:05Z

@ryankeairns mind if I do that in the follow-up PR? This one is already growing quite big.

ryankeairns · 2019-10-31T13:39:22Z

@majagrubic yep, that works for me, thanks!

kertal

Added one structural change request, think it makes sense to extract one sub component in it's own file. Update discussed this, and decided to keep it this way.

...egacy/core_plugins/kibana/public/discover/components/field_chooser/discover_field_search.tsx

kertal

Thank a lot for another important step in redesigning Discover's field selector 👍 . Code LGTM, tested locally in Chrome

elasticmachine · 2019-10-31T16:44:56Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 9ab9192

* New filter UI * Prototype of functionality * EUIFication of filters in Discover * Adding tests for field_selector component * Replacing FieldSelector with EuiForm components * Adding more discover field search tests * Removing console statement * Fixing failing functional test * Removing obsolete snapshot * design tweaks for filter popover * use compressed style inputs * Changing selectors to be EuiButtonGroup * Removing unnecessary if statement * Getting rid of ts-ignore warning

Maja Grubic added 2 commits October 14, 2019 18:16

New filter UI

67b7f0c

Prototype of functionality

20e5f8f

majagrubic force-pushed the discover-euification branch 2 times, most recently from e0e51a8 to d6fc63f Compare October 16, 2019 18:49

EUIFication of filters in Discover

d6fc63f

Adding tests for field_selector component

4c86921

majagrubic requested a review from kertal October 16, 2019 21:35

kertal reviewed Oct 17, 2019

View reviewed changes

Replacing FieldSelector with EuiForm components

9f2939b

Adding more discover field search tests

61502e2

Removing console statement

fa442a4

kertal requested review from mdefazio and ryankeairns October 17, 2019 17:50

Fixing failing functional test

37efd6e

Removing obsolete snapshot

ac56e38

majagrubic marked this pull request as ready for review October 17, 2019 21:51

majagrubic requested a review from a team as a code owner October 17, 2019 21:51

majagrubic added v7.5.0 v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Oct 17, 2019

Merge branch 'master' into discover-euification

fe6dd0a

design tweaks for filter popover

1ce90c9

ryankeairns mentioned this pull request Oct 29, 2019

use compressed style inputs majagrubic/kibana#2

Merged

Merge pull request #2 from ryankeairns/discover-euification

9b0f109

use compressed style inputs

Maja Grubic added 3 commits October 30, 2019 13:12

Changing selectors to be EuiButtonGroup

e14ac4e

Removing unnecessary if statement

9753c3c

Merge master and resolve conflicts

ee83f2e

ryankeairns approved these changes Oct 30, 2019

View reviewed changes

kertal self-requested a review October 31, 2019 13:25

kertal reviewed Oct 31, 2019

View reviewed changes

...egacy/core_plugins/kibana/public/discover/components/field_chooser/discover_field_search.tsx Show resolved Hide resolved

...egacy/core_plugins/kibana/public/discover/components/field_chooser/discover_field_search.tsx Outdated Show resolved Hide resolved

Getting rid of ts-ignore warning

9ab9192

majagrubic requested a review from kertal October 31, 2019 15:30

kertal approved these changes Oct 31, 2019

View reviewed changes

majagrubic merged commit 559ac84 into elastic:master Oct 31, 2019

majagrubic deleted the discover-euification branch October 31, 2019 17:53

majagrubic mentioned this pull request Oct 31, 2019

[Discover] Close filter popup on click outside #49934

Merged

4 tasks

majagrubic mentioned this pull request Nov 11, 2019

[7.x] [Discover] EUI-fication of the filters (#48452) #50148

Closed

majagrubic mentioned this pull request Nov 11, 2019

[7.x] [Discover] EUI-fication of the filters (#48452) #50180

Merged

kertal mentioned this pull request Dec 19, 2019

[Discover] Remove angular field filter template code #53513

Merged

Conversation

majagrubic commented Oct 16, 2019

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Oct 16, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Oct 16, 2019

💔 Build Failed

Uh oh!

kertal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

elasticmachine commented Oct 17, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Oct 17, 2019

💔 Build Failed

Uh oh!

elasticmachine commented Oct 17, 2019

💔 Build Failed

Uh oh!

majagrubic commented Oct 17, 2019

Uh oh!

elasticmachine commented Oct 17, 2019

💚 Build Succeeded

Uh oh!

ryankeairns commented Oct 18, 2019

Uh oh!

ryankeairns commented Oct 29, 2019

Uh oh!

majagrubic commented Oct 29, 2019

Uh oh!

elasticmachine commented Oct 29, 2019

💚 Build Succeeded

Uh oh!

ryankeairns commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Oct 30, 2019

💚 Build Succeeded

Uh oh!

ryankeairns left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Oct 30, 2019

💚 Build Succeeded

Uh oh!

majagrubic commented Oct 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryankeairns commented Oct 31, 2019

Uh oh!

kertal left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kertal left a comment

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Oct 31, 2019

💚 Build Succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ryankeairns commented Oct 30, 2019 •

edited

Loading

majagrubic commented Oct 31, 2019 •

edited

Loading

kertal left a comment •

edited

Loading