Skip to content

Fixes: Try the jetpack app not being shown properly when the font size is large #17540

Merged
AjeshRPai merged 8 commits intotrunkfrom
issue/17521-button-text-is-truncated
Nov 25, 2022
Merged

Fixes: Try the jetpack app not being shown properly when the font size is large #17540
AjeshRPai merged 8 commits intotrunkfrom
issue/17521-button-text-is-truncated

Conversation

@AjeshRPai
Copy link
Copy Markdown
Contributor

@AjeshRPai AjeshRPai commented Nov 25, 2022 •
edited
Loading

Fixes #17521

This PR fixes the issue when the display and font size are large, the button text is not visible.

Changes

  1. Makes the button wrap the button text so that its visible. The button text is set to have a max text line of 2 right now
  2. Makes the content scrollable including the buttons in Landcape and Portrait mode so that the content as well as the Button texts are visible .

The buttons would be visible on the bottom as before in normal text size and when the display size increases, the buttons will be visible when the content is scrolled.

Before Fix After fix
Screenshot_20221122_193453 Screenshot_20221125-165249

To test:

Pre- Requisites
Launch WP app -> Go to my site -> App settings -> Debug settings -> Enable jp_removal_one flag

Check if the fonts are displayed properly on Properly in Landscape and Portrait

  1. Launch the app -> Go to my site -> Access stats/reader/notification feature
  2. Verify that the UI is shown properly
Landscape Portrait
Screenshot_20221122_173149 Screenshot_20221122_173214

Check if the fonts are displayed properly on increasing the size of the fonts

  1. Go to system settings -> Display and font size -> Increase both display size and font size
  2. Launch the app -> Go to my site -> Access stats/reader/notification feature
  3. Verify that the UI is shown properly
Landscape(Scrollable) Portrait
Screenshot_20221125-002359 Screenshot_20221122_173422

Landscape Scrollable - Video capture
https://user-images.githubusercontent.com/17463767/203991283-b927abb2-1847-4dd0-b536-eb91b2b96789.mp4

cc: @JB184351 @zwarm for visibility

⚠️ Merge Instructions

  • Revert commit ffbe6fe - used for testing purposes
  • Remove Not Ready for Merge label
  • Merge as normal

Regression Notes

  1. Potential unintended areas of impact
    Jetpack overlay is not shown properly

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

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

PR submission checklist:

  • I have completed the Regression Notes.
  • 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.

@AjeshRPai AjeshRPai added Jetpack [Status] Needs Design Review A designer needs to sign off on the implemented design. [Status] Not Ready for Merge labels Nov 25, 2022
@AjeshRPai AjeshRPai added this to the 21.3 milestone Nov 25, 2022
@AjeshRPai AjeshRPai self-assigned this Nov 25, 2022
@peril-wordpress-mobile
Copy link
Copy Markdown

peril-wordpress-mobile bot commented Nov 25, 2022 •
edited
Loading

Warnings
⚠️ 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 🚫 dangerJS

@AjeshRPai
Copy link
Copy Markdown
Contributor Author

AjeshRPai commented Nov 25, 2022 •
edited
Loading

@ovitrif

First of all, thank you for fixing the Button being truncated issue with this PR. 🙌🏼 . I have used the same logic in the Feature removal overlay also. 😄

During the review of this PR, @shaunandrews pointed out that the primary colour of the button is hex colour #008710 - jetpack_green_50. Assuming the same applies to the jetpack-powered bottom sheet. I have updated the button colour in the jetpack-powered bottom sheet with this commit.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Nov 25, 2022 •
edited
Loading

WordPress📲 You can test these changes on WordPress by downloading wordpress-installable-build-pr17540-53bf625.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppWordPress
Build FlavorJalapeno
Build TypeDebug
Commit53bf625
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@wpmobilebot
Copy link
Copy Markdown
Contributor

wpmobilebot commented Nov 25, 2022 •
edited
Loading

Jetpack📲 You can test these changes on Jetpack by downloading jetpack-installable-build-pr17540-53bf625.apk
💡 Scan this QR code with your Android phone to download and install the APK directly on it.
AppJetpack
Build FlavorJalapeno
Build TypeDebug
Commit53bf625
Note: This installable build uses the JalapenoDebug build flavor, and does not support Google Login.

@ovitrif
Copy link
Copy Markdown
Contributor

ovitrif commented Nov 25, 2022

During the review of this PR, @/shaunandrews pointed out that the primary colour of the button is hex colour #008710 - jetpack_green_50. Assuming the same applies to the jetpack-powered bottom sheet. I have updated the button colour in the jetpack-powered bottom sheet with this commit.

Thank you for adjusting the color of the primary button on the jetpack-powered bottom sheet, I do agree with applying the change to align with Shaun's feedback 🙇

@ovitrif

First of all, thank you for fixing the Button being truncated issue with this PR. 🙌🏼 . I have used the same logic in the Feature removal overlay also. 😄

My pleasure @AjeshRPai, I'm really glad it could be useful 🙇, it wasn't straightforward to come up with this solution and it seemed to be the only one after so many other attempts failed 😅.
Seeing that you could leverage it for the feature removals overlay means the effort was worth it 👍

osullivanchris
osullivanchris approved these changes Nov 25, 2022
View reviewed changes
Copy link
Copy Markdown

@osullivanchris osullivanchris left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good @AjeshRPai nice work resolving those details!!

One small thing, there should be some padding between the body copy and the button in large display on landscape. Probably was not defined before when the buttons were sticky. But in this case where the button is not sticky, could do with 16dp padding there. Not a blocker if you can't address immediately.

image
edited to add image

Thanks!

@AjeshRPai
Copy link
Copy Markdown
Contributor Author

AjeshRPai commented Nov 25, 2022

@osullivanchris

I have added padding 16dp between the caption and button with this commit - 8b2ccfa

Screenshot_20221125-212438

I am going ahead and merging this PR.

@AjeshRPai AjeshRPai enabled auto-merge November 25, 2022 15:56
@osullivanchris
Copy link
Copy Markdown

osullivanchris commented Nov 25, 2022

Thanks @AjeshRPai looks good!

@AjeshRPai AjeshRPai merged commit 06ba2ef into trunk Nov 25, 2022
@AjeshRPai AjeshRPai deleted the issue/17521-button-text-is-truncated branch November 25, 2022 16:09
@ovitrif ovitrif mentioned this pull request Jan 13, 2023
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@osullivanchris osullivanchris osullivanchris approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@AjeshRPai AjeshRPai

Labels

Jetpack [Status] Needs Design Review A designer needs to sign off on the implemented design.

Projects

None yet

Milestone

21.3

Development

Successfully merging this pull request may close these issues.

Jetpack Focus: 'Try the new Jetpack app' button text is truncated for big fonts

4 participants

@AjeshRPai @wpmobilebot @ovitrif @osullivanchris