Add automated resource loading of icon-images #1000

tobrun · 2019-06-28T11:00:20Z

This PR is a proposal to automate loading of icon-image property of symbols. If core misses an style image, it will emit the image missing event. The symbol manager will pick this up and will attempt to load a drawable from resources with this id. This way users don't need to call addImage themselves, they can just pass in the id of the drawable resource as a String (see here).

Closes #742

@mapbox/maps-android

chloekraw · 2019-06-28T15:30:43Z

@tobrun, this is awesome! @julianrex @fabian-guerra, is this also possible on the iOS side?

julianrex · 2019-06-28T15:51:52Z

Yep, totally.

fabian-guerra · 2019-06-28T16:54:36Z

@chloekraw my answer would be yes and no 😅

Unlike Android listeners approach. iOS uses a delegate architecture where the map can "listen" for events and pass this to its single delegate. This means if I implement the delegate in the annotations controller then the developer won't be able to use the map's delegate in his own classes. This is what I mean with "no": I won't be using the same approach.

"Yes" means we can support it with a different approach.

LukasPaczos

Could we add a withIconImage option that accepts a resources ID, convert that to a String when passing to the map and back to an int in the ImageLoader?

plugin-annotation/scripts/annotation_manager.java.ejs

LukasPaczos

Also, could you include a test?

plugin-annotation/src/main/java/com/mapbox/mapboxsdk/plugins/annotation/SymbolManager.java

LukasPaczos

Looks great!
Could you add a line with an example to the SymbolOptions#withIconImage(String) javadoc explaining how a drawable name can be used as well?

LukasPaczos · 2019-07-22T11:22:18Z

...in/java/com/mapbox/mapboxsdk/plugins/testapp/activity/annotation/PressForSymbolActivity.java

@@ -146,5 +148,20 @@ static Bitmap getBitmapFromDrawable(Drawable drawable) {
      return bitmap;
    }
  }
+
+  @VisibleForTesting


Can changes here be reverted? They seem unused.

LukasPaczos · 2019-07-22T11:23:13Z

plugin-annotation/scripts/annotation_manager.java.ejs

@@ -74,12 +81,15 @@ public class <%- camelize(type) %>Manager extends AnnotationManager<<%- camelize
   */
  @UiThread
  public <%- camelize(type) %>Manager(@NonNull MapView mapView, @NonNull MapboxMap mapboxMap, @NonNull Style style, @Nullable String belowLayerId, @Nullable GeoJsonOptions geoJsonOptions) {
-    this(mapView, mapboxMap, style, new <%- camelize(type) %>ElementProvider(), belowLayerId, geoJsonOptions, new DraggableAnnotationController<<%- camelize(type) %>, On<%- camelize(type) %>DragListener>(mapView, mapboxMap));
+    this(mapView, mapboxMap, style, new <%- camelize(type) %>ElementProvider(), belowLayerId, geoJsonOptions, new DraggableAnnotationController<<%- camelize(type) %>, On<%- camelize(type) %>DragListener>(mapView, mapboxMap), new ImageLoader(mapView));


The ImageLoader argument addition should be if-checked for symbol manager.

LukasPaczos · 2019-07-22T11:23:21Z

plugin-annotation/scripts/annotation_manager.java.ejs

  }

  @VisibleForTesting
