Bugfix FXIOS-13606 Remove delay while clicking "Set as Default Browser" #29602
Conversation
There was a problem hiding this comment.
thanks @razvanlitianu!! Code looks good, but I have some small comments to consider, not blocking so approving. Could we also update the PR to show the before and after animations. It helps reviewers but also if we ever need to revisit this ticket, we can quickly glance at the issue.
| // swiftlint:disable:next unneeded_override | ||
| override public func viewDidAppear(_ animated: Bool) { | ||
| super.viewDidAppear(animated) | ||
| // No animation here - it's handled in viewWillAppear to prevent double animation | ||
| } |
There was a problem hiding this comment.
do we need to override this at all then? Could we remove?
There was a problem hiding this comment.
Removing it would make the code from the subclass to work meaning it would double the animation from the original BottomSheetViewController. It's strange that we have to do this I know.
|
|
||
| override public func viewWillAppear(_ animated: Bool) { | ||
| super.viewWillAppear(animated) | ||
| // Move animation from viewDidAppear to viewWillAppear for onboarding |
There was a problem hiding this comment.
I don't see the animation being removed from viewDidAppear, maybe this comment would be more helpful if we explained why we need to move the animation here and say that we don't want it in viewDidAppear because the animation was slow
|
hey @cyndichin, thanks for the review! I uploaded the recordings and updated the comments, I hope they are more suggestive. |
🧹 Tidy commitJust 3 file(s) touched. Thanks for keeping it clean and review-friendly! 🙌 Friday high-fiveThanks for pushing us across the finish line this week! 🙌 Client.app: Coverage: 37.31
ComponentLibrary: Coverage: 23.75
Generated by 🚫 Danger Swift against 4e2296c |
📜 Tickets
Jira ticket
Github issue
💡 Description
Animating the view in viewDidAppear was giving the impression that the animation was slow.
🎥 Demos
Before
ScreenRecording_09-26-2025.16-26-43_1.MP4
After
ScreenRecording_09-26-2025.16-35-10_1.MP4
Demo
📝 Checklist
@Mergifyio backport release/v150.0)