Swipe down to reveal steps list #1150

vincethecoder · 2018-02-22T00:37:27Z

Added functionality to announce current navigation instructions when user puck is tapped.

Issue addressed: #979

PS: Ongoing discussion on what should respond to user interaction with map interface.

cc @mapbox/navigation-ios

TODO:

Reveal the step table upon swiping downwards from the banner, leaving in place the current tap gesture.

# Conflicts: # MapboxNavigation/NavigationMapView.swift

bsudekum · 2018-02-23T16:53:06Z

@vincethecoder how shall we proceed here? Per voice yesterday, we discussed this probably is not the best user interaction since it's pretty hidden.

vincethecoder · 2018-02-23T19:33:27Z

@bsudekum Yes, ideally it makes sense to repeat the instructions when the user taps the instructionsBannerView and then have tiny list icon at then top-right corner of the instructionsBannerView to show the remaining instructions. I believe that's more user friendly.

…Added preview instructions button and action outlet to respond when button is tapped.

vincethecoder · 2018-02-26T20:43:39Z

Added a step icon between volume and feedback floating buttons at the right hand side of the map view.

1ec5 · 2018-02-26T21:02:26Z

Added a step icon between volume and feedback floating buttons at the right hand side of the map view.

This might not be discoverable/tappable enough for something as crucial as getting the list of steps. Can we nest the button in the banner somehow?

bsudekum · 2018-03-01T22:18:44Z

Per conversation today in chat, we came to the conclusion that we should change this to:

tapping top banner repeats instruction
swiping down on top banner shows steps list

# Conflicts: # MapboxNavigation/RouteMapViewController.swift

vincethecoder · 2018-03-07T05:50:03Z

As referenced from the ongoing conversation on slack channel, we came to a final conclusion today.

Add a subtle handle to the bottom of the banner, as a visual cue that the banner reveals additional steps, similar to what Apple and Google do.
Reveal the step table upon swiping downwards from the banner, leaving in place the current tap gesture.

Issue reference: #1138 (comment)

bsudekum · 2018-03-07T17:49:25Z

I'm getting a crash, to reproduce:

start with system muted
move along a step
unmute
tap top banner
asserts here

mapbox-navigation-ios/MapboxNavigation/MapboxVoiceController.swift

Line 79 in 0c0a769

assert(routeProgress != nil, "routeProgress should not be nil.")

This value is set here:

mapbox-navigation-ios/MapboxNavigation/RouteVoiceController.swift

Lines 208 to 210 in 0c0a769

    
           guard !NavigationSettings.shared.voiceMuted else { return } 
        
           routeProgress = notification.userInfo![RouteControllerNotificationUserInfoKey.routeProgressKey] as? RouteProgress

And is nil because the system is muted.

vincethecoder · 2018-03-07T18:37:12Z

@bsudekum cannot reproduce. Can you share the specific route, so that I can simulate it on my branch build? Thanks.

bsudekum · 2018-03-07T18:38:58Z

@vincethecoder I do not think this is specific to a route. The issue is that RouteVoiceController. routeProgress is never set while muted.

vincethecoder · 2018-03-07T18:43:35Z

@bsudekum Sure. I just wanted to test the referenced crash after the change was made. It didn't crash for me. I understand why this not setting the RouteVoiceController. routeProgress will be an issue.

vincethecoder · 2018-03-07T18:54:39Z

@bsudekum moreover, I don't believe the root cause of the identified issue will be in that function scope. We need to investigate why the notification's routeProgress is set to nil prior to our assert check. A separate issue ticket needs to be raised for that.

bsudekum · 2018-03-07T19:02:31Z

It's set to nil because the this check to voiceMuted is true:

mapbox-navigation-ios/MapboxNavigation/RouteVoiceController.swift

Lines 208 to 210 in 0c0a769

    
           guard !NavigationSettings.shared.voiceMuted else { return } 
        
           routeProgress = notification.userInfo![RouteControllerNotificationUserInfoKey.routeProgressKey] as? RouteProgress

…e state of the voice muted.

bsudekum · 2018-03-08T19:22:05Z

MapboxNavigation/RouteMapViewController.swift

