Clean up delegate method names; use UUIDs for feedback IDs #1413
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Named delegate method parameters in Objective-C. Replaced the first argument of several delegate methods with the sending object. Made method names more consistent between NavigationMapViewDelegate, RouteMapViewControllerDelegate, and NavigationViewControllerDelegate. Represent feedback IDs as UUID instead of String, just like in Core Navigation. Renamed “callback” to “completionHandler”. Fixed an issue where NavigationViewControllerDelegate.navigationMapView(_:shapeFor:) was getting called instead of navigationMapView(_:simplifiedShapeFor:). Fixed typos in documentation comments. Separated ViewController delegate implementations into extensions.
1ec5
added
bug
bsudekum
reviewed
May 10, 2018
| @@ -28,11 +29,27 @@ | |||
| ### Other changes | |||
|
|
|||
| * `DistanceFormatter`, `ReplayLocationManager`, `SimulatedLocationManager`, `LanesView`, and `ManueverView` are now subclassable. ([#1345](https://github.com/mapbox/mapbox-navigation-ios/pull/1345])) | |||
| * Renamed several `NavigationMapViewDelegate` methods ([#1364](https://github.com/mapbox/mapbox-navigation-ios/pull/1364), [#1338](https://github.com/mapbox/mapbox-navigation-ios/pull/1338), [#1318](https://github.com/mapbox/mapbox-navigation-ios/pull/1318), [#1378](https://github.com/mapbox/mapbox-navigation-ios/pull/1378)): | |||
| * Renamed many `NavigationViewController` and `NavigationMapViewDelegate` methods ([#1364](https://github.com/mapbox/mapbox-navigation-ios/pull/1364), [#1338](https://github.com/mapbox/mapbox-navigation-ios/pull/1338), [#1318](https://github.com/mapbox/mapbox-navigation-ios/pull/1318), [#1378](https://github.com/mapbox/mapbox-navigation-ios/pull/1378), [#1413](https://github.com/mapbox/mapbox-navigation-ios/pull/1413)): | |||
| * `NavigationViewControllerDelegate.navigationViewControllerDidCancelNavigation(_:)` to `NavigationViewControllerDelegate.navigationViewControllerDidDismiss(_:byCanceling:)` | |||
There was a problem hiding this comment.
This list pretty huge, maybe put it behind a more/toggle button thing.
There was a problem hiding this comment.
This changelog gets included in the jazzy docset’s cover page. Not sure whether jazzy’s Markdown formatter can handle <details>. Open to using a table, but then the concern is horizontal scrolling that could break page layouts.
There was a problem hiding this comment.
Yeah I think a table might help.
bsudekum
approved these changes
May 10, 2018
There was a problem hiding this comment.
Looks good, let's also test how this fairs in https://github.com/mapbox/navigation-ios-examples.
|
Updated examples based on mapbox/mapbox-navigation-ios-examples#20. |
Merged
1ec5
added
the
backwards incompatible
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This one is a doozy. Hot on the heels of #1378, most public NavigationViewControllerDelegate methods and some RouteControllerDelegate and TunnelIntersectionManagerDelegate methods have been renamed to conform to Cocoa naming conventions in both Swift and Objective-C. The biggest change is that all the NavigationViewControllerDelegate methods now begin with
navigationViewControllerinstead of sometimes beginning withnavigationMapView. If an implementation needs access to the map view, it can usenavigationViewController.mapView.NavigationViewControllerDelegate changes:
navigationMapView(_:routeStyleLayerWithIdentifier:source:)navigationViewController(_:routeStyleLayerWithIdentifier:source:)navigationMapView(_:routeCasingStyleLayerWithIdentifier:source:)navigationViewController(_:routeCasingStyleLayerWithIdentifier:source:)navigationMapView(_:shapeFor:)navigationViewController(_:shapeFor:)navigationMapView(_:simplifiedShapeFor:)navigationViewController(_:simplifiedShapeFor:)navigationMapView(_:waypointStyleLayerWithIdentifier:source:)navigationViewController(_:waypointStyleLayerWithIdentifier:source:)navigationMapView(_:waypointSymbolStyleLayerWithIdentifier:source:)navigationViewController(_:waypointSymbolStyleLayerWithIdentifier:source:)navigationMapView(_:shapeFor:legIndex:)navigationViewController(_:shapeFor:legIndex:)navigationMapView(_:didTap:)navigationViewController(_:didSelect:)navigationMapView(_:imageFor:)navigationViewController(_:imageFor:)navigationMapView(_:viewFor:)navigationViewController(_:viewFor:)navigationViewController(_:didSend:feedbackType:)navigationViewController(_:didSendFeedbackAssigned:feedbackType:)NavigationViewControllerDelegate changes in Objective-C only:
-navigationViewController:didArriveAt:-navigationViewController:didArriveAtWaypoint:-navigationViewController:shouldDiscard:-navigationViewController:shouldDiscardLocation:-navigationViewController:roadNameAt:-navigationViewController:roadNameAtLocation:TunnelIntersectionManagerDelegate changes:
tunnelIntersectionManager(_:willEnableAnimationAt:callback:)tunnelIntersectionManager(_:willEnableAnimationAt:completionHandler:)tunnelIntersectionManager(_:willDisableAnimationAt:callback:)tunnelIntersectionManager(_:willDisableAnimationAt:completionHandler:)Along for the ride, MapboxNavigation now represents feedback IDs as
UUIDs instead ofStrings, just like in Core Navigation, resulting in the following changes to RouteController:recordFeedback(type:description:)returnsStringrecordFeedback(type:description:)returnsUUIDupdateFeedback(feedbackId:type:source:description:)updateFeedback(uuid:type:source:description:)cancelFeedback(feedbackId:)cancelFeedback(uuid:)Finally, this PR fixes an issue (present in the current release) where
NavigationViewController.navigationMapView(_:shapeDescribing:)(nownavigationMapView(_:shapeFor:)) was getting called instead ofnavigationMapView(_:simplifiedShapeDescribing:)(nownavigationMapView(_:simplifiedShapeFor:)).Fixes #1376.
/cc @bsudekum @JThramer @vincethecoder