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

Address NoticePresenter iPad rotation workaround #23049

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

megmil
Copy link

@megmil megmil commented Apr 22, 2024

Fixes #11900

To test: Reproduce steps in #11345

iPad Air Simulator iPhone 15 Simulator
ezgif com-video-to-gif-converter iphonetest-ezgif com-video-to-gif-converter (1)

Description

New solution for bug in #11345 (Fix Notices showing anywhere but the bottom on iPad devices)

Regression Notes

  1. Potential unintended areas of impact
    Notices on iPhone
    Split view

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Tested on iPhone 15 simulator

  3. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing checklist:

  • WordPress.com sites and self-hosted Jetpack sites.
  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@megmil
Copy link
Author

megmil commented Apr 24, 2024

@justtwago @dvdchr @guarani @wargcm Could someone review this or let me know if there's any other steps to complete before this can be looked at?

@guarani
Copy link
Contributor

guarani commented May 6, 2024

Hi @megmil, thanks for your contribution!

I couldn't reproduce the issue with the notices disappearing on the iPad. The notices appear if I go to Posts or Pages and turn off the internet. As I rotate the device's orientation, the notices work as expected (they move so they're always at the bottom of the screen, no matter the orientation). I'm testing on a physical iPad 8th generation (iPadOS 16.3).

Could you please verify the issue is present on the latest trunk commit? If so, please file an issue with steps to reproduce.

Fixes #11900

Just a note: this should point to an open issue, not a PR.

@megmil
Copy link
Author

megmil commented May 7, 2024

Hi @guarani! The issue isn't the notices disappearing on iPad (anymore)--that was fixed in #11345. However, the fix in #11345 was a workaround "hack", and Apple engineers recommended a different way to fix it, according to #11900. That's what I implemented here.

My bad with the PR/bug thing. I just saw that it was marked as "Good First Issue."

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.

Initialize NoticePresenter in didFinishLaunchingWithOptions
2 participants