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.

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

Add ETA support for the notification back #1184

Merged
merged 1 commit into from
Aug 3, 2018

Conversation

Guardiola31337
Copy link
Contributor

In #1115 we removed the ETA from arrivalTime in MapboxNavigationNotification by mistake.

This PR brings that functionality back.

@Guardiola31337 Guardiola31337 added this to the 0.17.0 milestone Aug 3, 2018
@Guardiola31337 Guardiola31337 self-assigned this Aug 3, 2018
Copy link
Contributor

@danesfeder danesfeder left a comment

Choose a reason for hiding this comment

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

@Guardiola31337 great catch, one minor comment 🚀

@@ -187,8 +190,9 @@ private void updateArrivalTime(RouteProgress routeProgress) {
double durationRemaining = routeProgress.durationRemaining();
int timeFormatType = options.timeFormatType();
String arrivalTime = formatTime(time, durationRemaining, timeFormatType, isTwentyFourHourFormat);
collapsedNotificationRemoteViews.setTextViewText(R.id.notificationArrivalText, arrivalTime);
expandedNotificationRemoteViews.setTextViewText(R.id.notificationArrivalText, arrivalTime);
String formattedArrivalText = String.format(Locale.getDefault(), etaFormat, arrivalTime);
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 be better to get the Configuration Locale using LocaleUtils here? We can do this during initialize. I believe we are doing this everywhere else within the SDK, so it would be great to stay consistent.

Copy link
Contributor

@danesfeder danesfeder left a comment

Choose a reason for hiding this comment

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

@Guardiola31337 thanks for the clarification 🚀 🚢

@Guardiola31337 Guardiola31337 merged commit d52c8d7 into master Aug 3, 2018
@Guardiola31337 Guardiola31337 deleted the pg-notification-eta branch August 3, 2018 16:31
@Guardiola31337 Guardiola31337 mentioned this pull request Aug 10, 2018
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Defect to be fixed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants