PaoloCuscela / Cards

Awesome iOS 11 appstore cards in swift 5.
MIT License
4.21k stars 275 forks source link

Display and animation issues on iOS 12 #80

Closed Medwe closed 5 years ago

Medwe commented 6 years ago

I'm pretty sure you have noticed by now, but when using cards (at least CardArticle) on iOS 12, several things go horribly wrong, beginning with text that's displayed darker than it should be to severe bounds and animation issues.

I really don't know where to start honestly, so I took a short screen capture to show what exactly I mean (https://youtu.be/SHeDv5e89C8)

I hope you can resolve the issue soon, looking forward to a response :)

Varmoes commented 6 years ago

Hey Medwe!

I have problems playing your video on YouTube so I couldn't compare with my own problems. I took the liberty to create my own video to add support to probably the same problems!

https://youtu.be/1sPYonohBxU

Medwe commented 6 years ago

Oh I'm sorry the video didn't work but yeah, your video shows exactly the same issues. I would really appreciate a fix for this :)

Michele404 commented 6 years ago

Same bug

federico2390 commented 6 years ago

same

jgregmccormick commented 6 years ago

Anything here yet?

MayankAgarwal04 commented 6 years ago

@PaoloCuscela @amerhukic @Medwe @Varmoes @Michele404 @federico2390 @jgregmccormick

Hello Guys ,

I am facing the same problem on iOS 12.0 on production. Kindly let me know if anybody has resolved this issue.

EDMODJ commented 6 years ago

