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

[BUG] Pager within NavigationView scrolls to next page during edge swipe gesture (for navigating back) #252

Closed
robinst opened this issue Mar 17, 2022 · 13 comments
Labels
bug Something isn't working
Milestone

Comments

@robinst
Copy link

robinst commented Mar 17, 2022

Describe the bug
When using a Pager within subview of a NavigationView and then navigating back using a swipe gesture from the left screen edge, the gesture causes the pager to go to the next page while the swipe gesture is being performed, see video below.

To Reproduce

  1. Have a NavigationView that contains a NavigationLink with a destination view that contains a Pager
  2. Navigate into the destination view
  3. Swipe from the left screen edge (pop gesture) to navigate back

Expected: Navigates back to the previous view, the pager doesn't change.
Actual: The pager swipes to the next page while the gesture is being performed. It's doesn't always happen but pretty often, maybe depending on how quick the gesture is performed.

Code to reproduce (drop into the Examples in SwiftUIPagerExample):

import SwiftUI

struct NavigationExampleView: View {
    var body: some View {
        NavigationView {
            NavigationLink(destination: PagerView()) {
                Label("Navigate", systemImage: "folder")
            }
        }
    }
}

struct PagerView: View {
    @StateObject var page: Page = .first()
    // For older versions:
    // @State var page: Int = 0

    @State var items = [
        Item(name: "blue", color: Color.blue),
        Item(name: "red", color: Color.red)
    ]

    var body: some View {
        Pager(page: page,
              data: items,
              content: { item in
                  Text(item.name)
                      .frame(maxWidth: .infinity, maxHeight: .infinity)
                      .background(item.color)
              })
        // Doing this fixes the problem, but disables dragging (duh):
        // .allowsDragging(false)
    }
}

struct Item: Equatable, Identifiable {
    var name: String
    var color: Color

    var id: String {
        name
    }
}

struct NavigationExampleView_Previews: PreviewProvider {
    static var previews: some View {
        NavigationExampleView()
    }
}

Screenshots / Videos

SwiftUIPager.in.NavigationView.mp4

Notice how, while performing the edge swipe gesture to navigate back, the pager goes to the next page (from blue to red).

Environment:

  • OSX: Xcode 13.3, iOS 15.4
  • Device: iPhone 13 Pro
  • SwiftUIPager version: 2.3.2 (and earlier versions too)

Additional context
Some observations:

  • Problem seems to be in onDragChanged in PagerContent. When adding a print for newOffset at the end of the method, I'm getting values like 0.0, 4.0, -386.0. The last one seems strange. I'm not sure how exactly the ongoing gesture for NavigationView affects the pager's gesture coordinates, but somewhere there seems to be the problem.
  • When changing the swipeGesture to DragGesture(minimumDistance: minimumDistance, coordinateSpace: .global), it fixes the problem in this particular case, but that would probably have other unintended side effects.
@robinst robinst added the bug Something isn't working label Mar 17, 2022
@denizdogan
Copy link

I'm having the exact same issue, but with swiping from the top edge to view the notification center.

@fermoya
Copy link
Owner

fermoya commented Mar 23, 2022

Hi @robinst thanks for the suggestion. I'll take a look, I'm aware of the issue of that exists when you combine gestures in (or views that implicitly have a gesture) in SwiftUI. In UIKit there are option to request a gesture to fail before being recognized but the same doesn't apply here.

I'll take a look at your suggestion. I can always create a beta version and get feedback from it.

@denizdogan have you tried the suggestion above?

@fermoya
Copy link
Owner

fermoya commented Mar 23, 2022

Out of curiosity, have you tried delaysTouches and pagingPriority?

@denizdogan
Copy link

@fermoya My use of the component is currently very "barebones", but I'm thinking maybe it would be simpler to just adjust the "gesturable area", so that it doesn't react when the touch starts outside of the safe area?

@fermoya
Copy link
Owner

fermoya commented Mar 23, 2022

That should be already possible, the library ships with swipeInteractionArea modifier. It's possible to limit the interaction to just the page and not the whole container.

Problem here is there's no such thing as a safe area here. Pagerexpands and takes up as much space as its container. If the the parent expands till the status bar, then so does Pager. In your case it would be a matter of ensuring this. I'm guessing wrapping Pager into a VStack.

Similarly, using swipeInteractionArea might help in case .page is selected, although I don't think it would make much of a difference if the page reaches the edges of the screen.

I'll take a look at coordinateSpace, it seems like a good start

@robinst
Copy link
Author

robinst commented Mar 24, 2022

Out of curiosity, have you tried delaysTouches and pagingPriority?

Yeah just tried that, it doesn't change anything.

I'll take a look at coordinateSpace, it seems like a good start

Note that that breaks the "More" example where swiping is vertical or right to left (because of rotation3DEffect). But that could be fixed by changing the code in onDragChanged to be aware of the direction instead.

@fermoya fermoya added this to the 2.3.3 milestone Mar 25, 2022
@fermoya
Copy link
Owner

fermoya commented Mar 25, 2022

this should be fixed here in case you want to check it out:
https://github.com/fermoya/SwiftUIPager/releases/tag/2.3.3-beta.3

@robinst
Copy link
Author

robinst commented Mar 26, 2022

Yess, that fixes it, thank you :)! Hopefully it fixes @denizdogan's problem too.

@fermoya fermoya closed this as completed Mar 30, 2022
@robinst
Copy link
Author

robinst commented Mar 31, 2022

Thanks for the quick fix @fermoya, I've bought you a few coffees as a thank you :)

@denizdogan
Copy link

@fermoya So you just to circle back, swipeInteractionArea did nothing for me, as you expected. My issue is that the pager covers the entire screen, and when I swipe down from the very top of the screen to open the notification center, the pager also swipes up. Should I open another issue about this?

@fermoya
Copy link
Owner

fermoya commented Apr 3, 2022

@denizdogan try the mates version 2.3.3 if that helps but what you must ensure then is that Pager stats within the Safe Area. That's up to you, not the library, check out the examples in the repo.

@denizdogan
Copy link

@fermoya Sorry, I should have mentioned I was already using 2.3.3. That's unfortunate to hear, IMO the library would do good to allow us to define offsets from the edges where the pager should ignore gestures.

@fermoya
Copy link
Owner

fermoya commented Apr 4, 2022

@denizdogan please open a new issue and share your code or a small snippet to reproduce, maybe a video too. But please, first check out the examples in the repo. I’m using a Pager inside a NavigationView and it doesn’t use the safe area. What I’m saying is Pager doesn’t use the safe area unless you specify it so my guess is there’s something in your code that makes it use it.

please open a new thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants