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

Locale distance formatter #668

Merged
merged 32 commits into from
Feb 7, 2018
Merged

Locale distance formatter #668

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
6d1a6e1
Refactored formatting method name
Jan 24, 2018
441759c
Added DistanceUtilsTest
Jan 24, 2018
9cd072f
Added tests to DistanceUtilsTest
Jan 24, 2018
4284cab
Set spanEnabled to true in only place that it was set to false. This …
Jan 24, 2018
d6dde29
Changed SpannableStringBuilder to SpannableString
Jan 24, 2018
5728d4d
Removed decimalFormat and spanEnabled and added locale to DistanceUtils
Jan 24, 2018
c86ac35
Did refactoring and simlified code, and added tests for locale
Jan 24, 2018
26ba2b5
Updated all usages of DistanceUtils to use locale
Jan 24, 2018
bb93d6a
Fixed checkstyle issues
Jan 24, 2018
62e1fbd
Moved toString so that SummaryModel.getDistanceRemaining returns a st…
Jan 25, 2018
2feb26b
Merge remote-tracking branch 'origin/master' into devota-locale-dista…
Jan 25, 2018
1c116c5
Made fields final
Jan 25, 2018
dd18e3d
Removed whitespace
Jan 25, 2018
8d47efb
Rearranged fields
Jan 25, 2018
701a226
Made fields final
Jan 25, 2018
1473409
Removed whitespace
Jan 25, 2018
4e103e1
Formatted tests
Jan 25, 2018
db1380b
Added unit abbreviations to strings.xml for future localization. Also…
Jan 25, 2018
21a5fbe
Added context to all calls to DistanceUtils
Jan 25, 2018
9d85ef5
Used only context as parameter of DistanceUtils
Jan 25, 2018
95ab03b
Added special handling for locale and unit types and added tests for …
Jan 26, 2018
98970e5
Fixed checkstyle issues
Jan 26, 2018
dbb24e4
Removed unused locales
Jan 26, 2018
60daf6c
Merge branch 'master' into devota-locale-distance-formatter
Jan 29, 2018
80d9f36
Added LocaleUtils
Jan 29, 2018
1f603fa
Fixed typo
Jan 29, 2018
d5f80b1
Added special handling for deprecated locale
Jan 29, 2018
06ad29e
Merge remote-tracking branch 'origin/master' into devota-locale-dista…
Feb 5, 2018
4af1c51
Updated test for use of LocaleList for sdk>=N
Feb 5, 2018
35d8bfa
Removed directionsLanguage from NavigationViewOptions
Feb 7, 2018
a706541
Added setLocale helper method to LocaleUtils
Feb 7, 2018
c5bd115
Stopped storing context in InstructionListAdapter to prevent memory l…
Feb 7, 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
1 change: 1 addition & 0 deletions ...ion-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ private void extractConfiguration(NavigationViewOptions.Builder options) {
MapboxNavigationOptions navigationOptions = MapboxNavigationOptions.builder()
.unitType(preferences.getInt(NavigationConstants.NAVIGATION_VIEW_UNIT_TYPE,
NavigationUnitType.TYPE_IMPERIAL))
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be cool to get a default unit type based on the local here (if one isn't set in the preferences) with something like https://stackoverflow.com/questions/4898237/using-locale-settings-to-detect-wheter-to-use-imperial-units.

Current catch is the unit type isn't currently nullable in MapboxNavigationOptions, so we can switch that to @Nullable and if someone doesn't set it, we dont set the preference and get the default here when setting the default value for the SharedPreference

.locale(getResources().getConfiguration().locale)
Copy link
Contributor

Choose a reason for hiding this comment

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

Great call here using the configuration locale here instead of Locale.getDefault()

.build();
options.navigationOptions(navigationOptions);
}
Expand Down
22 changes: 7 additions & 15 deletions ...on-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/NavigationViewModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;
import com.mapbox.services.android.telemetry.location.LocationEngine;

import java.text.DecimalFormat;
import java.util.Locale;

public class NavigationViewModel extends AndroidViewModel implements ProgressChangeListener,
OffRouteListener, MilestoneEventListener, NavigationEventListener, FasterRouteListener {
Expand All @@ -52,21 +52,21 @@ public class NavigationViewModel extends AndroidViewModel implements ProgressCha
final MutableLiveData<Boolean> isRunning = new MutableLiveData<>();
final MutableLiveData<Boolean> shouldRecordScreenshot = new MutableLiveData<>();

private final SharedPreferences preferences;
private final Locale locale;
private MapboxNavigation navigation;
private NavigationInstructionPlayer instructionPlayer;
private ConnectivityManager connectivityManager;
private SharedPreferences preferences;
private DecimalFormat decimalFormat;
private int unitType;
private String feedbackId;
private String screenshot;

public NavigationViewModel(Application application) {
super(application);
preferences = PreferenceManager.getDefaultSharedPreferences(application);
locale = application.getResources().getConfiguration().locale;
initVoiceInstructions(application);
initConnectivityManager(application);
initDecimalFormat();
}

public void onDestroy() {
Expand All @@ -86,8 +86,8 @@ public void onDestroy() {
*/
@Override
public void onProgressChange(Location location, RouteProgress routeProgress) {
instructionModel.setValue(new InstructionModel(routeProgress, decimalFormat, unitType));
summaryModel.setValue(new SummaryModel(routeProgress, decimalFormat, unitType));
instructionModel.setValue(new InstructionModel(routeProgress, locale, unitType));
summaryModel.setValue(new SummaryModel(routeProgress, locale, unitType));
navigationLocation.setValue(location);
}

Expand Down Expand Up @@ -262,14 +262,6 @@ private void initConnectivityManager(Application application) {
connectivityManager = (ConnectivityManager) application.getSystemService(Context.CONNECTIVITY_SERVICE);
}

/**
* Initializes decimal format to be used to populate views with
* distance remaining.
*/
private void initDecimalFormat() {
decimalFormat = new DecimalFormat(NavigationConstants.DECIMAL_FORMAT);
}

/**
* Adds this class as a listener for progress,
* milestones, and off route events.
Expand Down Expand Up @@ -333,7 +325,7 @@ private boolean hasNetworkConnection() {
private void updateBannerInstruction(RouteProgress routeProgress, Milestone milestone) {
if (milestone instanceof BannerInstructionMilestone) {
bannerInstructionModel.setValue(new BannerInstructionModel((BannerInstructionMilestone) milestone,
routeProgress, decimalFormat, unitType));
routeProgress, locale, unitType));
}
}
}
6 changes: 3 additions & 3 deletions ...java/com/mapbox/services/android/navigation/ui/v5/instruction/BannerInstructionModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,15 @@
import com.mapbox.services.android.navigation.v5.milestone.BannerInstructionMilestone;
import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;

import java.text.DecimalFormat;
import java.util.Locale;

public class BannerInstructionModel extends InstructionModel {

private BannerInstructionMilestone milestone;

public BannerInstructionModel(BannerInstructionMilestone milestone, RouteProgress progress,
DecimalFormat decimalFormat, int unitType) {
super(progress, decimalFormat, unitType);
Locale locale, int unitType) {
super(progress, locale, unitType);
this.milestone = milestone;
}

Expand Down
10 changes: 5 additions & 5 deletions .../main/java/com/mapbox/services/android/navigation/ui/v5/instruction/InstructionModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
import com.mapbox.services.android.navigation.v5.navigation.NavigationUnitType;
import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;

import java.text.DecimalFormat;
import java.util.List;
import java.util.Locale;

public class InstructionModel {

Expand All @@ -17,11 +17,11 @@ public class InstructionModel {
private RouteProgress progress;
private int unitType;

public InstructionModel(RouteProgress progress, DecimalFormat decimalFormat,
public InstructionModel(RouteProgress progress, Locale locale,
@NavigationUnitType.UnitType int unitType) {
this.progress = progress;
this.unitType = unitType;
buildInstructionModel(progress, decimalFormat, unitType);
buildInstructionModel(progress, locale);
}

BannerText getPrimaryBannerText() {
Expand All @@ -48,8 +48,8 @@ int getUnitType() {
return unitType;
}

private void buildInstructionModel(RouteProgress progress, DecimalFormat decimalFormat, int unitType) {
stepResources = new InstructionStepResources(progress, decimalFormat, unitType);
private void buildInstructionModel(RouteProgress progress, Locale locale) {
stepResources = new InstructionStepResources(progress, locale, unitType);
extractStepInstructions(progress);
}

Expand Down
18 changes: 9 additions & 9 deletions ...va/com/mapbox/services/android/navigation/ui/v5/instruction/InstructionStepResources.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,32 @@
package com.mapbox.services.android.navigation.ui.v5.instruction;

import android.text.SpannableStringBuilder;
import android.text.SpannableString;

import com.mapbox.api.directions.v5.models.IntersectionLanes;
import com.mapbox.api.directions.v5.models.LegStep;
import com.mapbox.api.directions.v5.models.StepIntersection;
import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;
import com.mapbox.services.android.navigation.v5.utils.DistanceUtils;

import java.text.DecimalFormat;
import java.util.List;
import java.util.Locale;

class InstructionStepResources {

private SpannableStringBuilder stepDistanceRemaining;
private SpannableString stepDistanceRemaining;
private String maneuverViewModifier;
private String maneuverViewType;
private String thenStepManeuverModifier;
private String thenStepManeuverType;
private boolean shouldShowThenStep;
private List<IntersectionLanes> turnLanes;

InstructionStepResources(RouteProgress progress, DecimalFormat decimalFormat, int unitType) {
formatStepDistance(progress, decimalFormat, unitType);
InstructionStepResources(RouteProgress progress, Locale locale, int unitType) {
formatStepDistance(progress, locale, unitType);
extractStepResources(progress);
}

SpannableStringBuilder getStepDistanceRemaining() {
SpannableString getStepDistanceRemaining() {
return stepDistanceRemaining;
}

Expand Down Expand Up @@ -79,9 +79,9 @@ private void extractStepResources(RouteProgress progress) {
}
}

private void formatStepDistance(RouteProgress progress, DecimalFormat decimalFormat, int unitType) {
stepDistanceRemaining = DistanceUtils.distanceFormatter(progress.currentLegProgress()
.currentStepProgress().distanceRemaining(), decimalFormat, true, unitType);
private void formatStepDistance(RouteProgress progress, Locale locale, int unitType) {
stepDistanceRemaining = DistanceUtils.formatDistance(progress.currentLegProgress()
.currentStepProgress().distanceRemaining(), locale, unitType);
}

private void intersectionTurnLanes(LegStep step) {
Expand Down
18 changes: 5 additions & 13 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 @@ -51,7 +51,7 @@
import com.mapbox.services.android.navigation.v5.routeprogress.ProgressChangeListener;
import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;

import java.text.DecimalFormat;
import java.util.Locale;

/**
* A view that can be used to display upcoming maneuver information and control
Expand All @@ -70,6 +70,7 @@
public class InstructionView extends RelativeLayout implements FeedbackBottomSheetListener {

public boolean isMuted;
private final Locale locale;
private ManeuverView upcomingManeuverView;
private TextView upcomingDistanceText;
private TextView upcomingPrimaryText;
Expand All @@ -93,7 +94,6 @@ public class InstructionView extends RelativeLayout implements FeedbackBottomShe
private Animation rerouteSlideUpTop;
private Animation rerouteSlideDownTop;
private AnimationSet fadeInSlowOut;
private DecimalFormat decimalFormat;
private LegStep currentStep;
private NavigationViewModel navigationViewModel;
private boolean isRerouting;
Expand All @@ -108,6 +108,7 @@ public InstructionView(Context context, @Nullable AttributeSet attrs) {

public InstructionView(Context context, @Nullable AttributeSet attrs, int defStyleAttr) {
super(context, attrs, defStyleAttr);
locale = context.getResources().getConfiguration().locale;
Copy link
Contributor

Choose a reason for hiding this comment

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

Replace with extraction from SharedPreferences

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 was thinking just pass the context to DistanceUtils, and then DistanceUtils can get the locale from SharedPreferences

Copy link
Contributor

Choose a reason for hiding this comment

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

@devotaaabel yeah that sounds good to me - just need to make sure we aren't calling DistanceUtils somewhere where we don't have Context. Or have a backup method that can be called without context

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 To localize the strings like Eric said we'll need the context, but it shouldn't be an issue anyway

init();
}

Expand All @@ -124,7 +125,6 @@ protected void onFinishInflate() {
initBackground();
initTurnLaneRecyclerView();
initDirectionsRecyclerView();
initDecimalFormat();
initAnimations();
}

Expand Down Expand Up @@ -198,7 +198,7 @@ public void onChanged(@Nullable Boolean isOffRoute) {
@SuppressWarnings("UnusedDeclaration")
public void update(RouteProgress routeProgress, @NavigationUnitType.UnitType int unitType) {
if (routeProgress != null && !isRerouting) {
InstructionModel model = new InstructionModel(routeProgress, decimalFormat, unitType);
InstructionModel model = new InstructionModel(routeProgress, locale, unitType);
updateViews(model);
updateTextInstruction(model);
}
Expand Down Expand Up @@ -487,22 +487,14 @@ private void initTurnLaneRecyclerView() {
* Sets up the {@link RecyclerView} that is used to display the list of instructions.
*/
private void initDirectionsRecyclerView() {
instructionListAdapter = new InstructionListAdapter();
instructionListAdapter = new InstructionListAdapter(locale);
rvInstructions.setAdapter(instructionListAdapter);
rvInstructions.setHasFixedSize(true);
rvInstructions.setNestedScrollingEnabled(true);
rvInstructions.setItemAnimator(new DefaultItemAnimator());
rvInstructions.setLayoutManager(new LinearLayoutManager(getContext()));
}

/**
* Initializes decimal format to be used to populate views with
* distance remaining.
*/
private void initDecimalFormat() {
decimalFormat = new DecimalFormat(NavigationConstants.DECIMAL_FORMAT);
}

/**
* Initializes all animations needed to show / hide views.
*/
Expand Down
18 changes: 4 additions & 14 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 @@ -13,12 +13,12 @@

import com.mapbox.services.android.navigation.ui.v5.NavigationViewModel;
import com.mapbox.services.android.navigation.ui.v5.R;
import com.mapbox.services.android.navigation.v5.navigation.NavigationConstants;
import com.mapbox.services.android.navigation.v5.navigation.NavigationUnitType;
import com.mapbox.services.android.navigation.v5.routeprogress.ProgressChangeListener;
import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;

import java.text.DecimalFormat;
import java.util.Locale;

/**
* A view with {@link android.support.design.widget.BottomSheetBehavior}
Expand All @@ -32,13 +32,11 @@
public class SummaryBottomSheet extends FrameLayout {

private static final String EMPTY_STRING = "";

private final Locale locale;
private TextView distanceRemainingText;
private TextView timeRemainingText;
private TextView arrivalTimeText;
private ProgressBar rerouteProgressBar;

private DecimalFormat decimalFormat;
private boolean isRerouting;

public SummaryBottomSheet(Context context) {
Expand All @@ -51,6 +49,7 @@ public SummaryBottomSheet(Context context, AttributeSet attrs) {

public SummaryBottomSheet(Context context, AttributeSet attrs, int defStyleAttr) {
super(context, attrs, defStyleAttr);
locale = context.getResources().getConfiguration().locale;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here with the SharedPref deal

init();
}

Expand All @@ -63,7 +62,6 @@ public SummaryBottomSheet(Context context, AttributeSet attrs, int defStyleAttr)
protected void onFinishInflate() {
super.onFinishInflate();
bind();
initDecimalFormat();
}

public void subscribe(NavigationViewModel navigationViewModel) {
Expand Down Expand Up @@ -103,7 +101,7 @@ public void onChanged(@Nullable Boolean isOffRoute) {
@SuppressWarnings("UnusedDeclaration")
public void update(RouteProgress routeProgress, @NavigationUnitType.UnitType int unitType) {
if (routeProgress != null && !isRerouting) {
SummaryModel model = new SummaryModel(routeProgress, decimalFormat, unitType);
SummaryModel model = new SummaryModel(routeProgress, locale, unitType);
arrivalTimeText.setText(model.getArrivalTime());
timeRemainingText.setText(model.getTimeRemaining());
distanceRemainingText.setText(model.getDistanceRemaining());
Expand Down Expand Up @@ -149,14 +147,6 @@ private void bind() {
rerouteProgressBar = findViewById(R.id.rerouteProgressBar);
}

/**
* Initializes decimal format to be used to populate views with
* distance remaining.
*/
private void initDecimalFormat() {
decimalFormat = new DecimalFormat(NavigationConstants.DECIMAL_FORMAT);
}

/**
* Clears all {@link View}s.
*/
Expand Down
18 changes: 8 additions & 10 deletions ...n-ui/src/main/java/com/mapbox/services/android/navigation/ui/v5/summary/SummaryModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,27 +5,25 @@
import com.mapbox.services.android.navigation.v5.navigation.NavigationUnitType;
import com.mapbox.services.android.navigation.v5.routeprogress.RouteProgress;

import java.text.DecimalFormat;
import java.util.Locale;

import static com.mapbox.services.android.navigation.v5.utils.DistanceUtils.distanceFormatter;
import static com.mapbox.services.android.navigation.v5.utils.DistanceUtils.formatDistance;
import static com.mapbox.services.android.navigation.v5.utils.time.TimeUtils.formatArrivalTime;
import static com.mapbox.services.android.navigation.v5.utils.time.TimeUtils.formatTimeRemaining;

public class SummaryModel {

private SpannableStringBuilder distanceRemaining;
private SpannableStringBuilder timeRemaining;
private String arrivalTime;
private final String distanceRemaining;
private final SpannableStringBuilder timeRemaining;
private final String arrivalTime;

public SummaryModel(RouteProgress progress, DecimalFormat decimalFormat,
@NavigationUnitType.UnitType int unitType) {
distanceRemaining = distanceFormatter(progress.distanceRemaining(),
decimalFormat, false, unitType);
public SummaryModel(RouteProgress progress, Locale locale, @NavigationUnitType.UnitType int unitType) {
distanceRemaining = formatDistance(progress.distanceRemaining(), locale, unitType).toString();
timeRemaining = formatTimeRemaining(progress.durationRemaining());
arrivalTime = formatArrivalTime(progress.durationRemaining());
}

SpannableStringBuilder getDistanceRemaining() {
String getDistanceRemaining() {
return distanceRemaining;
}

Expand Down
Loading