fermoya / SwiftUIPager

Native Pager in SwiftUI
MIT License
1.29k stars 172 forks source link

[FEAT] Modifier to customize page sensitivity #129

Closed AXR-Tech closed 4 years ago

AXR-Tech commented 4 years ago

Describe the bug Scroll is inconsistent. At times requires larger swipe to trigger scroll to next page and other times even with a long swipe the scroll is not triggered.

To Reproduce Normal implementation, did try with infinite loop or no loop.

Expected behavior Increased sensitivity to swipe (lower swipe distance) or more consistency.

Screenshots / Videos Don't quite need? This may have been already addressed and just requires the sensitivity modifier?

Environment: iOS 14 and PadOS 14 and iOS 13.0 iPhone XS Max and iPad mini, iPad Pro 12.9" 3rd Gen SwiftUIPager 1.9.0

Additional context Didn't use to occur before. Did read in prior issues that a feature affected it and that v1.9.0 had it fixed, not sure if I need to add a modifier to the Pager or not.

fermoya commented 4 years ago

Hi @Alexander-techIOS , there was a bug introduced in 1.9.0, can you please try the latest version?

AXR-Tech commented 4 years ago

@fermoya I have updated to v1.11.0 and I understand now what was being said in the previous issue. Triggering the animation (scroll) is based upon scroll velocity? If one swipes quickly it scrolls, but if the velocity is reduced then it starts the scroll but then the threshold for distance moved isn't triggered, which results in the object moving back into position. Is there a way to reduce the distance the object is required to be dragged before triggering the animation?

fermoya commented 4 years ago

@Alexander-techIOS why you ask? Are your items too small? Currently, you need to drag from sizer to size to move from one page to the other. This means it's normalized. If your items are too small and want to page more than one item, you might want to use multiplePagination.

I can't have single pagination not normalised because otherwise you'd swipe more than one page from side to side but the increment would always be one. That's what multiplePagination is for

AXR-Tech commented 4 years ago

@fermoya Ic, no my items aren't small it's card based and the device is in landscape and takes up about 55% of the width of the screen below is a snippet.

     `Pager(page: self.$vm.dashboardPage, data: self.vm.dashboardOptions, id: \.self) { item in

           Image(item.imageName)
                    .resizable()
                    .aspectRatio(contentMode: .fill)
                    .frame(width: UIScreen.main.bounds.width * 0.55, height: UIScreen.main.bounds.height * 0.7)
                    .onTapGesture {
                        switch item {
                        case .feature:
                            self.appSettings.showFeature = true
                        default:
                            self.vm.alertItem = AlertItem(title: "Info", message: "Feature still coming", button: .default(Text("OK")))
                        }
                }
            }
            .itemSpacing(10)
            .itemAspectRatio(0.92)
            .interactive(0.6)
            .loopPages()
            .swipeInteractionArea(.allAvailable)
            .background(Image(Strings.images.Dashboard_Bg.rawValue).resizable())`

If I start to drag slowly from the centre of the image, the image always snaps back. Let me try multiplePagination.

AXR-Tech commented 4 years ago

@fermoya MultiplePagination does not work well with loopPages. It tends to scroll to the end of the List and then the last item isn't initialized for the loop yet.

AXR-Tech commented 4 years ago

@fermoya there is a minimum distance in Pager Content. If that was reduced to like 8 what would happen? I can't seem to modify it myself, probably because I'm using SPM.. But I see you have tests to make sure the minimum distance is 15. Which leads me to assume the number is significant for something else as well.

fermoya commented 4 years ago

That’s the minimum distance to start recognizing the dragging. If reduced, then it makes ScrollView not work when Pager is embedded in it

fermoya commented 4 years ago

@Alexander-techIOS currently I have it so that if you need to scroll 50% of the Pager width. This makes sense to me, it's how I've always implemented carrousels:

  1. breather than 50%, then increment (or decrease) the index.
  2. if the swipe was fast enough, then don't increment.
  3. For items smaller than the pager's width, normalize the scroll so that one touch can't move further than +- 1 page.

I don't think I should change the first point. I can maybe adapt the second point to make it scroll even more easily, I'd have to give it a look altho that's a matter of likes, to be honest. And the third point, I think I explained in my previous comments. The reason is if it's not normalised you could drag further than +- 1 page, but then the maximum increment is 1.

What are your thoughts?

dinotrnka commented 4 years ago