-  <%- camelize(type) %>Manager(@NonNull MapView mapView, @NonNull MapboxMap mapboxMap, @NonNull Style style, @NonNull CoreElementProvider<<%- camelize(type) %>Layer> coreElementProvider, @Nullable String belowLayerId, @Nullable GeoJsonOptions geoJsonOptions, DraggableAnnotationController<<%- camelize(type) %>, On<%- camelize(type) %>DragListener> draggableAnnotationController) {
+  <%- camelize(type) %>Manager(@NonNull MapView mapView, @NonNull MapboxMap mapboxMap, @NonNull Style style, @NonNull CoreElementProvider<<%- camelize(type) %>Layer> coreElementProvider, @Nullable String belowLayerId, @Nullable GeoJsonOptions geoJsonOptions, DraggableAnnotationController<<%- camelize(type) %>, On<%- camelize(type) %>DragListener> draggableAnnotationController, ImageLoader imageLoader) {


Same as above.

stale · 2019-09-20T11:51:50Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale · 2019-09-20T12:51:51Z

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

oantajames · 2019-09-30T12:38:19Z

@tobrun @LukasPaczos any updates on this? 😄 It's currently a blocker, since the update to the latest mapbox sdk version.

stale · 2019-11-29T13:10:26Z

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale · 2019-11-29T14:10:28Z

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

stale · 2020-01-28T17:16:35Z

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale · 2020-01-28T18:16:36Z

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

stale · 2020-03-28T22:14:17Z

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale · 2020-03-28T23:14:18Z

This pull request has been automatically detected as stale because it has not had recent activity and will be archived. Thank you for your contributions.

tobrun added the annotation-plugin label Jun 28, 2019

tobrun added this to the annotation-future milestone Jun 28, 2019

tobrun requested review from langsmith and LukasPaczos June 28, 2019 11:00

tobrun self-assigned this Jun 28, 2019

tobrun mentioned this pull request Jun 28, 2019

Track SymbolManager image additions? #860

Closed

chloekraw mentioned this pull request Jun 28, 2019

icon-image loader #742

Closed

julianrex mentioned this pull request Jun 28, 2019

Add automated resource loading of icon-images mapbox/mapbox-annotation-extension#15

Closed

LukasPaczos reviewed Jul 1, 2019

View reviewed changes

plugin-annotation/scripts/annotation_manager.java.ejs Outdated Show resolved Hide resolved

plugin-annotation/scripts/annotation_manager.java.ejs Outdated Show resolved Hide resolved

LukasPaczos reviewed Jul 1, 2019

View reviewed changes

langsmith reviewed Jul 1, 2019

View reviewed changes

plugin-annotation/src/main/java/com/mapbox/mapboxsdk/plugins/annotation/SymbolManager.java Outdated Show resolved Hide resolved

tobrun closed this Jul 3, 2019

tobrun force-pushed the tvn-load-icons branch from 0830332 to 5a787b0 Compare July 3, 2019 13:39

tobrun reopened this Jul 3, 2019

[annotation] - add automated resource loading of icon-images

a123c51

tobrun force-pushed the tvn-load-icons branch from bdf5ecd to a123c51 Compare July 16, 2019 09:02

LukasPaczos reviewed Jul 22, 2019

View reviewed changes

stale bot added the archived Archived by Stale bot. label Sep 20, 2019

stale bot closed this Sep 20, 2019

tobrun reopened this Sep 20, 2019

stale bot removed the archived Archived by Stale bot. label Sep 20, 2019

stale bot added the archived Archived by Stale bot. label Nov 29, 2019

stale bot closed this Nov 29, 2019

tobrun reopened this Nov 29, 2019

stale bot removed the archived Archived by Stale bot. label Nov 29, 2019

stale bot added the archived Archived by Stale bot. label Jan 28, 2020

stale bot closed this Jan 28, 2020

tobrun reopened this Jan 28, 2020

stale bot removed the archived Archived by Stale bot. label Jan 28, 2020

stale bot added the archived Archived by Stale bot. label Mar 28, 2020

stale bot closed this Mar 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add automated resource loading of icon-images #1000

Add automated resource loading of icon-images #1000

tobrun commented Jun 28, 2019

chloekraw commented Jun 28, 2019

julianrex commented Jun 28, 2019

fabian-guerra commented Jun 28, 2019

LukasPaczos left a comment

LukasPaczos left a comment

LukasPaczos left a comment

LukasPaczos Jul 22, 2019

LukasPaczos Jul 22, 2019

LukasPaczos Jul 22, 2019

stale bot commented Sep 20, 2019

stale bot commented Sep 20, 2019

oantajames commented Sep 30, 2019 •

edited

Loading

stale bot commented Nov 29, 2019

stale bot commented Nov 29, 2019

stale bot commented Jan 28, 2020

stale bot commented Jan 28, 2020

stale bot commented Mar 28, 2020

stale bot commented Mar 28, 2020

Add automated resource loading of icon-images #1000

Add automated resource loading of icon-images #1000

Conversation

tobrun commented Jun 28, 2019

chloekraw commented Jun 28, 2019

julianrex commented Jun 28, 2019

fabian-guerra commented Jun 28, 2019

LukasPaczos left a comment

Choose a reason for hiding this comment

LukasPaczos left a comment

Choose a reason for hiding this comment

LukasPaczos left a comment

Choose a reason for hiding this comment

LukasPaczos Jul 22, 2019

Choose a reason for hiding this comment

LukasPaczos Jul 22, 2019

Choose a reason for hiding this comment

LukasPaczos Jul 22, 2019

Choose a reason for hiding this comment

stale bot commented Sep 20, 2019

stale bot commented Sep 20, 2019

oantajames commented Sep 30, 2019 • edited Loading

stale bot commented Nov 29, 2019

stale bot commented Nov 29, 2019

stale bot commented Jan 28, 2020

stale bot commented Jan 28, 2020

stale bot commented Mar 28, 2020

stale bot commented Mar 28, 2020

oantajames commented Sep 30, 2019 •

edited

Loading