nachonavarro / Pages

📖 A lightweight, paging view solution for SwiftUI
MIT License
586 stars 55 forks source link

Page change animated twice #18

Open hnstbndr opened 4 years ago

hnstbndr commented 4 years ago

Hi,

When I change the page programmatically, it is animated once. If I swipe to the next page, I get it animated twice, i.e., once with my swipe and again (to the same page) upon release. This ONLY happens if i change the color of the buttons' texts according to the index. If i leave the color as is, it works fine.

Used your demo "OnboardingView" for this.

`struct OnboardingView: View {

@State var index: Int = 0

var body: some View {
    VStack {
        HStack {
            Button(action: {
                self.index = 0
            }) {
                Text("page 1")
                    .foregroundColor(self.index == 0 ? Color.red : Color.black)
            }
            Button(action: {
                self.index = 1
            }) {
                Text("page 2")
                .foregroundColor(self.index == 1 ? Color.red : Color.black)
            }
            Button(action: {
                self.index = 2
            }) {
                Text("page 3")
                .foregroundColor(self.index == 2 ? Color.red : Color.black)
            }
            Button(action: {
                self.index = 3
            }) {
                Text("page 4")
                .foregroundColor(self.index == 3 ? Color.red : Color.black)
            }
        }
        Pages(currentPage: $index, wrap: false) {
            WelcomePage(background: "avenue",
                        title: "Ready. Set. Apple.",
                        subtitle: "Insert witty remark about your app that will catch potential users.")
            WelcomePage(background: "elephant",
                        title: "Wow. An elephant.",
                        subtitle: "Did you know these magnificent mammals spend between 12 to 18 hours eating grass, plants and fruit every single day?")
            WelcomePage(background: "nature",
                        title: "Nature.",
                        subtitle: "I'm running out of subtitles.")
            WelcomePage(background: "landscape",
                        title: "Ah yes, Scotland.",
                        subtitle: "They may take our lives, but they'll never take our freedom!")
        }.padding()

    }
}

}`

Same issue with wrap: true.

Printout (e.g., Current page is 1, going to 0) is done once, when the page is changed twice.

Afraid that I have no idea how to fix this...

hnstbndr commented 4 years ago

Setting animated to false in line 72 in PageController.swift fixes it. But then the animation is gone when clicking and the view just appears...

shbedev commented 4 years ago

Setting animated to false in line 72 in PageController.swift fixes it. But then the animation is gone when clicking and the view just appears...

Thanks for sharing! I am experiencing the same thing. It only fixes it for scroll gestures, but when changing the index of the page programmatically, it does not animate.

swofml4 commented 4 years ago

PageViewController.swift line 115 causes this on pageCurl. commenting it out fixes it, but breaks update on the control dots. If you aren't using those and want a page curl, just cloning a copy and commenting that out will do it. If I find a better fix, I will post that.

mlm249 commented 4 years ago

Seeing this as well but @swofml4 suggestion works for me since not using the controls

danielsaidi commented 4 years ago

I see this when I bind currentPage to a published ObservableObject property, but not when I use @State.

If I setup a @State prop from a published ObservableObject prop (I persist the page to be able to restore it later), it works, but if I last viewed page 3 and the view is restored on page 3, swiping back takes me directly to page 0.

sjmueller commented 4 years ago

The reason for the bug is because this block instantiates a UIHostingController every time the views are refreshed and state changes. Passing brand new UIHostingControllers to UIPageViewController on every update causes unpredictable behavior. The solution is to ensure that UIHostingControllers are reused per view, which is challenging because there's no good way to identify a view without requiring more from the consumer (i.e. forcing child pages to implement Identifiable).

The least invasive way to fix the problem:

This solution has a limitation that you cannot add/remove/reorder Pages dynamically, because the number of cached HostingControllers would get out of sync and potentially cause out of range exceptions.

I'll try to issue a PR that adds more dynamic controller caching (and in the right place), but for now this should unblock the majority of scenarios.

nachonavarro commented 4 years ago

Hey @sjmueller thanks for the catch, I was going crazy with this bug. I still think that adding/removing/reordering are important features to have, so I'm going to keep it as it is for now. If you find out how to make it work please do issue a PR!

danielsaidi commented 3 years ago

The issue now occurs in iOS 14, even when I bind to a state with initial value 0 😓

danielsaidi commented 3 years ago

I just found out that TabView can be made to behave like a page control in iOS 14. It's well described here: https://stackoverflow.com/questions/58388071/how-can-i-implement-pageview-in-swiftui/63159912

You basically just have to set .tabViewStyle(PageTabViewStyle(indexDisplayMode: indexDisplayMode)) on the TabView, which is strange since the result isn't semantically a tab view.

However, the end result is very nice and stable, so you may want to consider using it as an internal implementation detail for iOS 14. It lets you get rid of all UIKit wrapping, which also makes it run on other platforms as well.

This is the code I use in a personal library (https://github.com/danielsaidi/SwiftUIKit). Just let me know if you would like me to provide it to you as a PR:

@available(iOS 14.0, *)
public struct MultiPageView: View {

    public init<PageType: View>(
        pages: [PageType],
        indexDisplayMode: PageTabViewStyle.IndexDisplayMode = .automatic,
        currentPageIndex: Binding<Int>) {
        self.pages = pages.map { $0.any() }
        self.indexDisplayMode = indexDisplayMode
        self.currentPageIndex = currentPageIndex
    }

    public init<Model, ViewType: View>(
        items: [Model],
        indexDisplayMode: PageTabViewStyle.IndexDisplayMode = .automatic,
        currentPageIndex: Binding<Int>,
        pageBuilder: (Model) -> ViewType) {
        self.pages = items.map { pageBuilder($0).any() }
        self.indexDisplayMode = indexDisplayMode
        self.currentPageIndex = currentPageIndex
    }

    private let pages: [AnyView]
    private let indexDisplayMode: PageTabViewStyle.IndexDisplayMode
    private var currentPageIndex: Binding<Int>

    public var body: some View {
        TabView(selection: currentPageIndex) {
            ForEach(Array(pages.enumerated()), id: \.offset) {
                $0.element.tag($0.offset)
            }
        }
        .tabViewStyle(PageTabViewStyle(indexDisplayMode: indexDisplayMode))
    }
}
elai950 commented 3 years ago

Does anyone found a solution for this bug yet?

Cart00nHero commented 3 years ago

f won't change current page in PagesCoordinator didFinishAnimating everything works fine except flip page by changing currentPage in my project view.

func pageViewController(
    _ pageViewController: UIPageViewController,
    viewControllerAfter viewController: UIViewController
) -> UIViewController? {
    guard let index = parent.controllers.firstIndex(of: viewController) else {
        return nil
    }
    return index == parent.controllers.count - 1 ? (self.parent.wrap ? parent.controllers.first : nil) : parent.controllers[index + 1]
}

func pageViewController(
    _ pageViewController: UIPageViewController,
    didFinishAnimating finished: Bool,
    previousViewControllers: [UIViewController],
    transitionCompleted completed: Bool
) {
    if completed,
    let visibleViewController = pageViewController.viewControllers?.first,
    let index = parent.controllers.firstIndex(of: visibleViewController) {
        print("current page:\(index)")
        /* parent.currentPage = index */
    }
}
MakakWasTaken commented 2 years ago

20 This pull request should fix it