Wrap long cell labels on Android #37993
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Prior to this change, the cell label container was disallowed from shrinking to accommodate its siblings. This caused longer labels to force the sibling elements out of the viewport causing them to be invisible or cut-off. Additional changes were applied to the link suggestion styles to ensure the sibling of cell icons do not encroach upon the now static width icon.
dcalhoun
added
Mobile App - i.e. Android or iOS
|
Size Change: +355 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
2 tasks
fluiddot
approved these changes
Jan 17, 2022
There was a problem hiding this comment.
LGTM 🎊 !
I checked that all test cases pass and added some minor comments (none of them should be considered as blockers) in relation to the third case and within the file changes:
- Case 1: Long Label Wraps ✅
- Case 2: Scaled Text Controls Remain Accessible ✅
- Case 3: Link Picker Aligned Correctly ✅ (with a comment)
I think it makes sense to have the same padding on both sides, however, I'm wondering whether the content (i.e. icon and link as a single item) under the separator line is expected to be aligned with it (see attached screenshot), wdyt?
- Case 4: Android Media Options Aligned Correctly ✅
Tested on Samsung Galaxy S20 FE 5G (Android 10) and Simulator - iPhone 12 Pro Max (iOS 15.0).
These styles are the default values and redundant.
Force link suggestion title to fill parent width, which causes the link suggestion to match the parent separator line width.
Good question. It is difficult to tell from the original implementation images and discussion within wordpress-mobile/gutenberg-mobile#2484. However, I went ahead and updated the styles to ensure the link suggestion title does align with the right side of the separator line.
I plan to move forward with merging this bug fix, but @iamthomasbishop please let me know if you disagree with the alignment change in the images. |
|
Looks good to me, thanks @dcalhoun! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related PRs
Description
Fixes #37858. Prior to this change, the cell label container was disallowed from shrinking to accommodate its siblings. This caused longer labels to force the sibling elements out of the viewport causing them to be invisible or cut-off.
Additional changes were applied to the link suggestion styles to ensure the sibling of cell icons do not encroach upon the now static width icon.
Lastly, #37995 was created to capture an existing, unrelated issue identified while testing this work.
How has this been tested?
Case 1: Long Label Wraps
English, e.g. Spanish.
Case 2: Scaled Text Controls Remain Accessible
Case 3: Link Picker Aligned Correctly
Case 4: Android Media Options Aligned Correctly
Screenshots
Long Label Wraps
Scaled Text Controls Remain Accessible
Link Picker Suggestion Aligned Correctly
Media Options Aligned Correctly
Types of changes
Bug fix
Checklist:
*.native.jsfiles for terms that need renaming or removal).