The problem is that originalFrame value changes (Don't know why yet) Just because I had several headaches and i needed to find a solution. Here is a rudimentary and temporal fix.

Using a singleton you can persist the value of originalFrame

For example: using AppDelegate (Not recommended but same result)

Declare on AppDelegate.swift: var originalFrameValue: CGRect?

Modify Animator.swift (from Cards classes): 1.- Create an instance of our singleton:

func animateTransition(using transitionContext: UIViewControllerContextTransitioning) {
        ...
        container.addSubview(from.view)
        let appDelegate     = UIApplication.shared.delegate as! AppDelegate // Instance of Singleton
        ...

2.- Set value on presenting (Just After guard presenting else { ... }):

        ...
        // Detail View Controller Present Animations
        card.isPresenting  = true
        appDelegate.originalFrameValue =  card.originalFrame  // Set value on Singleton
        ...

2.- Use that value for animation (Inside guard presenting else { ... } ):

      ...
      guard presenting else {
           ...
           // Layout with bounce effect
            let originalFrame   = appDelegate.originalFrameValue // Return and set value from Singleton
            UIView.animate(withDuration: velocity/2, delay: 0, options: .curveEaseOut, animations: {

                detailVC.layout(originalFrame!, isPresenting: false, transform: bounce)  // Use originalFrame
                self.card.delegate?.cardIsHidingDetail?(card: self.card)

            }) { _ in UIView.animate(withDuration: self.velocity/2, delay: 0, options: .curveEaseOut, animations: {

                detailVC.layout(originalFrame!, isPresenting: false)  // Use originalFrame
                self.card.delegate?.cardIsHidingDetail?(card: self.card)
                })
            }
            return
       }
        ...

Hope it helps !!! Works on ios12

Michele404 commented 6 years ago

Its not fixing the open issue, only the close issue @EDMODJ

EDMODJ commented 6 years ago

Hi @ Michele404, I have no problems opening. Maybe you are using full screen mode, it does not work as it is originally but it is not as bad as the closing problem.

without full screen mode works fine

Michele404 commented 6 years ago

and how can I fix the full screen?

EDMODJ commented 6 years ago

Hi @Michele404 , it seems the problem is on the first animation when the card is opening.

The animation just ignores (don't know why) the width and height transformation

I fixed it commenting the lines of first animation and leave the second one does the job

On Animator.swift

        ...
        // Layout with bounce effect
        UIView.animate(withDuration: velocity/2, delay: 0, options: .curveEaseOut, animations: {
            //Comment this two lines
            //detailVC.layout(detailVC.view.frame, isPresenting: true, transform: bounce)
            //self.card.delegate?.cardIsShowingDetail?(card: self.card)

        }) { _ in UIView.animate(withDuration: self.velocity/2, delay: 0, options: .curveEaseOut, animations: 
        {
        ...

Hope it helps !!

@amerhukic @Medwe @Varmoes @MayankAgarwal04 @federico2390 @jgregmccormick

Michele404 commented 6 years ago

Thanks!!! Do you know why the image of card is rounded with iOS12, too? @edmodj

federico2390 commented 6 years ago

@EDMODJ but when dismiss the problem is the same. how we can I solve the problem ? do you know a method ?

EDMODJ commented 6 years ago

hi @federico2390, for Dismiss here a basic solution:

The problem is that originalFrame value changes (Don't know why yet) Just because I had several headaches and i needed to find a solution. Here is a rudimentary and temporal fix.

Using a singleton you can persist the value of originalFrame

For example: using AppDelegate (Not recommended but same result)

Declare on AppDelegate.swift: var originalFrameValue: CGRect?

Modify Animator.swift (from Cards classes): 1.- Create an instance of our singleton:

func animateTransition(using transitionContext: UIViewControllerContextTransitioning) {
        ...
        container.addSubview(from.view)
        let appDelegate     = UIApplication.shared.delegate as! AppDelegate // Instance of Singleton
        ...

2.- Set value on presenting (Just After guard presenting else { ... }):

        ...
        // Detail View Controller Present Animations
        card.isPresenting  = true
        appDelegate.originalFrameValue =  card.originalFrame  // Set value on Singleton
        ...

2.- Use that value for animation (Inside guard presenting else { ... } ):

      ...
      guard presenting else {
           ...
           // Layout with bounce effect
            let originalFrame   = appDelegate.originalFrameValue // Return and set value from Singleton
            UIView.animate(withDuration: velocity/2, delay: 0, options: .curveEaseOut, animations: {

                detailVC.layout(originalFrame!, isPresenting: false, transform: bounce)  // Use originalFrame
                self.card.delegate?.cardIsHidingDetail?(card: self.card)

            }) { _ in UIView.animate(withDuration: self.velocity/2, delay: 0, options: .curveEaseOut, animations: {

                detailVC.layout(originalFrame!, isPresenting: false)  // Use originalFrame
                self.card.delegate?.cardIsHidingDetail?(card: self.card)
                })
            }
            return
       }
        ...

Hope it helps !!! Works on ios12

MayankAgarwal04 commented 6 years ago

@EDMODJ I followed your code step by step but still its not working for me. Here is,

  1. created singleton instance in appdelegate
  2. stored value of originalFrame as well
  3. used in Inside guard presenting else { ... } Help me.
EDMODJ commented 6 years ago

@MayankAgarwal04 could you post your animateTransition function code?

federico2390 commented 6 years ago

@MayankAgarwal04 could you post your animateTransition function code?

I'm curious

Michele404 commented 6 years ago

@EDMODJ do you know why the image when you open the card is rounded on iOS12?

PaoloCuscela commented 6 years ago

~~Try now with the changes of @robin850. ( Install manually not from cocoapods )~~

Medwe commented 6 years ago

@PaoloCuscela I don't see how @robin850's changes have anything to do with the issue.As far as I see, all that was done is renaming method calls to deprecated versions. (bringSubview(toFront: detailVC.view) -> bringSubviewToFront(detailVC.view)

I'm pretty confused as to how any of this makes sense?

PaoloCuscela commented 6 years ago

Yep. You're right.

federico2390 commented 6 years ago

i have ever this problem guys please how can I fix it @PaoloCuscela @EDMODJ

dyjh2310

Michele404 commented 6 years ago

Same @federico2390

akring commented 6 years ago

So, problem is not solved, it's just been closed 😹

Medwe commented 6 years ago

@akring The other two Issues have been closed, not because the issue was solved but because there already is this Issue. Having all three of them open would be redundant and confusing.

akring commented 6 years ago

The key appears to be the card.originalFrame is changed due to the override open func draw(_ rect: CGRect) {), I just logged the value of the originalFrame:

(lldb) po originalFrame
▿ (20.0, 136.0, 335.0, 150.0)
  ▿ origin : (20.0, 136.0)
    - x : 20.0
    - y : 136.0
  ▿ size : (335.0, 150.0)
    - width : 335.0
    - height : 150.0

(lldb) po self.card.originalFrame
▿ (0.0, 0.0, 335.0, 150.0)
  ▿ origin : (0.0, 0.0)
    - x : 0.0
    - y : 0.0
  ▿ size : (335.0, 150.0)
    - width : 335.0
    - height : 150.0

The self.card.originalFrame's origin is reseted to (0,0), which might cause the card view bounce up to the top-left of the view, and spread later.

federico2390 commented 6 years ago

how we can resolve that?

akring commented 6 years ago

Here's a very "hack" way

  1. Modify the Cards.swift in line 166:
override open func draw(_ rect: CGRect) {
        super.draw(rect)

        if originalFrame == CGRect.zero {
            originalFrame = rect
        }

...
}

This would fix the bug when you try to dismiss the card.

  1. Modify Animator.swift line 101 :
    
    UIView.animate(withDuration: velocity/2, delay: 0, options: .curveEaseOut, animations: {

// detailVC.layout(detailVC.view.frame, isPresenting: true, transform: bounce) // self.card.delegate?.cardIsShowingDetail?(card: self.card)

...
}


Comment these lines will fix the bug when you present the card. However, as you can see, the bounce animation will be disabled.

This is just a nasty workaround so I won't make it as a pull request, there should be a better solution.
federico2390 commented 6 years ago

Hey Guyyyysssss I have found a solution fully work:

In Cards.swift, in line 165

Overwrite: originalFrame = rect

With this: if originalFrame == CGRect.zero { originalFrame = rect }

Thank u @akring

In the Animator.swift

Overwrite the line 61 with this: detailVC.layout(originalFrame, isPresenting: false)

And comment this line:

Stay tuned! 🎉

so... this for me work perfectly, maybe the problem for the Card in iOS 12 is the bounce effect.

AIRLegend commented 6 years ago

None of the above solutions worked for me. I've solved it myself by commenting one line in Card.swift override open func draw(_ rect: CGRect) { super.draw(rect) LINE TO COMMENT //originalFrame = rect

kraj011 commented 6 years ago

Just a quick heads up if you are fixing this issue and you are editing a file installed by cocoapod, make sure you clean your build and then re build to make sure the changes apply!

Varmoes commented 6 years ago

Anyone got a definite fix?

StriderHND commented 6 years ago

Would be great if we have a oficial fix with a PR, @PaoloCuscela can you help us with this?

federico2390 commented 6 years ago

Maybe his have other works ☺️

Il giorno 26 nov 2018, alle ore 23:45, Erick Gonzales notifications@github.com<mailto:notifications@github.com> ha scritto:

Would be great if we have a oficial fix with a PR, @PaoloCuscelahttps://github.com/PaoloCuscela can you help us with this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/PaoloCuscela/Cards/issues/80#issuecomment-441829590, or mute the threadhttps://github.com/notifications/unsubscribe-auth/Ao_xuSDzraH48JgjElInzMq9W-mbRQyCks5uzG77gaJpZM4W1Rvh.

PaoloCuscela commented 6 years ago

Hi guys, I'm sorry for the delay, I' ve been quite busy these days...

Apparently (idk if it's an iOS12 glitch or some code flaws) the first animation block of the chain in the Animator.swift was executed as a normal code block ignoring the duration property... So the 2nd animation was executed in the wrong time and everything screwed up.

I've solved in a very hacky way: I' ve set up a first 'fake' animation and executed inside some code that should be executed synchronously...

// This code should actually be executed outside the animation but idk why this first animation is executed and skipped ( even with 'duration' setted up )
        UIView.animate(withDuration: 0, delay: 0, options: .curveEaseIn, animations: {
            // Setting detail VC in dismissed mode (VC frame = card frame)
            detailVC.layout(self.card.originalFrame, isPresenting: false)

        }, completion: { finished in UIView.animate(withDuration: self.velocity/2, delay: 0, options: .curveEaseIn, animations: {
                detailVC.blurView.alpha = 1
                detailVC.snap.alpha = 1
                detailVC.layout(detailVC.view.frame, isPresenting: true, transform: bounceOffset)

            }, completion: { (_) in UIView.animate(withDuration: self.velocity/2, delay: 0, options: .curveEaseIn, animations: {
                    detailVC.layout(detailVC.view.frame, isPresenting: true)

                }, completion: { (_) in
                    detailVC.layout(detailVC.view.frame, isPresenting: true)
                    transitionContext.completeTransition(true)

                })
            })
        })

If you guys know why is that happening could you let me know ? Anyway, I didn't had enough time to test it properly, could you please test it out and let me know about any bugs ?

Thanks! ❤️  

EDIT: I forgot, I' ve pushed the new release to cocoapods too ( v1.3.5 )

federico2390 commented 6 years ago

Thx

Varmoes commented 5 years ago

Thanks a lot, @PaoloCuscela ! Working flawlessly again!