+        displayPreviewInstructions()
+    }
+
+    private func repeatInstructions() {


Per #1138 (comment), we discussed that the first two steps would be implemented first. No need to do anything around repeating the instruction yet.

@bsudekum done ✅. This private function will just live here till we are ready to plug it in a delegate or gesture recognizer

@vincethecoder I think we should remove it and add it when it's time to add this feature.

…fied qualifiers of InstructionsBannerViewDelegate to optional objc delegate methods.

bsudekum · 2018-03-09T20:06:59Z

MapboxNavigation/RouteVoiceController.swift

        routeProgress = notification.userInfo![RouteControllerNotificationUserInfoKey.routeProgressKey] as? RouteProgress
        assert(routeProgress != nil, "routeProgress should not be nil.")

+        guard !NavigationSettings.shared.voiceMuted else { return }


this can be undone.

@bsudekum is there a reason why we have to undo this?

bsudekum · 2018-03-13T21:21:03Z

Let's break this PR up into two PRs:

This pr which implements the gesture handlers
Adding subtle button indicator view that clues the user they can pull down.

bsudekum · 2018-03-13T21:22:35Z

MapboxNavigation/InstructionsBannerViewLayout.swift

@@ -56,6 +56,8 @@ extension BaseInstructionsBannerView {
        self.separatorView = separatorView

        addTarget(self, action: #selector(BaseInstructionsBannerView.tappedInstructionsBanner(_:)), for: .touchUpInside)


Tests are failing because this function still exists but was removed here: https://github.com/mapbox/mapbox-navigation-ios/pull/1150/files#diff-7bae0cf60f54997d0d5e86fe02b5cddbL60 and renamed to draggedInstructionsBanner

Resolved ✅

bsudekum · 2018-03-14T00:41:26Z

~~@vincethecoder when running locally, it appears that tapping the top banner no longer shows the upcoming list view; only swiping.~~

Scratch that, my pull didn't grab everything.

bsudekum

Per conversation, we're going to add the indicator button view in a followup PR.

bsudekum · 2018-03-14T15:57:06Z

@vincethecoder the changelog addition can go under the master section.

vincethecoder added 4 commits February 21, 2018 10:20

Added functionality to repeat an instruction when user puck is tapped.

3880545

Removed objc tag from user course delegate function.

901b49f

Merge branch 'master' into navigation/repeat_instruction

4370248

# Conflicts: # MapboxNavigation/NavigationMapView.swift

Added Delegate to respond to user puck tapped.

bdd79e5

vincethecoder added the ✓ ready for review label Feb 22, 2018

vincethecoder added 3 commits February 26, 2018 10:40

New feature: repeat instructions when instructions banner is tapped. …

480db44

…Added preview instructions button and action outlet to respond when button is tapped.

Added a placeholder image for preview instructions.

e14af2f

Removed delegate methods used by the User Course Puck View.

5663dbb

Updated assets to use a preview directions icon.

a0171d8

vincethecoder changed the title ~~Repeat Current Instructions when User Puck is Tapped.~~ Repeat Current Instructions - [Onging Discussion] Mar 6, 2018

vincethecoder changed the title ~~Repeat Current Instructions - [Onging Discussion]~~ Repeat Current Instructions - [Ongoing Discussion] Mar 6, 2018

vincethecoder added 3 commits March 6, 2018 15:28

Changes to repeat instructions when top banner is tapped.

7de55a1

Reverted storyboard to master file.

bf4abf3

Merge branch 'master' into navigation/repeat_instruction

c697482

# Conflicts: # MapboxNavigation/RouteMapViewController.swift

vincethecoder changed the title ~~Repeat Current Instructions - [Ongoing Discussion]~~ Repeat Current Instructions - [When Banner is Tapped] Mar 6, 2018

Added feature to drag instructions to preview instructions.

3874911

vincethecoder changed the title ~~Repeat Current Instructions - [When Banner is Tapped]~~ Repeat Current Instructions - [When Banner is Tapped], Show Instructions Preview - [When Banner is Dragged] Mar 7, 2018

Merge branch 'master' into navigation/repeat_instruction

52b985c

A precautionary measure to update the route progress regardless of th…

15e068b

…e state of the voice muted.

bsudekum changed the title ~~Repeat Current Instructions - [When Banner is Tapped], Show Instructions Preview - [When Banner is Dragged]~~ Swipe down to reveal steps list Mar 7, 2018

bsudekum reviewed Mar 8, 2018

View reviewed changes

vincethecoder added 3 commits March 9, 2018 10:40

Removed the tap top banner functionality on instructions banner. Modi…

22188d5

…fied qualifiers of InstructionsBannerViewDelegate to optional objc delegate methods.

Removed logic to repeat instructions.

1c32b35

Line spacing

3800a6b

bsudekum reviewed Mar 9, 2018

View reviewed changes

bsudekum added this to the v0.16.x milestone Mar 12, 2018

vincethecoder added 3 commits March 13, 2018 13:35

Undo changes for checking if routeProgress is nil

3c73048

Merge branch 'master' into navigation/repeat_instruction

60786e2

Resolved cartfile.

fb4c423

bsudekum reviewed Mar 13, 2018

View reviewed changes

Added a tap gesture recognizer to reveal instructions preview

9a1659b

bsudekum approved these changes Mar 14, 2018

View reviewed changes

Updated the change log

d2b03db

bsudekum merged commit 7155faa into master Mar 14, 2018

bsudekum deleted the navigation/repeat_instruction branch March 14, 2018 23:20

brsbl-zz mentioned this pull request Mar 15, 2018

Make step table more discoverable #1138

Closed

3 tasks

1ec5 mentioned this pull request Mar 12, 2019

Add a gesture for repeating an instruction #979

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Swipe down to reveal steps list #1150

Swipe down to reveal steps list #1150

vincethecoder commented Feb 22, 2018 •

edited by bsudekum

Loading

bsudekum commented Feb 23, 2018

vincethecoder commented Feb 23, 2018

vincethecoder commented Feb 26, 2018

1ec5 commented Feb 26, 2018

bsudekum commented Mar 1, 2018 •

edited

Loading

vincethecoder commented Mar 7, 2018 •

edited

Loading

bsudekum commented Mar 7, 2018

vincethecoder commented Mar 7, 2018

bsudekum commented Mar 7, 2018

vincethecoder commented Mar 7, 2018 •

edited

Loading

vincethecoder commented Mar 7, 2018

bsudekum commented Mar 7, 2018

bsudekum Mar 8, 2018

vincethecoder Mar 9, 2018

bsudekum Mar 9, 2018

vincethecoder Mar 9, 2018

bsudekum Mar 9, 2018

vincethecoder Mar 13, 2018

bsudekum commented Mar 13, 2018

bsudekum Mar 13, 2018

vincethecoder Mar 13, 2018

bsudekum commented Mar 14, 2018 •

edited

Loading

bsudekum left a comment

bsudekum commented Mar 14, 2018

		@@ -56,6 +56,8 @@ extension BaseInstructionsBannerView {
		self.separatorView = separatorView

		addTarget(self, action: #selector(BaseInstructionsBannerView.tappedInstructionsBanner(_:)), for: .touchUpInside)

Swipe down to reveal steps list #1150

Swipe down to reveal steps list #1150

Conversation

vincethecoder commented Feb 22, 2018 • edited by bsudekum Loading

bsudekum commented Feb 23, 2018

vincethecoder commented Feb 23, 2018

vincethecoder commented Feb 26, 2018

1ec5 commented Feb 26, 2018

bsudekum commented Mar 1, 2018 • edited Loading

vincethecoder commented Mar 7, 2018 • edited Loading

bsudekum commented Mar 7, 2018

vincethecoder commented Mar 7, 2018

bsudekum commented Mar 7, 2018

vincethecoder commented Mar 7, 2018 • edited Loading

vincethecoder commented Mar 7, 2018

bsudekum commented Mar 7, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsudekum commented Mar 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsudekum commented Mar 14, 2018 • edited Loading

bsudekum left a comment

Choose a reason for hiding this comment

bsudekum commented Mar 14, 2018

vincethecoder commented Feb 22, 2018 •

edited by bsudekum

Loading

bsudekum commented Mar 1, 2018 •

edited

Loading

vincethecoder commented Mar 7, 2018 •

edited

Loading

vincethecoder commented Mar 7, 2018 •

edited

Loading

bsudekum commented Mar 14, 2018 •

edited

Loading