I also feel that the current implementation requires too much effort in order to swipe pages, especially if item size is smaller than screen width. For example, I have set the pager in my app so you can see the previous and next choice on the left and right side, while the selected item is in the center, and it's kind of hard to swipe it to the next option.

Could you at least make the percentage in point 1 (which is currently 50%) a parameter? It would be great.

And thanks for an awesome library!

fermoya commented 4 years ago

What do you mean by it’s “currently 50%”? Can you look at PagerContent and find that setting?

The scroll is normalized so that scrolling from side to side of the pager will increment (or decrement) pageIndex by one. The drag velocity helps with that too. Otherwise, you could drag past the next page and it would always bounce back, I find that weird.

I haven’t noticed such issue in the example provided in the repository, but I could be wrong. Could you please take a look at that project please?

There’s also multiplePagination that should be of help here

fermoya commented 4 years ago

@Alexander-techIOS I just remembered there's delaysTouches that you can set to false. This way the minimumDistance will be 0. This will affect the way Pager behaves inside (or wrapped into) a ScrollView.

The dragging is normalized as it's a single pagination Pager. I don't see why you'd want it not to be normalized, the effect looks weird as you can drag the page past the next/previous page and then bounces back.

This normalization happens in PagerContent.swift line 234:

let normalizedRatio = self.allowsMultiplePagination ? 1 : (self.pageDistance / side)

Feel free to play with it and let me know if you might find better approach. I'm closing this but feel free to reopen.

dinotrnka commented 4 years ago

What do you mean by it’s “currently 50%”? Can you look at PagerContent and find that setting?

The scroll is normalized so that scrolling from side to side of the pager will increment (or decrement) pageIndex by one. The drag velocity helps with that too. Otherwise, you could drag past the next page and it would always bounce back, I find that weird.

I haven’t noticed such issue in the example provided in the repository, but I could be wrong. Could you please take a look at that project please?

There’s also multiplePagination that should be of help here

I looked at the example project, and the behavior that I'm referring to is on the Infinite Pager screen. In the top example called "Appending on the fly", the yellow page items take up most of the screen width, and they're easy to swipe back and forth to the next/previous page.

But in the bottom example called "Looping Pager", the items are narrower and don't take up the entire screen width. This is my case. In this case, I'd like swiping to take less effort because if I don't swipe all the way, the page doesn't change, so the pager bounces back to where it was.

I don't know if I'm communicating this clearly, but the intent is to make it possible to change pages with less dragging. Is that possible? I guess one way is to increase the normalizedRatio in PagerContent to a higher number (closer to 1), but I was wondering if there is a cleaner way since as you said above, this results in an unwanted bouncing effect.

fermoya commented 4 years ago

@dinotrnka I think you got your idea. You mean something like dragSensitivity to determine whether to scroll or not to the next/previous page based on the percentage of distance dragged once the drag gesture ends, is that it? I think I got now why you mentioned that 50%.

If so, I can look into it, reopen this issue and label it as enhancement

dinotrnka commented 4 years ago

@dinotrnka I think you got your idea. You mean something like dragSensitivity to determine whether to scroll or not to the next/previous page based on the percentage of distance dragged once the drag gesture ends, is that it? I think I got now why you mentioned that 50%.

If so, I can look into it, reopen this issue and label it as enhancement

Yes, that's exactly what I meant 🙂 Sorry for not being clear enough. It would be a great enhancement for the current functionality! Thank you! 🙏

fermoya commented 4 years ago

Oh no, it's totally my fault, I've had a lot of trouble understanding this one. I'll let you know here when this is done. Thanks!

fermoya commented 4 years ago

@dinotrnka @Alexander-techIOS , new modifier sensitivity(_ value:) available in 1.12.0-beta.1, will you give it a look please? Use .high for your case or custom(CGFloat) with the desired relative increment needed to change the page index

dinotrnka commented 4 years ago

@dinotrnka @Alexander-techIOS , new modifier sensitivity(_ value:) available in 1.12.0-beta.1, will you give it a look please? Use .high for your case or custom(CGFloat) with the desired relative increment needed to change the page index

I just tried it with the .high parameter and it's perfect! It's seamless to scroll the pager now, and this is such a great UX. 🙂

Thanks again so much for this improvement 🙂 This industry definitely needs more people like you! 🙏

fermoya commented 4 years ago

You're too kind, thanks a lot! Expect a production version soon