Skip to content

Conversation

ChronicLynx
Copy link
Contributor

@ChronicLynx ChronicLynx commented Feb 20, 2025

First time contributor checklist

Contributor checklist

  • My commits are rebased on the latest main branch
  • My commits are in nice logical chunks
  • My contribution is fully baked and is ready to be merged as is
  • I have tested my contribution on these devices:
  • iPhone 16 Pro (Xcode Simulator - 18.2)

Description

Potentially fixes #5976

My theory, given the error in #5976, is that because the original code failed to wait for the dismissal to complete, it would occasionally try to present a new view while an existing view was present. This caused the animation for the navigation buttons in ImageEditorCropViewController and similar image editing views to be left in a state with no bottom navigation buttons. Therefore, the user has to force quit the app in order to exit the view. I think this may stabilize that flow and prevent such situations from occurring.

ImageEditorCropViewController:

I would imagine when encountering the error

2025/02/20 00:54:25:091 ERR❤️ [AttachmentPrepViewController.swift:221 presentFullScreen(viewController:)]: Already has presented view controller. [<SignalUI.ImageEditorCropViewController: 0x127aef700>

that it possibly short circuits or resets the logic responsible for summoning the bottom buttons. I was originally fixated on the logic summoning the buttons until I noticed a seemingly correlated error with the occurrence of the bug in AttachmentPrepViewController.

override func viewDidAppear(_ animated: Bool) {
        super.viewDidAppear(animated)

        transitionUI(toState: .final, animated: true)
    }
private func transitionUI(toState state: UIState, animated: Bool, completion: ((Bool) -> Void)? = nil) {
        let layoutGuide: UILayoutGuide = {
            switch state {
            case .initial: return initialStateContentLayoutGuide
            case .final: return finalStateContentLayoutGuide
            }
        }()

        let hideControls = state == .initial
        let setControlsHiddenBlock = {
            let alpha: CGFloat = hideControls ? 0 : 1
            self.footerView.alpha = alpha
            self.cropView.setState(state == .initial ? .initial : .normal, animated: false)
            self.bottomBar.setControls(hidden: hideControls)
        }

        let animationDuration: TimeInterval = 0.15

        let imageCornerRadius: CGFloat = state == .initial ? ImageEditorView.defaultCornerRadius : 0
        if animated {
            let animation = CABasicAnimation(keyPath: #keyPath(CALayer.cornerRadius))
            animation.fromValue = imageView.layer.cornerRadius
            animation.toValue = imageCornerRadius
            animation.duration = animationDuration
            imageView.layer.add(animation, forKey: "cornerRadius")
        }
        imageView.layer.cornerRadius = imageCornerRadius

        if animated {
            UIView.animate(withDuration: animationDuration,
                           animations: {
                setControlsHiddenBlock()
                self.constrainContent(to: layoutGuide)
                self.updateImageViewTransform()
                // Animate layout changes made within bottomBar.setControls(hidden:).
                self.view.setNeedsDisplay()
                self.view.layoutIfNeeded()
            },
                           completion: completion)
        } else {
            setControlsHiddenBlock()
            constrainContent(to: layoutGuide)
            updateImageViewTransform()
            completion?(true)
        }
    }

@ChronicLynx ChronicLynx force-pushed the #5976-FixDisappearingButtons branch from 85f398e to cded66d Compare February 20, 2025 19:16
Comment on lines 228 to +230
viewController.modalPresentationStyle = .fullScreen
zoomOut(animated: true) { [weak self] in
self?.presentFullScreen(viewController, animated: false)
self?.present(viewController, animated: false, completion: nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Especially since this isn't related to the fix, I'm more inclined to drop the viewController.modalPresentationStyle = .fullScreen here, and continue to call self.presentFullScreen here.

@sashaweiss-signal
Copy link
Contributor

I'm not sure that this will fix the linked issues, but it seems like a reasonable change nonetheless. I can get this merged internally (and I can make the adjustment I left a comment about, while I'm there).

Our flow for external contributions is to recreate the diff internally, and commit it using the external author's information. Would you mind emailing me at [email protected] with a name and email you'd like used in the commit authorship? Thank you!

@sashaweiss-signal
Copy link
Contributor

This was merged internally as d7e9a88, which should become public tomorrow with our upcoming release! Thanks for the contribution.

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

Successfully merging this pull request may close these issues.

Disappearing Bottom Controls in ImageEditorCropViewController
2 participants