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

Implementation of feature querying on iOS #177

Merged
merged 16 commits into from
Jun 21, 2020

Conversation

TimothySealy
Copy link
Collaborator

@TimothySealy TimothySealy commented Jan 14, 2020

Closes #163

Remark: Filtering on iOS and Android do not seem to be compatible. The current example works on iOS but not on Android (gives the following Error converting filter: [0]: Unknown expression "type == 'zoo'".)

@tobrun
Copy link
Collaborator

tobrun commented Jan 20, 2020

Error converting filter: [0]: Unknown expression "type == 'zoo'".)

imo that is to be expected as type == zoo isn't a valid string expression. I'm guessing that iOS SDK either has kept old legacy filters around or this is a valid NSExpression that is later converted to a mapbox valid expression.

I'm guessing a valid expression would look like: ["==", get("type"), "zoo"].

I can look into the Android side of things.

Thanks again for some great work @TimothySealy

@TimothySealy
Copy link
Collaborator Author

So I'm still tumbling down the rabbit hole regarding the expressions.

You are correct that the "type == 'zoo'" is only valid as an NSPredicate (on iOS). This class is used for filtering in the MGLMapView.visibleFeaturesAtPoint function. This is not compatible with how expressions are parsed on Android. The Android examples I've found so far use strong typed Expressions. For example, filtering on zoo's would look something like this:
Expression filterExpression = Expression.eq(Expression.get("type"), Expression.literal("zoo")); or Expression filterExpression = eq(get("type"), literal("zoo")); in short.

If we move to the way expressions are defined in the style spec defined in the JS library we would pass the expression as an array and need to translate this to the relevant objects in the different platform. Filtering zoo's would like this: ["==", ["get", "type"] , "zoo"] if I'm correct. Or maybe even ["==", "type" , "zoo"]. Currently I'm looking into both SDKs to check whether util methods are provided to translate this array to expressions (or predicates).

@tobrun
Copy link
Collaborator

tobrun commented Feb 15, 2020

@TimothySealy maybe the most straightforward solution for both platforms is to only support a string representation of expressions. On Android you can do something as Expression.raw(String expression) and if I recall correctly we do have an equivalent on iOS as well.

@TimothySealy
Copy link
Collaborator Author

I've updated the code so that the filtering now works on iOS and Android. In my first attempt I've passed a string representation of the expression to use in the filtering. After having a working implementation I've switched to passing the actual array containing the expression instead of its string representation.

CI now breaks because of a mismatch in package versions (for the location package). This has been fixed in #239.

@m0nac0
Copy link
Collaborator

m0nac0 commented Jun 2, 2020

@TimothySealy I've just tested this, but the filter example doesn't seem to have any effect on Android.
Am I right by expecting that only zoos and nothing else should be returned if the filtering is turned on?

@TimothySealy
Copy link
Collaborator Author

TimothySealy commented Jun 3, 2020

@m0nac0 I have implemented a toggler for enabling and disabling the filter. Default the filter is turned of. On the user interface page please check whether the filter is enabled (first option in the list). It is correct that if the filter is enabled you should only receive zoo features.

An improvement would be to display the polygon of the returned features as visual feedback. Currently the results are only printed as debug messages.

I've rebased the branch in order fix the Page conflict and merge issue.

@m0nac0
Copy link
Collaborator

m0nac0 commented Jun 3, 2020

@TimothySealy I did actually toggle that button, and I also checked that _featureQueryFilter was set to ["==", ["get", "type"] , "zoo"], but it didn't have any effect. I still got back all kinds of points and polygons.

@TimothySealy
Copy link
Collaborator Author

@m0nac0 That's strange. I've just tested it in an emulator after the rebase and it seemed to work. Could you maybe provide some of the debug message? I'm particularly interested in the Map click: ${point.x},${point.y} ${latLng.latitude}/${latLng.longitude} and the printed features. The filter ["==", ["get", "type"] , "zoo"] seems to be correct.

@m0nac0
Copy link
Collaborator

m0nac0 commented Jun 3, 2020

@TimothySealy Nevermind, it's working now for me, as well. Apparently the issue was that I didn't provide a valid access token when I was testing it yesterday, because the tiles were still cached anyways. For some reason, rendering the tiles seems to still work without the access token, but filtering not. I'm sorry about the confusion.

# Conflicts:
#	lib/src/controller.dart
#	pubspec.lock
@tobrun tobrun added this to the v0.7.0 milestone Jun 6, 2020
@tobrun
Copy link
Collaborator

tobrun commented Jun 6, 2020

Thank you for rebasing against the latest restructure changes. LGTM

@tobrun tobrun modified the milestones: v0.7.0, v0.8.0 Jun 6, 2020
# Conflicts:
#	lib/src/controller.dart
#	pubspec.lock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No implementation found for method map#queryRenderedFeatures
3 participants