Add Scale bar plugin #955

Chaoba · 2019-05-15T07:34:32Z

Resolves #11

tobrun

Looks great @Chaoba! did a quick initial review. Will try finding some time to run this against the profiler. Would be great if you could add some tests in the meantime.

app/build.gradle

app/src/main/java/com/mapbox/mapboxsdk/plugins/testapp/activity/scalebar/ScalebarActivity.kt

plugin-scalebar/src/androidTest/java/com/mapbox/pluginscalebar/ExampleInstrumentedTest.java

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBar.java

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarWidget.java

plugin-scalebar/src/main/res/values/strings.xml

plugin-scalebar/src/test/java/com/mapbox/pluginscalebar/ExampleUnitTest.java

app/src/main/java/com/mapbox/mapboxsdk/plugins/testapp/activity/scalebar/ScalebarActivity.kt

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBar.java

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarWidget.java

…y/scalebar/ScalebarActivity.kt Co-Authored-By: Langston Smith <langston.smith@mapbox.com>

…ar.java Co-Authored-By: Langston Smith <langston.smith@mapbox.com>

…arWidget.java Co-Authored-By: Langston Smith <langston.smith@mapbox.com>

Chaoba · 2019-05-20T05:54:41Z

@tobrun When using double click to zoom in, ScaleListener can't get notified, how can we handle this scenario?

LukasPaczos · 2019-05-20T10:32:38Z

@Chaoba instead of using MapboxMap.OnScaleListener I think it'd better to use OnCameraMoveListener and invalidate the view whenever zoom or latitude changes. Additionally, it'd be great to cache camera values and invalidate the view in loops, every x milliseconds (if the camera position value has changed) instead of on each update which can be extremely frequent.

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarWidget.java

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBar.java

LukasPaczos

A couple of comments below. Generally looks great!

When it comes to exposing the setters, the more the better, but I've marked only the ones that I feel are necessary to match basic use-cases. An Options class (basically a builder approach) would fit nicely. You could provide those options to the widget through the plugin's constructor, or even in the runtime.

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarWidget.java

app/src/main/java/com/mapbox/mapboxsdk/plugins/testapp/activity/scalebar/ScalebarActivity.kt

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBar.java

LukasPaczos

I've done a little bit more tests with the current setup on the Pixel 2 XL - rapidly zooming in and out:

Refresh interval	`ScaleBarWidget#onDraw` processor time %
5ms	0.65%
10ms	0.54%
16ms	0.46%
25ms	0.20%
100ms	0.18%

From the above, I think it's safe to decrease the default minimum refresh interval to 15ms. It increases the UX on performant devices by allowing us to render the widget in around 60 FPS and doesn't lose pace to the map renderer visually. The interval can be adjusted to lower end devices by developers.

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarOption.java

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarManager.java

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarOption.java

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarManager.java

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarOption.java

app/src/main/java/com/mapbox/mapboxsdk/plugins/testapp/activity/scalebar/ScalebarActivity.kt

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarOption.java

…arOption.java Co-Authored-By: Langston Smith <langston.smith@mapbox.com>

…y/scalebar/ScalebarActivity.kt Co-Authored-By: Langston Smith <langston.smith@mapbox.com>

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarPlugin.java

app/src/androidTest/java/com/mapbox/mapboxsdk/plugins/scalebar/ScaleBarTest.java

plugin-scalebar/build.gradle

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarOptions.java

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarPlugin.java

LukasPaczos

LGTM!

Please rebase and clean up the git history before merging, or use the squash merge.

Chaoba · 2019-05-29T09:02:56Z

@LukasPaczos Thanks for your help. 🚀

virtuoso80 · 2019-08-26T10:57:45Z

Tried it works and looks great. It would be nicer though if you could define a fixed horizontal size for the Scale Bar so that it doesn't constantly resize itself. Thanks

LukasPaczos · 2019-08-26T11:11:31Z

Hey @virtuoso80, thanks for the feedback! Could you cut a separate ticket for your request? Thank you!

kevin added 2 commits May 15, 2019 15:45

Create scale bar module

f995d4b

Implement scale bar function

7fd1275

Chaoba force-pushed the kl-scalebar branch from acfe8b7 to 8612469 Compare May 15, 2019 07:50

Chaoba self-assigned this May 15, 2019

revert manifest

439dd2a

Chaoba force-pushed the kl-scalebar branch from 8612469 to 439dd2a Compare May 15, 2019 08:02

tobrun reviewed May 16, 2019

View reviewed changes

langsmith reviewed May 16, 2019

View reviewed changes

app/src/main/java/com/mapbox/mapboxsdk/plugins/testapp/activity/scalebar/ScalebarActivity.kt Outdated Show resolved Hide resolved

langsmith reviewed May 16, 2019

View reviewed changes

app/src/main/java/com/mapbox/mapboxsdk/plugins/testapp/activity/scalebar/ScalebarActivity.kt Outdated Show resolved Hide resolved

langsmith reviewed May 16, 2019

View reviewed changes

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBar.java Outdated Show resolved Hide resolved

langsmith reviewed May 16, 2019

View reviewed changes

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarWidget.java Outdated Show resolved Hide resolved

Kevin Li and others added 5 commits May 17, 2019 08:09

Update app/src/main/java/com/mapbox/mapboxsdk/plugins/testapp/activit…

0e1c7a6

…y/scalebar/ScalebarActivity.kt Co-Authored-By: Langston Smith <langston.smith@mapbox.com>

Update plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleB…

e87c8d5

…ar.java Co-Authored-By: Langston Smith <langston.smith@mapbox.com>

Update plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleB…

65e4c38

…arWidget.java Co-Authored-By: Langston Smith <langston.smith@mapbox.com>

fix review comments

321fb75

Add test cases

f934912

langsmith mentioned this pull request May 20, 2019

Scale Widget Plugin #11

Closed

kevin added 2 commits May 22, 2019 11:28

Add Handler to limit fresh frequent

2a6e8b2

Add instrumentation test

781c487

Chaoba changed the title ~~[WIP] Add Scale bar plugin~~ Add Scale bar plugin May 22, 2019

rm redundant codes

027cf78

Chaoba added the ready for review When your PR has been personally reviewed, its time for an external contributors to approve label May 22, 2019

LukasPaczos reviewed May 22, 2019

View reviewed changes

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarWidget.java Outdated Show resolved Hide resolved

move judgement of WeakReference

496f8de

LukasPaczos reviewed May 22, 2019

View reviewed changes

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBar.java Outdated Show resolved Hide resolved

LukasPaczos reviewed May 23, 2019

View reviewed changes

Add ScalebarOption.

c7ae1a2

LukasPaczos reviewed May 27, 2019

View reviewed changes

Add interface for dimen parameters.

126b513

LukasPaczos reviewed May 28, 2019

View reviewed changes

fix review comments

c8212cf

langsmith reviewed May 29, 2019

View reviewed changes

app/src/main/java/com/mapbox/mapboxsdk/plugins/testapp/activity/scalebar/ScalebarActivity.kt Outdated Show resolved Hide resolved

langsmith reviewed May 29, 2019

View reviewed changes

plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleBarOption.java Outdated Show resolved Hide resolved

Kevin Li and others added 3 commits May 29, 2019 12:24

Update plugin-scalebar/src/main/java/com/mapbox/pluginscalebar/ScaleB…

7469cab

…arOption.java Co-Authored-By: Langston Smith <langston.smith@mapbox.com>

Update app/src/main/java/com/mapbox/mapboxsdk/plugins/testapp/activit…

acfc038

…y/scalebar/ScalebarActivity.kt Co-Authored-By: Langston Smith <langston.smith@mapbox.com>

rename ScaleBarOptions

869cbcd

LukasPaczos reviewed May 29, 2019

View reviewed changes

fix review comments

15fb84b

LukasPaczos approved these changes May 29, 2019

View reviewed changes

LukasPaczos added this to the scalebar-0.1.0 milestone May 29, 2019

Chaoba merged commit e811054 into master May 29, 2019

Chaoba deleted the kl-scalebar branch May 29, 2019 09:03

This was referenced Jun 11, 2019

Add Scalebar Plugin example mapbox/mapbox-android-demo#1101

Closed

Release plugins with the Maps SDK v8.0.0 #987

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Scale bar plugin #955

Add Scale bar plugin #955

Chaoba commented May 15, 2019 •

edited

Loading

tobrun left a comment

Chaoba commented May 20, 2019

LukasPaczos commented May 20, 2019

LukasPaczos left a comment

LukasPaczos left a comment

LukasPaczos left a comment

Chaoba commented May 29, 2019

virtuoso80 commented Aug 26, 2019

LukasPaczos commented Aug 26, 2019

Add Scale bar plugin #955

Add Scale bar plugin #955

Conversation

Chaoba commented May 15, 2019 • edited Loading

tobrun left a comment

Choose a reason for hiding this comment

Chaoba commented May 20, 2019

LukasPaczos commented May 20, 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

Chaoba commented May 29, 2019

virtuoso80 commented Aug 26, 2019

LukasPaczos commented Aug 26, 2019

Chaoba commented May 15, 2019 •

edited

Loading