Skip to content

[Discover] De-angularize side bar search field#46679

Merged
kertal merged 19 commits intoelastic:masterfrom
kertal:kertal-pr-2019-09-23-discover-field-filter
Oct 2, 2019
Merged

[Discover] De-angularize side bar search field#46679
kertal merged 19 commits intoelastic:masterfrom
kertal:kertal-pr-2019-09-23-discover-field-filter

Conversation

@kertal
Copy link
Member

@kertal kertal commented Sep 26, 2019

Summary

De-angularize and adapt new design for Discover's field search.

Aside the search/filter by name, additional fields for filtering can be displayed. Note that they are not part of the PR (still angular), will be targeted soon.

One difference to the original behavior is that the filtering/searching also takes place in selected fields.

Additional filter fields folded:
Bildschirmfoto 2019-09-26 um 17 59 04
Additional fields unfolded:
Bildschirmfoto 2019-09-26 um 17 59 29

Checklist

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

For maintainers

- [ ] This was checked for breaking API changes and was labeled appropriately

@kertal kertal self-assigned this Sep 26, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kertal kertal added the Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// label Sep 26, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@kertal kertal requested a review from ryankeairns September 26, 2019 17:19
@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

Great start! It looks like the Lens stuff is moving along, so we can steal from them soon to improve this further :)

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kertal kertal marked this pull request as ready for review September 27, 2019 05:32
@kertal kertal requested a review from a team as a code owner September 27, 2019 05:32
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kertal
Copy link
Member Author

kertal commented Sep 30, 2019

@elasticmachine merge upstream

@kertal kertal added v8.0.0 Feature:Discover Discover Application labels Sep 30, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Tested in chrome and works, LGTM 👍

One thing I noticed (not a blocker for this PR): Currently _source is implicitly selected if nothing else is selected. Somehow (this is probably a separate bug) it is counted as a missing field and thus not shown in the selected list from now on (even if it's there and shown in the table). It might make sense to show _source as the initial selected field when Discover is opened, just to be consistent with what is shown in the table and in the side bar.

<EuiFlexItem grow={false}>
<EuiButtonIcon
aria-expanded={showFilter}
aria-label={filterBtnAriaLabel}
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO it would be nice to also have this in a tooltip - what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 will do!

Copy link
Member Author

Choose a reason for hiding this comment

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

done!

…om:kertal/kibana into kertal-pr-2019-09-23-discover-field-filter
@kertal
Copy link
Member Author

kertal commented Sep 30, 2019

Tested in chrome and works, LGTM 👍

One thing I noticed (not a blocker for this PR): Currently _source is implicitly selected if nothing else is selected. Somehow (this is probably a separate bug) it is counted as a missing field and thus not shown in the selected list from now on (even if it's there and shown in the table). It might make sense to show _source as the initial selected field when Discover is opened, just to be consistent with what is shown in the table and in the side bar.

good catch, thx! now _source works like before!

@elasticmachine
Copy link
Contributor

💔 Build Failed

- due to padding issues collapsing didn't work
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@kertal kertal merged commit 03ccb43 into elastic:master Oct 2, 2019
@kertal kertal deleted the kertal-pr-2019-09-23-discover-field-filter branch October 2, 2019 08:11
kertal added a commit to kertal/kibana that referenced this pull request Oct 2, 2019
* Add DiscoverFieldSearch react component

* Adapt field_chooser.js

* Add jest test

* Adapt scss
kertal added a commit that referenced this pull request Oct 2, 2019
* Add DiscoverFieldSearch react component

* Adapt field_chooser.js

* Add jest test

* Adapt scss
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Discover Discover Application release_note:enhancement Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.5.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants