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

Fixed Navigation Bar disappearing on zooming in and out. #5944

Closed

Conversation

MarlowBrown
Copy link
Contributor

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, iOS 18.2

Description

Fixed a bug causing the navigation bar to disappear and not able to reappear while specifically zooming on the MediaPageViewController. I tested this by repeatedly trying different combinations of tapping and zooming in and out of an image to make the navigation bar appear and disappear. This fixes #5889.

Simulator.Screen.Recording.-.iPhone.16.-.2025-01-22.at.15.08.06.mp4

…eappear while specifically zooming on the MediaPageViewController.

(cherry picked from commit ca57ff35dde20e8199e1d7512c9d800356c601cd)
Copy link
Contributor

@sashaweiss-signal sashaweiss-signal left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution! I'll plan to remove the comment when I make the internal commit, and fix up the whitespace changes that are failing our linter scripts.

@@ -403,6 +403,13 @@ extension MediaItemViewController: UIScrollViewDelegate {
delegate?.mediaItemViewControllerWillBeginZooming(self)
}

//This is called at the end of whenever we zoom in, or zoom out. "Zooming" in this context means both zooming in and out.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: this comment isn't adding much.

@MarlowBrown
Copy link
Contributor Author

That sounds good! Thank you very much!

@sashaweiss-signal
Copy link
Contributor

This was merged internally as 836eeaaabb, which should become public in a week or two with an upcoming release!

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

Successfully merging this pull request may close these issues.

2 participants