Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Expression filters #11429

Merged
merged 2 commits into from
Mar 19, 2018
Merged

Expression filters #11429

merged 2 commits into from
Mar 19, 2018

Conversation

tobrun
Copy link
Member

@tobrun tobrun commented Mar 9, 2018

WIP, this PR provides Android binding integration for #11251:

  • remove old Filter.java API
  • integrate setFilter(Expression filter)
    • update MapboxMap#queryRenderedFeatures
    • update Source#querySourceFeatures on source types
    • update Layer#setFilter on layer types (code-gen)
  • add getFilter
    • manage state corretly when no filter is availlable (java.lang.Error: in get())
  • integrate generated instrumentation test for set/get filter
  • migrate test app code to expression syntax

@tobrun tobrun added Android Mapbox Maps SDK for Android annotations Annotations on iOS and macOS or markers on Android labels Mar 9, 2018
@tobrun tobrun added this to the android-v6.0.0 milestone Mar 9, 2018
@tobrun tobrun self-assigned this Mar 9, 2018
@tobrun tobrun changed the base branch from master to release-boba March 9, 2018 10:53
@tobrun tobrun force-pushed the tvn-filter-expressions branch from 6d8c7ac to 172c2b0 Compare March 9, 2018 13:49
@tobrun tobrun removed the annotations Annotations on iOS and macOS or markers on Android label Mar 9, 2018
@tobrun tobrun force-pushed the tvn-filter-expressions branch 2 times, most recently from 50fcf59 to 63f211a Compare March 13, 2018 10:11
@tobrun tobrun force-pushed the tvn-filter-expressions branch from 63f211a to 04cc3ee Compare March 13, 2018 17:28
@tobrun tobrun requested a review from ivovandongen March 13, 2018 17:28
public Expression getFilter() {
Expression expression = null;
Object object = nativeGetFilter();
if (object != null && object instanceof JsonArray) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any other object types we expect here? Should we deal with those cases?

@@ -68,6 +68,8 @@ class Layer : private mbgl::util::noncopyable {

void setFilter(jni::JNIEnv&, jni::Array<jni::Object<>>);

jni::Object<jni::ObjectTag> getFilter(jni::JNIEnv&);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we type this more specific than Object?

@tobrun
Copy link
Member Author

tobrun commented Mar 19, 2018

@ivovandongen thank you for the review, I addressed your comments in 145e1e0. (FWIW flagging that most of the code in this PR will be refactored out with #11419 and #11420)

@tobrun tobrun merged commit f011035 into release-boba Mar 19, 2018
@tobrun tobrun deleted the tvn-filter-expressions branch March 19, 2018 14:11
@tobrun tobrun mentioned this pull request Mar 20, 2018
10 tasks
@vanshg
Copy link

vanshg commented Apr 12, 2018

I see that Filter.none has been removed with no explicit replacement (as well as Filter.notHas). Are there any plans on making those explicitly available, or will we have to go with something more verbose such as Expression.not(Expression.any) and Expression.not(Expression.has)?

@tobrun
Copy link
Member Author

tobrun commented Apr 12, 2018

I see that Filter.none has been removed with no explicit replacement (as well as Filter.notHas). Are there any plans on making those explicitly available, or will we have to go with something more verbose such as Expression.not(Expression.any) and Expression.not(Expression.has)?

the latter

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Android Mapbox Maps SDK for Android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants