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

Page change animated twice #18

Open
hnstbndr opened this issue Jun 6, 2020 · 12 comments
Open

Page change animated twice #18

hnstbndr opened this issue Jun 6, 2020 · 12 comments

Comments

@hnstbndr
Copy link

hnstbndr commented Jun 6, 2020

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
Copy link
Author

hnstbndr commented Jun 7, 2020

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
Copy link

shbedev commented Jun 19, 2020

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
Copy link

swofml4 commented Jul 10, 2020

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
Copy link

mlm249 commented Aug 3, 2020

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

@danielsaidi
Copy link

danielsaidi commented Sep 15, 2020

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
Copy link

sjmueller commented Sep 19, 2020

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:

  • add caching to Pages component
    @State private var controllers: [UIHostingController<AnyView>]
    
  • Instantiate the cache on init here
      self._controllers = State(initialValue: self.pages.map { UIHostingController(rootView: $0.eraseToAnyView()) })
    
  • Use the cache in this block
      controllers: pages.enumerated().map { i, page in
          let h = controllers[i]
          h.rootView = page.eraseToAnyView()
          return h
      }
    

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
Copy link
Owner

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
Copy link

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

@danielsaidi
Copy link

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
Copy link

elai950 commented Aug 10, 2021

Does anyone found a solution for this bug yet?

@Cart00nHero
Copy link

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
Copy link

#20 This pull request should fix it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants