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

Fix text-only empty state view layout #19820

Merged
merged 3 commits into from
Jan 9, 2023

Conversation

salimbraksa
Copy link
Contributor

@salimbraksa salimbraksa commented Dec 28, 2022

Fixes #19620

Description

This PR resolves a layout issue impacting the "No media matching your search" message of the Media Picker screen.

Before After
CleanShot.2022-12-29.at.14.23.06.mp4
CleanShot.2022-12-29.at.13.58.46.mp4

Test Instructions

  1. Run WordPress app and log into your account
  2. Tap "+" floating button then tap "Blog Post"
  3. Add gallery block
  4. Add media via WordPress Media Library
  5. Select at least one image
  6. Enter search term that will not return any results
  7. Tap "Edit 1"
  8. If you're not already using landscape mode, rotate now
  9. Tap "<WordPress Media" to go back
  10. Expect the "No media matching your search" message to be centered horizontally

Regression Notes

  1. Potential unintended areas of impact
    The NoResultsViewController code was slightly modified and shouldn't cause unintended bugs. But it would be great to double check the empty state of other screen looks normal ( e.g. Notifications screen ).

  1. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manual testing.

  2. What automated tests I added (or what prevented me from doing so)
    None.

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.
@salimbraksa salimbraksa added this to the 21.6 milestone Dec 28, 2022
@salimbraksa salimbraksa self-assigned this Dec 28, 2022
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Dec 28, 2022

You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19820-cc8e54e on your iPhone

If you need access to App Center, please ask a maintainer to add you.
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Dec 28, 2022

You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr19820-cc8e54e on your iPhone

If you need access to App Center, please ask a maintainer to add you.
@salimbraksa salimbraksa marked this pull request as ready for review December 29, 2022 14:16
Copy link
Contributor

@twstokes twstokes left a comment

Choose a reason for hiding this comment

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

LGTM @salimbraksa! 🚀 The testing steps passed for me.

I was curious why this check existed in the first place and saw that it was added in #11550 due to a crash. I failed to reproduce it (although they couldn't in the PR either) and noticed that the stack trace in the issue included the call to traitCollectionDidChange which was replaced semi-recently in #19555. Perhaps that's no longer a risk.

I tested a few other views that use the NRV and didn't spot any issues:

  • Site Tags
  • Themes
  • Stock Photos
  • Pages
  • Posts
@salimbraksa salimbraksa enabled auto-merge (squash) January 9, 2023 14:14
@salimbraksa salimbraksa merged commit 2d6eeca into trunk Jan 9, 2023
@salimbraksa salimbraksa deleted the task/19620-fix-media-no-results-layout branch January 9, 2023 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants