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] Modal transitions stopped working after first hero transition #644

Closed
StefanLdhl opened this issue Nov 14, 2019 · 24 comments
Closed

Comments

@StefanLdhl
Copy link

What did you do?

I use the following to display a VC with a modal transition:
self.present(vc, animated: true) {}

What did you expect to happen?

I expect the normal behavior of IOS 13:
IMG_6509

However, this only works before using a Hero transition at another place in the app the first time!

What happened instead?

As soon as I have used Hero Transition the first time elsewhere in the app, the same modal transition behaves like this:

IMG_6510

From this point, all modal transitions in the app behave like this.

Any ideas?
Thanks in advance!

  • Hero Version:
    1.5.0

  • iOS Version(s):
    13.2

  • Swift Version:
    5.0

  • Devices/Simulators:
    Both

@ardavydov
Copy link

Agree

@JoeMatt JoeMatt modified the milestones: 1.6.0, 1.7.0 Nov 17, 2019
@JoeMatt JoeMatt changed the title Modal transitions stopped working after first hero transition [Bug] Modal transitions stopped working after first hero transition Nov 17, 2019
@Valpertui
Copy link

I'm experiencing the same issue, is there a known workaround for it?
I would prefer not to have to remove Hero from my project but it is currently blocking me from updating to SDK 13 properly :/

@kaich
Copy link

kaich commented Dec 3, 2019

I have the same issue. And the statusbar will disappear when app enter background then back.

@pauloec
Copy link

pauloec commented Feb 27, 2020

This is preventing all projects that rely on modals to use Hero, is there a solution?

@jordanpwood
Copy link

I also ran into this issue. Is there a plan for fixing this? This is a major roadblock for my project.

@AdrianZghibarta
Copy link

Same here, did someone found a solution for this?

@tanujkumar1640
Copy link

Any solution?

@keyhan76
Copy link

Still have this issue on iOS 14!

@nobre84
Copy link

nobre84 commented Nov 18, 2020

Any hint as to why this happen so I could analyze the source code? I'm perfectly OK in not using hero on the flows I need native pageSheet/formSheet presentations, but if you use Hero anywhere else it will break all future native modals.

@kagemiku
Copy link

I found differences of view hierarchy before/after using Hero modal. the position of UIDimmingView is different.

View hierarchy of normal modal (not Hero) before using hero modal

  • UIWindow -> UITransitionView -> UIDropShadowView -> BaseVC -> UIDimmingView -> UITransitionView -> ... -> PresentedVC(Modal)

View hierarchy of normal modal (not Hero) after using hero modal

  • UIWindow -> UITransitionView -> UIDropShadowView -> UIDimmingView -> BaseVC -> UITransitionView -> ... -> PresentedVC(Modal)

Here is screenshot list. the highlighted view is UIDimmingView

Before After
Screen Shot 2020-11-20 at 10 40 46 Screen Shot 2020-11-20 at 10 41 11

@nobre84
Copy link

nobre84 commented Nov 20, 2020

Interesting. I discovered today that a nasty work around for this is presenting a dummy VC and dismissing it in .fullScreen mode. This is able to clean up the changes Hero made that breaks the native presentation. Unfortunately it can flicker a bit so it's not feasible as a solution. We must understand how to restore the hierarchy to the original setup.

@kagemiku
Copy link

yes, .fullScreen doesn't break anything. and also .currentContext doesn't break anything, but .overCurrentContext causes this issue.

  • .fullScreen and .currentContext : OK
  • .overFullScreen and .overCurrentContext : NO

This situation happens at least hero is used when dismissing. I mean, showing modal with normal way, and dismissing modal with Hero by setting hero.isEnabled = true before dismissing.

@nobre84
Copy link

nobre84 commented Nov 20, 2020

What I mean is, after having broken .pageSheet presentations when using hero transitions, one can fix the hierarchy by presenting and dismissing a dummy VC in fullscreen before trying to present your real VC in .pageSheet style.
I think it might be caused by this work around below, as it messes directly with the UIWindow hierarchy

if isPresenting != finished, !inContainerController, transitionContext != nil {
        // only happens when present a .overFullScreen VC
        // bug: http://openradar.appspot.com/radar?id=5320103646199808
        UIApplication.shared.keyWindow?.addSubview(isPresenting ? fromView : toView)
}

@kagemiku
Copy link

kagemiku commented Nov 20, 2020

I think I found a solution for this issue.

HeroTransition.fromView and .toView is computed property which comes from HeroTransition.fromViewController?.view and .toViewController?.view : https://github.com/HeroTransitions/Hero/blob/develop/Sources/Transition/HeroTransition.swift#L176...L177

I changed those computed properties to stored properties by getting those values from context.view(forKey: .from) and context.view(forKey: .to) , like this:

// HeroTransition+UIViewControllerTransitioningDelegate.swift
public func animateTransition(using context: UIViewControllerContextTransitioning) {
    transitionContext = context
    fromViewController = fromViewController ?? context.viewController(forKey: .from)
    toViewController = toViewController ?? context.viewController(forKey: .to)
    fromView = fromView ?? context.view(forKey: .from)
    toView = toView ?? context.view(forKey: .to)
    transitionContainer = context.containerView
    start()
  }

and removed workaround mentioned in @nobre84 's post: https://github.com/HeroTransitions/Hero/blob/develop/Sources/Transition/HeroTransition+Complete.swift#L89...L93

ref: https://stackoverflow.com/questions/24338700/from-view-controller-disappears-using-uiviewcontrollercontexttransitioning/25901154#25901154

GitHub
Elegant transition library for iOS & tvOS. Contribute to HeroTransitions/Hero development by creating an account on GitHub.
GitHub
Elegant transition library for iOS & tvOS. Contribute to HeroTransitions/Hero development by creating an account on GitHub.
Stack Overflow
I got one problem and i have described it below.

I am using UIViewControllerContextTransitioning for custom transitions.

I have 2 view controllers, first view controller and second view controlle...

@nobre84
Copy link

nobre84 commented Nov 20, 2020 via email

@nobre84
Copy link

nobre84 commented Nov 20, 2020

Making myself clearer, when I start using fromView/toView from the transitionContext, there's a lot of code that actually trusts the fromView to be set and then Hero does nothing when doing a .overFullscreen transition. I'm playing with the code to try and make it more flexible to not having a fromView, but hasn't succeeded yet

@nobre84
Copy link

nobre84 commented Nov 21, 2020

Okay I think I've got a solid work around:

import UIKit

extension UIViewController {

    /// Hero does not honor UIViewControllerTransitioning's contract correctly on transitions that show through the previous view controller, leaving the hierarchy wrong on dismissal. In order to present with .automatic, .formSheet or .pageSheet styles, we do a work around that allows UIKit to fix the window hierarchy before presenting.
    /// - Parameters:
    ///   - viewController: the view controller to present
    ///   - animated: wether to animate the presentation
    ///   - completionHandler: handler called when the new view controller has appeared
    func presentWithHeroFix(_ viewController: UIViewController,
                            animated: Bool,
                            completion: (() -> Void)? = nil) {

        func presentAction() {
            present(viewController, animated: animated, completion: completion)
        }

        guard #available(iOS 13, *),
              viewController.modalPresentationStyle != .fullScreen,
              let windowSnapshot = view.window?.snapshotView(afterScreenUpdates: false)
        else {
            print("Could not get window snapshot to presentWithHeroFix")
            return presentAction()
        }

        // Now we present and dismiss a view controller to allow UIKit to rebuild the window's presenting hierarchy. The view is just a snapshot of the current window contents to avoid any flickering.
        let snapshotViewController = SnapshottingViewController(snapshot: windowSnapshot)
        present(snapshotViewController, animated: false) {
            snapshotViewController.dismiss(animated: false) {
                presentAction()
            }
        }

    }

}

private class SnapshottingViewController: UIViewController {

    private let snapshot: UIView

    init(snapshot: UIView) {
        self.snapshot = snapshot

        super.init(nibName: nil, bundle: nil)
    }

    @available(*, unavailable)
    required init?(coder: NSCoder) {
        fatalError("init(coder:) has not been implemented")
    }

    override func loadView() {
        view = snapshot
    }

    override open var modalPresentationStyle: UIModalPresentationStyle {
        get { return .fullScreen }
        set { _ = newValue }
    }

}

@nobre84
Copy link

nobre84 commented Nov 21, 2020

The final solution would be to refactor Hero to work without assuming fromView is free to use on non full screen transitions, which is not true since iOS 8.

@kagemiku
Copy link

yes, unfortunately, fromView from context can be nil...

@kagemiku
Copy link

kagemiku commented Nov 26, 2020

UIApplication.shared.keyWindow?.addSubview(isPresenting ? fromView : toView)

I found why this workaround causes this bug. This is because adding fromView/toView to wrong position.

Ideal:

  • UIWindow
    • UITransitionView
      • UIDropShadowView
        • Here

but, actual:

  • UIWindow
    • UITransitionView
      • UIDropShadowView
    • Here

so, we have to add fromView/toView under correct UIDropShadowView, like this:

let transitionView = UIApplication.shared.keyWindow?.subviews.first
let dropShadowView = transitionView?.subviews.first
dropShadowView?.addSubview(isPresenting ? fromView : toView)

(Please be careful, the above code doesn't work when modal on modal.)

  1. Why fullScreen presentation style fixes this broken hierarchy?
    Because fullScreen presentation remove baseVC from DropShadowView, and re-add baseVC to correct DropShadowView when dismissing

  2. Why that workaround is needed when overFullScreen presentation style with Hero
    Because baseVC is removed from original DropShadowView when addSubview is called here.
    Maybe iOS doesn't recover baseVC when overFullScreen is specified because it's assumed baseVC is still added under DropShadowView.

@JoeMatt
Copy link
Collaborator

JoeMatt commented Feb 15, 2021

Does this still happen in 1.6.0?
If so, can you make a PR please?

@ardavydov
Copy link

In my case this bug still exists in 1.6.1

@JoeMatt
Copy link
Collaborator

JoeMatt commented Mar 14, 2021

I know it's an odd request from a manager, but I'm not super up to speed with a lot of heros workings.

If you have a patch could you please submit a pull request for review and I'll get to get this resolved as soon as possible. Thanks.

@pkulikow
Copy link

pkulikow commented Oct 6, 2021

I have found a different workaround but it may be specific for my presentation architecture. I need to use Hero transitions in a specific controller to present some items as expanded in a separate view controller. The items are displayed in a collection view and taping one of them opens the other view controller using Hero animation which simulates expanding of that item. Sometimes after I present the list controller I need to present some modal view on top of it with some success message or other information about some business flow. This caused the issue described in this thread where after second time I perform this flow the transitions stopped working and default push was used.

I have found that if I disable hero in viewWillAppear and enable it again in the viewDidAppear methods of the first view controller (the one with the list) it fixes the issue and now every transition works correctly no matter how many times I show some modals on top using fullScreen or overFullscreen mode.

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

No branches or pull requests