-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
8674ffb
to
521070b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These examples are awesome 👏
Minor details.
} | ||
findViewById(R.id.fab).setVisibility(View.GONE); | ||
|
||
// reset camera to init position |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about extracting this block of code into a private
method and give it a name based on the comment? This way onOptionsItemSelected
will be easier to read and understand and comment will become superfluous so it won't be necessary.
.build() | ||
)); | ||
|
||
// play animation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here ☝️
} | ||
|
||
private Animator createExamepleInterpolator(int menuItemId) { | ||
switch (menuItemId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about removing the switch
using a map/dictionary? AnimatorSet
s could be created beforehand and inserted into a map using the different menu ids as values.
521070b
to
50d781a
Compare
@tobrun I found a crash when testing these examples when clicking the different options back and forth logcat
Symbolicated Android crash
Moto X (2nd gen) Android 5.0 |
} | ||
|
||
private Animator createExamepleInterpolator(int menuItemId) { | ||
switch (menuItemId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking something like this
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/camera/CameraAnimatorActivity.java b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/camera/CameraAnimatorActivity.java
index 134086145..c6e3c7a05 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/camera/CameraAnimatorActivity.java
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/main/java/com/mapbox/mapboxsdk/testapp/activity/camera/CameraAnimatorActivity.java
@@ -25,6 +25,9 @@ import com.mapbox.mapboxsdk.maps.MapboxMap;
import com.mapbox.mapboxsdk.maps.OnMapReadyCallback;
import com.mapbox.mapboxsdk.testapp.R;
+import java.util.HashMap;
+import java.util.Map;
+
/**
* Test activity showcasing using Android SDK animators to animate camera position changes.
*/
@@ -206,27 +209,50 @@ public class CameraAnimatorActivity extends AppCompatActivity implements OnMapRe
}
}
- private Animator createExamepleInterpolator(int menuItemId) {
- switch (menuItemId) {
- case R.id.menu_action_accelerate_decelerate_interpolator:
- AnimatorSet animatorSet = new AnimatorSet();
- animatorSet.playTogether(
- createLatLngAnimator(START_LAT_LNG, new LatLng(37.826715, -122.422795)),
- createExampleInterpolator(new FastOutSlowInInterpolator(), 2500));
- return animatorSet;
- case R.id.menu_action_bounce_interpolator:
- AnimatorSet bounceAnimatorSet = new AnimatorSet();
- bounceAnimatorSet.playTogether(
- createLatLngAnimator(START_LAT_LNG, new LatLng(37.787947, -122.407432)),
- createExampleInterpolator(new BounceInterpolator(), 3750)
- );
- return bounceAnimatorSet;
- case R.id.menu_action_anticipate_overshoot_interpolator:
- return createExampleInterpolator(new AnticipateOvershootInterpolator(), 2500);
- case R.id.menu_action_path_interpolator:
- return createExampleInterpolator(PathInterpolatorCompat.create(.22f, .68f, 0, 1.71f), 2500);
+ private final Map<Integer, AnimatorBuilder> ANIMATORS = new HashMap<Integer, AnimatorBuilder>() {
+ {
+ put(R.id.menu_action_accelerate_decelerate_interpolator, new AnimatorBuilder() {
+ @Override
+ public Animator build() {
+ AnimatorSet animatorSet = new AnimatorSet();
+ animatorSet.playTogether(
+ createLatLngAnimator(START_LAT_LNG, new LatLng(37.826715, -122.422795)),
+ createExampleInterpolator(new FastOutSlowInInterpolator(), 2500));
+ return animatorSet;
+ }
+ });
+ put(R.id.menu_action_bounce_interpolator, new AnimatorBuilder() {
+ @Override
+ public Animator build() {
+ AnimatorSet bounceAnimatorSet = new AnimatorSet();
+ bounceAnimatorSet.playTogether(
+ createLatLngAnimator(START_LAT_LNG, new LatLng(37.787947, -122.407432)),
+ createExampleInterpolator(new BounceInterpolator(), 3750)
+ );
+ return bounceAnimatorSet;
+ }
+ });
+ put(R.id.menu_action_anticipate_overshoot_interpolator, new AnimatorBuilder() {
+ @Override
+ public Animator build() {
+ return createExampleInterpolator(new AnticipateOvershootInterpolator(), 2500);
+ }
+ });
+ put(R.id.menu_action_path_interpolator, new AnimatorBuilder() {
+ @Override
+ public Animator build() {
+ return createExampleInterpolator(PathInterpolatorCompat.create(.22f, .68f, 0, 1.71f), 2500);
+ }
+ });
}
- return null;
+ };
+
+ interface AnimatorBuilder {
+ Animator build();
+ }
+
+ private Animator createExamepleInterpolator(int menuItemId) {
+ return ANIMATORS.get(menuItemId).build();
}
private Animator createExampleInterpolator(Interpolator interpolator, long duration) {
What do you think @tobrun?
50d781a
to
1c5969b
Compare
@Guardiola31337 neat, has been adapted, additionally changed target branch to release-agua. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor detail.
These examples are awesome 🚀
|
||
private final LatLng latLng = new LatLng(); | ||
private Animator createExampleInterpolator(int menuItemId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor thing, what about changing the name of the method here to obtainExampleAnimator(int menuItemId)
instead? We're not actually creating the animators here and there's another method with the same exact name (createExampleInterpolator(Interpolator interpolator, long duration)
) which makes it kinda confusing.
1c5969b
to
43e1ba3
Compare
This PR adds a couple of examples around using animators and interpolators.
cc @mapbox/android