Skip to content

Rebuild MapboxNavigationNotification for each update#1455

Merged
danesfeder merged 1 commit into
masterfrom
dan-rebuild-notification
Oct 24, 2018
Merged

Rebuild MapboxNavigationNotification for each update#1455
danesfeder merged 1 commit into
masterfrom
dan-rebuild-notification

Conversation

@danesfeder
Copy link
Copy Markdown
Contributor

Fixes #1188 and Closes #1441

See #1441 for further explanation / discussion.

@danesfeder danesfeder added bug Defect to be fixed. ✓ ready for review labels Oct 24, 2018
@danesfeder danesfeder self-assigned this Oct 24, 2018
Copy link
Copy Markdown
Contributor

@Guardiola31337 Guardiola31337 left a comment

Choose a reason for hiding this comment

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

Thanks @danesfeder for implementing the suggestions from #1441 (comment)

Let's give this a try and see if we don't see that TransactionTooLargeException 🐛 anymore!

🚢 🚢

private int currentManeuverId;
private boolean isTwentyFourHourFormat;
private String etaFormat;
private final Context context;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor comment - to note that should be an application context, what about calling it applicationContext instead? As in

@Danny-James
Copy link
Copy Markdown

@danesfeder The data in the Notification is still not matching the BannerText how can I force an update of the Notification?

And is their also a way to prevent the Notification from showing at all?

screenshot_20181106-105038_route info
screenshot_20181106-105042_route info

@danesfeder
Copy link
Copy Markdown
Contributor Author

@Danny-James
Copy link
Copy Markdown

@danesfeder Instead of creating my own, is their a documented way of stopping the default notification, as i don't think we need it to be honest?

But if we do in the future at least i'm aware on disabling the default and/or creating my own.

@danesfeder
Copy link
Copy Markdown
Contributor Author

@Danny-James we had to remove the option to disable because on Oreo+ a ForegroundService must be accompanied with a notification due to it being the most aggressive service you can run on Android. We do this so navigation doesn't get killed by the system in the background.

@Danny-James
Copy link
Copy Markdown

@danesfeder does the default notification have a layout file I could replicate?

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.

3 participants