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

Fix crash in EndOfRouteViewController and restore its presentation #71

Merged
merged 2 commits into from
Jun 27, 2024

Conversation

michaelkirk
Copy link
Collaborator

@michaelkirk michaelkirk commented Jun 24, 2024

This is a subset of #58, extracted and rebased now that #54 is merged.

Before #54, upon completion of a route, the EndOfRouteViewController (EORVC) would be presented. It was intentionally removed in #54, but I'd like to revisit that decision. Frankly there was so much going on in #54, that I didn't realize the implications of this particular change.

I think it's a good idea to include a reasonable "you've finished the route" UI by default. Same as before #54, it can be disabled by setting vc.showsEndOfRouteFeedback = false.

Note that there is still a separate bug where the route is only considered "completed" when the route contains audio instructions. As suggested, I'll follow up with a fix for that separately.

eorvc-default.mov.mp4

For my own App, I've overridden the appearance of the button to be a "big red button", but I left the default one styled basic (as it was before my changes):

Screenshot 2024-06-24 at 13 43 23

If people think this is a better default, I can upstream those style changes too.

Copy link
Collaborator

@hactar hactar left a comment

Choose a reason for hiding this comment

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

I'd propose a small code cycle change, but nothing blocking.

MapboxNavigation/NavigationView.swift Outdated Show resolved Hide resolved
FIXES maplibre#57 - by rewriting EORVC as programmatic VC rather than storyboard.
FIXES maplibre#62 - Omitted the "Rate this route" feedback while rewriting the EORVC.

Note this was cherry-picked, and no longer works due to change in maplibre#54
Where possible, the particulars of presentation were copied from
b790f96 (aka before maplibre#54).
@michaelkirk michaelkirk force-pushed the mkirk/show-end-of-route-2 branch from 5664935 to 6e075a1 Compare June 27, 2024 21:53
@michaelkirk michaelkirk merged commit cdb0756 into maplibre:main Jun 27, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants