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.

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

[Maps] Hide feature when it has no corresponding term join #36617

Merged
merged 18 commits into from
Jul 2, 2019

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented May 15, 2019

Closes #34662

This is essentially blocked by #36466, as that one modifies some of the style-refresh logic.


todo:

  • make this configurable so we support both behaviors (left inner join and inner join) will change the default behavior to hide features instead.

open ended issues

  • what to do with fitting? On EMS-layers, we could just fit on the visible results, but for ES-layers, ES produces the bbox. the latter approach will not work when mashed up with a client-side join. will just fit on the entire source data
  • this is implemented using mapbox-filters. retain mapbox filter implementation
    • it could also be implemented by just removing the data from source altogether, creating a new source after joining. this implementation resets the source when the data updates
    • could also use an if-then rule for styling properties too, ignoring to apply the style rules when it has n join.It might not be worth the extra complexity either.

@thomasneirynck thomasneirynck added WIP Work in progress [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.2.0 labels May 15, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis

@thomasneirynck thomasneirynck changed the title [Maps] Hide features when no corresponding joins [Maps] Hide feature when it has no corresponding join data May 15, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@thomasneirynck thomasneirynck added v7.3.0 and removed WIP Work in progress labels Jun 27, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@nreese
Copy link
Contributor

nreese commented Jul 1, 2019

Notice in the screen show below that the time filter excludes all data so no matches from the terms join are returned. Notice how the legend does not signal that the source, EMS vector layers, is now empty because no features are visible because of the join.

Can you also add some logic to VectorLayer.getCustomIconAndTooltipContent to provide a message when terms join has filtered out all features? That way, the legend icon will signal that the layer is not showing any data. The logic should check if the layer has joins and then verify at least one feature is visible. If not, then return the minus icon with a useful error message.

Screen Shot 2019-07-01 at 9 51 49 AM

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@thomasneirynck thomasneirynck changed the title [Maps] Hide feature when it has no corresponding join data [Maps] Hide feature when it has no corresponding term join Jul 1, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@@ -98,25 +119,39 @@ export class VectorLayer extends AbstractLayer {
getCustomIconAndTooltipContent() {
const sourceDataRequest = this.getSourceDataRequest();
const featureCollection = sourceDataRequest ? sourceDataRequest.getData() : null;


Copy link
Contributor

Choose a reason for hiding this comment

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

remove extra space

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

lgtm
code review, tested in chrome

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@gchaps
Copy link
Contributor

gchaps commented Jul 9, 2019

@thomasneirynck Could you please add details about this change to the 7.3 breaking changes doc? You'll find the doc under docs/migration/migration_7_3.asciidoc

For sample content and how to format it, look at the 7.2 breaking changes doc:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] hide features that do not have any matching properties for data-driven style rules
4 participants