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.

Sign up for GitHub

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

Jump to bottom

Fixed unit type bug #769

Merged
merged 4 commits into from
Mar 16, 2018
Merged

Fixed unit type bug #769

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8448171
Fixed unit type bug and cleaned up some code
Mar 15, 2018
495ae4b
Added logic to save DistanceUtils but update if locale or unitType ar…
Mar 16, 2018
2df3634
Merge remote-tracking branch 'origin/master' into devota-unit-type-bug
Mar 16, 2018
e8b288b
Merge remote-tracking branch 'origin/master' into devota-unit-type-bug
Mar 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions ...igation-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,10 +370,7 @@ public void startNavigation(NavigationViewOptions options) {
}

private void setLocale(NavigationViewOptions options) {
Locale locale = options.navigationOptions().locale();
if (locale == null) {
locale = LocaleUtils.getDeviceLocale(getContext());
}
Locale locale = LocaleUtils.getNonNullLocale(getContext(), options.navigationOptions().locale());
@NavigationUnitType.UnitType int unitType = options.navigationOptions().unitType();

instructionView.setLocale(locale);
Expand Down
12 changes: 6 additions & 6 deletions ...c/main/java/com/mapbox/services/android/navigation/ui/v5/instruction/InstructionView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import android.graphics.drawable.Drawable;
import android.os.Build;
import android.os.Handler;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.constraint.ConstraintLayout;
import android.support.constraint.ConstraintSet;
Expand Down Expand Up @@ -211,7 +212,6 @@ public void onChanged(@Nullable Boolean isOffRoute) {
@SuppressWarnings("UnusedDeclaration")
public void update(RouteProgress routeProgress) {
if (routeProgress != null && !isRerouting) {
locale = LocaleUtils.getNonNullLocale(getContext(), locale);
InstructionModel model =
new InstructionModel(getContext(), routeProgress, locale, unitType);
updateViews(model);
Expand Down Expand Up @@ -320,9 +320,10 @@ public void showInstructionList() {
}

/**
* Inflates this layout needed for this view.
* Inflates this layout needed for this view and initializes the locale as the device locale.
*/
private void init() {
locale = LocaleUtils.getDeviceLocale(getContext());
inflate(getContext(), R.layout.instruction_view_layout, this);
}

Expand Down Expand Up @@ -495,8 +496,7 @@ private void initTurnLaneRecyclerView() {
* Sets up the {@link RecyclerView} that is used to display the list of instructions.
*/
private void initDirectionsRecyclerView() {
locale = LocaleUtils.getNonNullLocale(getContext(), locale);
instructionListAdapter = new InstructionListAdapter(getContext(), locale, unitType);
instructionListAdapter = new InstructionListAdapter();
rvInstructions.setAdapter(instructionListAdapter);
rvInstructions.setHasFixedSize(true);
rvInstructions.setNestedScrollingEnabled(true);
Expand Down Expand Up @@ -814,15 +814,15 @@ private void updateLandscapeConstraintsTo(int layoutRes) {
* @param model to provide the current steps and unit type
*/
private void updateInstructionList(InstructionModel model) {
instructionListAdapter.updateSteps(model.getProgress());
instructionListAdapter.updateSteps(getContext(), model.getProgress(), locale, unitType);
Copy link
Contributor

Choose a reason for hiding this comment

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

Was the locale and unitType moved here so they can be updated on the fly and the list will reflect the latest changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danesfeder yes because otherwise, the locale was being set in the adapter when it was initialized, which was before the locale was actually set in InstructionView

}

/**
* Sets the locale to use for languages and default unit type
*
* @param locale to use
*/
public void setLocale(Locale locale) {
public void setLocale(@NonNull Locale locale) {
this.locale = locale;
}

Expand Down
7 changes: 4 additions & 3 deletions ...rc/main/java/com/mapbox/services/android/navigation/ui/v5/summary/SummaryBottomSheet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import android.arch.lifecycle.LifecycleOwner;
import android.arch.lifecycle.Observer;
import android.content.Context;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.v7.widget.RecyclerView;
import android.util.AttributeSet;
Expand Down Expand Up @@ -101,7 +102,6 @@ public void onChanged(@Nullable Boolean isOffRoute) {
@SuppressWarnings("UnusedDeclaration")
public void update(RouteProgress routeProgress) {
if (routeProgress != null && !isRerouting) {
locale = LocaleUtils.getNonNullLocale(getContext(), locale);
SummaryModel model = new SummaryModel(getContext(), routeProgress, locale, unitType);
arrivalTimeText.setText(model.getArrivalTime());
timeRemainingText.setText(model.getTimeRemaining());
Expand Down Expand Up @@ -132,9 +132,10 @@ public void hideRerouteState() {
}

/**
* Inflates this layout needed for this view.
* Inflates this layout needed for this view and initializes the locale as the device locale.
*/
private void init() {
locale = LocaleUtils.getDeviceLocale(getContext());
inflate(getContext(), R.layout.summary_bottomsheet_layout, this);
}

Expand All @@ -161,7 +162,7 @@ private void clearViews() {
* Sets the locale to use for languages and default unit type
* @param locale to use
*/
public void setLocale(Locale locale) {
public void setLocale(@NonNull Locale locale) {
this.locale = locale;
}

Expand Down
10 changes: 6 additions & 4 deletions ...ava/com/mapbox/services/android/navigation/ui/v5/summary/list/InstructionListAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,8 @@ public class InstructionListAdapter extends RecyclerView.Adapter<InstructionView
private LegStep currentStep;
private DistanceUtils distanceUtils;

public InstructionListAdapter(Context context,
Locale locale, @NavigationUnitType.UnitType int unitType) {
public InstructionListAdapter() {
stepList = new ArrayList<>();
distanceUtils = new DistanceUtils(context, locale, unitType);
}

@Override
Expand Down Expand Up @@ -70,7 +68,11 @@ public void onViewDetachedFromWindow(InstructionViewHolder holder) {
holder.itemView.clearAnimation();
}

public void updateSteps(RouteProgress routeProgress) {
public void updateSteps(Context context, RouteProgress routeProgress,
Locale locale, @NavigationUnitType.UnitType int unitType) {
if (distanceUtils == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to store locale or unitType and then check if they have changed / re-initialize DistanceUtils here if they have? I believe the formatting elsewhere (like in InstructionView) can change on the fly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@danesfeder I agree, we should probably add something like that to InstructionStepResource too so we're not creating a new DistanceUtils object everytime

distanceUtils = new DistanceUtils(context, locale, unitType);
}
addLegSteps(routeProgress);
updateStepList(routeProgress);
}
Expand Down