WordPress-iOS
WordPress-iOS copied to clipboard
Fix oversize label in no result side menu on iPad
Fixes #19244 #19246
Description:
This PR fixes the issue where the labels are oversizing in its container view.
| • | Before | After |
|---|---|---|
| iPad | ![]() |
![]() |
| iPad-RTL | ![]() |
![]() |
Root Cause:
In the xib NoResults.storyboard, the container view of the labels only has the width constraint Width <= 360, but it doesn't have a constraint related to its super view. This will let the labels go outside if its super view is smaller than the label's content.
In this case, the side menu view only has width == 320. The container view will still go up to 360 because it doesn't care about its super view size.
Fix:
- Add missing width constraint between the container view and its super view
Test Step:
- Launch the app.
- Log in with an account without any notification or sign up for a new account.
- Navigate to the
NotificationsorMy Sitetab. - Notice the text should fit inside the left layout.
Regression Notes
- Potential unintended areas of impact
- I list all impacted pages in here with screenshots.
- What I did to test those areas of impact (or what existing automated tests I relied on)
- Manually test on all impacted pages on iPad, and key pages on iPhone, and list the test result here
- What automated tests I added (or what prevented me from doing so) add unitTest at 41c4cb3900b9b4893dcabdbace0ca2c7a3d3246d for testing labels width in different sizes of the container.
PR submission checklist:
- [x] I have completed the Regression Notes.
- [x] I have considered adding unit tests for my changes.
- [x] I have considered adding accessibility improvements for my changes.
- [x] I have considered if this change warrants user-facing release notes and have added them to
RELEASE-NOTES.txtif necessary.
Any views that present NoResultViewController
👋 @ipalm0423 - do you think we should list out the views that use NoResultsViewController so when potentially non-technical testers test these changes, they'll know some to try? Thanks!
Any views that present NoResultViewController
👋 @ipalm0423 - do you think we should list out the views that use
NoResultsViewControllerso when potentially non-technical testers test these changes, they'll know some to try? Thanks!
Sure, I created a view list that is inspected on this PR.
Also, I added some tests in the 41c4cb3900b9b4893dcabdbace0ca2c7a3d3246d. To test the NoResultsViewController label width in different width of container.
CI branch: https://github.com/wordpress-mobile/WordPress-iOS/pull/19333
This looks really good @ipalm0423! Thanks for being so thorough with your manual tests and screenshots. 🙇 The unit tests are great, too.
I noticed the margins on this view were a little tight. Do you think we should tweak this?
![]()
Yes, we could tweak the horizontal margin to make it look nice. Do we have the UI design specification for this margin size? (Figma, Zeplin...etc?)
I only found this spec, but not sure if it's the correct one.
Yes, we could tweak the horizontal margin to make it look nice. Do we have the UI design specification for this margin size? (Figma, Zeplin...etc?)
I only found this spec, but not sure if it's the correct one.
Typically we would pull in a designer for Figma designs, but how do you feel about adding just a few points of margin on the sides so it's not as snug?
Typically we would pull in a designer for Figma designs, but how do you feel about adding just a few points of margin on the sides so it's not as snug?
I tweaked the margin for 20 on f05670ec1022b74c5209a3a61933966ddcacd2df , also modify the unit-test
Tested on the narrow key pages:
margin 20 |
default | RTL |
|---|---|---|
| iPhone | ![]() |
![]() |
| iPad | ![]() |
![]() |
I tweaked the margin for
20on f05670e , also modify the unit-test
Great! Thanks for doing that. I've pushed the latest to the CI branch to run tests. 👍
| Warnings | |
|---|---|
| :warning: | This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone |
Generated by :no_entry_sign: dangerJS
@ipalm0423 can we update
RELEASE-NOTES.txtfor these changes? Thanks!
No, problem. Rebased to trunk, and update the release note at ec96ae642b249bd35cf81b1b8dce3182b8151c3d.
Latest has been pushed to CI branch.







