Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Maps] clean up source settings #52644

Merged
merged 11 commits into from
Dec 11, 2019
Merged

[Maps] clean up source settings #52644

merged 11 commits into from
Dec 11, 2019

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Dec 10, 2019 •
edited
Loading

Fixes #47467

Source settings placed all inputs into a single panel. This was starting to create a mess. This PR allows each source to add multiple panels for their source settings so settings can be organized better.

Screen Shot 2019-12-10 at 12 20 45 PM

@nreese nreese added chore release_note:skip Skip the PR/issue when compiling release notes [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v7.6.0 v8.0.0 labels Dec 10, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-docs (Team:Docs)

@nreese nreese requested a review from gchaps December 10, 2019 19:46
thomasneirynck
thomasneirynck approved these changes Dec 11, 2019
View reviewed changes
Copy link
Contributor

@thomasneirynck thomasneirynck left a comment

Choose a reason for hiding this comment

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

Looks sooo much better!

Two small suggestions:

  • consider adding some division between the sort-order (field and order-dropdowns) optins and the top-hits options (switch and entity-dropdown)

  • consider putting the filter-query editor on a single line. It looks a little unbalanced now imho

e.g.:

  • unset
    image

  • set

image

docs/maps/maps-aggregations.asciidoc Outdated Show resolved Hide resolved
x-pack/legacy/plugins/maps/public/layers/sources/es_geo_grid_source/update_source_editor.js Outdated Show resolved Hide resolved
gchaps
gchaps reviewed Dec 11, 2019
View reviewed changes
Copy link
Contributor

@gchaps gchaps left a comment •
edited
Loading

Choose a reason for hiding this comment

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

For the UI text:

"Show documents per entity" is a little vague for the radio button description. Some suggestions:

Show top documents per entity
Show top hits per entity

But that begs to question, is there a better word for entity?

docs/maps/maps-aggregations.asciidoc Outdated Show resolved Hide resolved
docs/maps/maps-aggregations.asciidoc Outdated Show resolved Hide resolved
@nreese
Copy link
Contributor Author

nreese commented Dec 11, 2019

consider adding some division between the sort-order (field and order-dropdowns) optins and the top-hits options (switch and entity-dropdown)

Added a EuiHorizontalRule rule between sort and top hits options

consider putting the filter-query editor on a single line. It looks a little unbalanced now imho

I will leave as-is for now since its consistent with adding Metrics and adding tooltip fields.

@nreese nreese requested a review from gchaps December 11, 2019 19:35
gchaps
gchaps approved these changes Dec 11, 2019
View reviewed changes
Copy link
Contributor

@gchaps gchaps left a comment

Choose a reason for hiding this comment

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

LGTM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

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

@nreese nreese merged commit eb8c0e3 into elastic:master Dec 11, 2019
nreese added a commit to nreese/kibana that referenced this pull request Dec 11, 2019
@nreese
[Maps] clean up source settings (elastic#52644)
67c5a43 
* [Maps] clean up source settings

* update es_search_source editor

* geo grid source

* pew pew source

* fix i18n

* doc updates for changing UI

* docs re-wording

* use correct image name

* update snapshot

* review feedback
@nreese nreese mentioned this pull request Dec 11, 2019
Merged
nreese added a commit that referenced this pull request Dec 11, 2019
@nreese
[Maps] clean up source settings (#52644) (#52818)
fce83be 
* [Maps] clean up source settings

* update es_search_source editor

* geo grid source

* pew pew source

* fix i18n

* doc updates for changing UI

* docs re-wording

* use correct image name

* update snapshot

* review feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gchaps gchaps gchaps approved these changes

@thomasneirynck thomasneirynck thomasneirynck approved these changes

Assignees
No one assigned
Labels
chore [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:skip Skip the PR/issue when compiling release notes Team:Docs v7.6.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Maps] Source settings seem random
4 participants
@nreese @elasticmachine @thomasneirynck @gchaps