[iOS] Exit guided tour when opening from external intent [chromium/src : main]
Robbie Gibson (Gerrit)
Robbie Gibson voted and added 2 comments![]( "Open in Gerrit")
Votes added by Robbie Gibson
| Commit-Queue | +1 |
2 comments
+gambard, PTAL, especially at my question in tab_grid_view_controller. I have very little context for whether that change is safe. I don't know when the first branch is supposed to execute.
if (!IsNewTabGridTransitionsEnabled()) {After making the tour dismiss as part of `SceneCoordinator closePresentedViews]`, I encountered a bug where the toolbar would get hidden and never reappear. It looks like below, in `contentWillDisappearAnimated:`, during the dismissal, there was a `self.transitionCoordinator`, so `animateToolbarsForDisappearance` was called, hiding the toolbars.
But then when the tab grid reappeared, there was no transition coordinator, so the toolbars never showed.
The Transition Coordinator might have come from the Guided Tour bubble having a custom `UIViewControllerTransitioningDelegate`/animator.
I'm not sure if moving the check to outside this whole block could cause issues somewhere else.
Related details
- Gauthier Ambard
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Gauthier Ambard (Gerrit)
Gauthier Ambard voted and added 6 comments![]( "Open in Gerrit")
Votes added by Gauthier Ambard
| Code-Review | +1 |
6 comments
case GuidedTourStep::kTabGridIncognito:Is it expected that this one is doing the same thing as the kTabGridTabGroup? Maybe add a comment explaining why.
[tabGridHandler hideTabGridGuidedTour];Add a break here
- (void)hideTabGridGuidedTour;Add empty line and comment
// will be executed after the step dismisses.Update comment. Consider adding an empty line before the "hide" method and a comment
if (!IsNewTabGridTransitionsEnabled()) {After making the tour dismiss as part of `SceneCoordinator closePresentedViews]`, I encountered a bug where the toolbar would get hidden and never reappear. It looks like below, in `contentWillDisappearAnimated:`, during the dismissal, there was a `self.transitionCoordinator`, so `animateToolbarsForDisappearance` was called, hiding the toolbars.
But then when the tab grid reappeared, there was no transition coordinator, so the toolbars never showed.
The Transition Coordinator might have come from the Guided Tour bubble having a custom `UIViewControllerTransitioningDelegate`/animator.
I'm not sure if moving the check to outside this whole block could cause issues somewhere else.
I am not sure animateToolbarsForAppearance is used at all. I suspect it is a leftover from when the BVC was presented?
At least, I am not able to trigger it.
Please try this code with the flag disabled to make sure it is working as you expect 😊
Related details
- Robbie Gibson
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Robbie Gibson (Gerrit)
Robbie Gibson added 5 comments![]( "Open in Gerrit")
Is it expected that this one is doing the same thing as the kTabGridTabGroup? Maybe add a comment explaining why.
Yes, it's expected, adding comment.
[tabGridHandler hideTabGridGuidedTour];Robbie GibsonAdd a break here
Done
Add empty line and comment
Done.
Update comment. Consider adding an empty line before the "hide" method and a comment
- Gauthier Ambard
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |