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 delegate methods for tapping route duration. #4133

Merged
merged 2 commits into from
Sep 28, 2022

Conversation

ShanMa1991
Copy link
Contributor

Description

This PR is to fix #3847 by adding the ability for users to detect the taps on duration annotations for routes and continuous alternative routes.

Implementation

When users tapped the NavigationMapView, the NavigationMapView will check whether duration annotations for routes and continuous alternative routes have been tapped. If users tapped duration annotations, it will notify the NavigationMapViewDelegate.navigationMapView(_: didTap:) about the taps. If not, it will check the taps on Waypoints, the closest route and the closest continuous alternative route.

The NavigationMapViewDelegate.navigationMapView(_: didTap:) provides the route or continuous alternative route whose duration annotation has been tapped.

Screenshots or Gifs

@ShanMa1991 ShanMa1991 added the UI Work related to visual components, Android Auto, Camera, 3D, voice, etc. label Sep 7, 2022
@ShanMa1991 ShanMa1991 added this to the v2.8 milestone Sep 7, 2022
@ShanMa1991 ShanMa1991 modified the milestones: v2.8, v2.9 Sep 15, 2022
@ShanMa1991 ShanMa1991 force-pushed the shan/tap-duration-3847 branch from 88cb6ad to 26b7517 Compare September 16, 2022 16:07
@ShanMa1991 ShanMa1991 marked this pull request as ready for review September 16, 2022 22:13
@ShanMa1991 ShanMa1991 requested a review from a team September 16, 2022 22:13
@ShanMa1991 ShanMa1991 force-pushed the shan/tap-duration-3847 branch from 863a6a3 to fad1796 Compare September 19, 2022 21:57
Example/ViewController.swift Outdated Show resolved Hide resolved
for layerId in layerIds {
group.enter()
let options = RenderedQueryOptions(layerIds: [layerId], filter: nil)
routeIndexFromMapQuery(with: options, at: point) { [weak self] (routeIndex) in
Copy link
Contributor

Choose a reason for hiding this comment

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

The user would tap on what visuallly appears to be the callout bubble. However, the callout bubble is offset from the actual feature data. This call queries for feature data located at the tap point, which would be some distance away from where the feature data is actually located.

For this approach to be more reliable than routes(closeTo:), we’d technically need to figure out the callout bubble’s screen rect. Alternatively, since we’re filtering to just the annotation layers, we could query for all the features in the current viewport, then select the result that’s closest to the tap point.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ShanMa1991 confirmed that these days, MapboxMap.queryRenderedFeatures(at:options:completion:) actually does account for label bounds and offsets. That’s a big deal. I think it resolves this concern and probably also some other issues in our backlog.

/ref #4165 (comment)

Copy link
Contributor

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

A couple things that’ll prevent confusion down the line, but otherwise good to go.

CHANGELOG.md Outdated Show resolved Hide resolved
Sources/MapboxNavigation/NavigationMapView.swift Outdated Show resolved Hide resolved
Sources/MapboxNavigation/NavigationMapView.swift Outdated Show resolved Hide resolved
@@ -2120,12 +2120,60 @@ open class NavigationMapView: UIView {
if let selected = waypointTest?.first {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method becomes more complex with time, I wonder if we could cover it with unit-tests to make sure that delegate methods are called based on expected priority.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right now it still follows the previous sequence, but between the waypoints and route, it would check the duration annotation. I'll add test for this one later.

@ShanMa1991 ShanMa1991 force-pushed the shan/tap-duration-3847 branch from b955ec0 to 49d50ee Compare September 28, 2022 19:03
@ShanMa1991 ShanMa1991 merged commit 24d6403 into main Sep 28, 2022
@ShanMa1991 ShanMa1991 deleted the shan/tap-duration-3847 branch September 28, 2022 19:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI Work related to visual components, Android Auto, Camera, 3D, voice, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement the ability to detect taps on the route duration annotations.
3 participants