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] add indicator when layer is filtered by search bar #43283

Merged
merged 4 commits into from
Aug 16, 2019
Merged

[Maps] add indicator when layer is filtered by search bar #43283

merged 4 commits into from
Aug 16, 2019

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Aug 14, 2019

fixes #42972

It can be difficult figuring out which layers are effected by the search bar. This PR adds a filter icon next to the layer name for each layer that is effected by the search bar when searches are applied.

Below is an example when the search bar is empty. No layers have the filter icon because there is no active filtering
Screen Shot 2019-08-14 at 9 51 20 AM

In this screen shot, a filter has been applied an layers effected by the global query are identified with the filter icon next to the name in the layer TOC

Screen Shot 2019-08-14 at 9 51 31 AM

Does this help better identify how the search bar is effecting layers? Or does this just add a bunch of visual clutter?

cc @alexfrancoeur @cchaos

@nreese nreese added release_note:enhancement [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.4.0 labels Aug 14, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis

@nreese nreese changed the title [Maps] add indicator when layer is filtered by filter pills and/or query bar [Maps] add indicator when layer is filtered by search bar Aug 14, 2019
@cchaos
Copy link
Contributor

cchaos commented Aug 14, 2019

I'm confused why this is necessary at all. In no other apps do we indicate that the graph/table/dashboard is being filtered.

@nreese
Copy link
Contributor Author

nreese commented Aug 14, 2019 •
edited
Loading

I'm confused why this is necessary at all. In no other apps do we indicate that the graph/table/dashboard is being filtered.

Maps is different from graph/table/dashboard in that certain layers will never be filtered by search bar, like EMS tiles, WMS, and TMS. Then other layers can turn off the global search bar so filters and queries are not applied to those layers.

This makes it really confusing for users to know which layers are filtered and which are not. This is especially true for users that did not create the map. How should users know which layers are from elasticsearch or which layers are not from elasticasearch? How should users know if the search bar is impacting a layer or not?

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cchaos
Copy link
Contributor

cchaos commented Aug 14, 2019

Then I'd say just make sure there is a space between the icon and the layer name and consider adding the icon and some text explaining it to the layer tooltip similar to how the partial results icon is handled.

@alexfrancoeur
Copy link

@cchaos could we treat them in a similar fashion as we do layers that are hidden from view but just use a different color / icon? The hover tooltip could indicate that this layer has been filtered.

image

image

The icon feels like a bit too much clutter to me, but if @cchaos is cool with it, I am too.

@cchaos
Copy link
Contributor

cchaos commented Aug 14, 2019

We can't just change the color of the label (color alone is not accessible) and we can't change the legend icon because there are still points on the map so the user needs to refer to the legend to match layer styles. This is why it needs an icon but the icon can be subdued and there then needs to be corresponding text in the tooltip.

@thomasneirynck
Copy link
Contributor

i'm very much 👍 on this one. I like that it is just put next to the layer name instead of putting itin the legend-icon, since it's so context-dependent. Agreed with @cchaos that it can behave identically to the "partial results" info button, with some tooltip explanation.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@nreese
Copy link
Contributor Author

nreese commented Aug 15, 2019

Agreed with @cchaos that it can behave identically to the "partial results" info button, with some tooltip explanation.

Screen Shot 2019-08-15 at 5 00 08 PM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@alexfrancoeur
Copy link

Just played around with this, LGTM 👍

thomasneirynck
thomasneirynck approved these changes Aug 16, 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.

Awesome little touch.

(not sure why that percy test is failing)

There's still the edge-case where the indicator is showing when the courier:ignoreFilterIfFieldNotInIndex is set to true.

This is does require a larger discussion, and I don't think this needs to be handled here. imho, it's really difficult for me to see a place for courier:ignoreFilterIfFieldNotInIndex flag in the Maps-app (it should always be true. but Maps should also handle the exclusion/inclusion of filters itself, so the UI can act accordingly).

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@nreese nreese merged commit f882584 into elastic:master Aug 16, 2019
nreese added a commit to nreese/kibana that referenced this pull request Aug 16, 2019
@nreese
[Maps] add indicator when layer is filtered by search bar (elastic#43283
06bd912 
)

* [Maps] add indicator when layer is filtered by filter pills and/or query bar

* add message about icon to tooltip
@nreese nreese mentioned this pull request Aug 16, 2019
Merged
nreese added a commit that referenced this pull request Aug 16, 2019
@nreese
[Maps] add indicator when layer is filtered by search bar (#43283) (#…
e4f40e5 
…43491)

* [Maps] add indicator when layer is filtered by filter pills and/or query bar

* add message about icon to tooltip
jloleysens added a commit to jloleysens/kibana that referenced this pull request Aug 19, 2019
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into chore/console…
98e12c2 
…_update_json_spec

* 'master' of github.com:elastic/kibana: (35 commits)
  fix: 🐛 pass whole action context to isCompatible() method (elastic#43457)
  Deleted old kbn-top-nav directive (elastic#43168)
  [ML] Fixing cloning of single metric distinct count job (elastic#43435)
  Update @elastic/charts version 8.1.6 > 9.1.1 (elastic#43516)
  [Inspector Views] [Request View] - Migrate inspector_views to new platform (elastic#43191)
  [ML] Adding loading indicators to all wizard charts (elastic#43382)
  disable flaky test (elastic#43492)
  feature(code/frontend): cancel file blob and directory commits request if outdated (elastic#43348)
  fix(code/frontend): button group url should have previous query string (elastic#43428)
  [SIEM] Fixes index substring incorrectly matching configured indices and failing to install ML job (elastic#43409)
  [SIEM] Adds performance enhancements such by removing wasted renderers and adding incremental DOM rendering (elastic#43157)
  disable flaky test (elastic#37859)
  Added sass lint to Canvas (elastic#43410)
  [Maps] add indicator when layer is filtered by search bar (elastic#43283)
  Properly validate current user password during password change. (elastic#43447)
  Spaces - allow for hex color codes that include uppercase characters (elastic#43470)
  [Reporting] Add a bit more logging and a few more logging level promotions (elastic#43415)
  Partially convert index pattern server to typescript (elastic#43291)
  [Infra UI] Use sum for aggregating AWS metrics. (elastic#43293)
  [SIEM] Format bytes columns in timeline (elastic#43147)
  ...
@cchaos
Copy link
Contributor

cchaos commented Aug 19, 2019

I know this is merged now, but you should have access to the partial icon type now to swap out the generic iInCircle.

@nreese
Copy link
Contributor Author

nreese commented Aug 19, 2019

I know this is merged now, but you should have access to the partial icon type now to swap out the generic iInCircle.

Thanks for pointing out the partial icon. That is a much better icon for this use case. I created
#43550 to use partial icon

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

@thomasneirynck thomasneirynck thomasneirynck approved these changes

@nickpeihl nickpeihl Awaiting requested review from nickpeihl

Assignees
No one assigned
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:enhancement v7.4.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Maps] Show indication in LayerTOC that apply-global-filter is turned off
5 participants
@nreese @elasticmachine @cchaos @alexfrancoeur @thomasneirynck