Skip to content

Conversation

@staskus
Copy link
Contributor

@staskus staskus commented Dec 28, 2022

Fixes #19720

Description

Fixing Insights Update prompt header not adapting to dynamic text size changes. This is a minor issue since the view does adapt to dynamic type and the bug only happens when dynamic type changes when the view is open.

The issue happens because Insights Update uses a more advanced CollapsableHeaderViewController which has more complex header height calculations that are not recalculated when the dynamic text changes.

The solution is to manually ask to layout header when traitCollectionDidChange event is triggered by the system

Testing instructions

Enable "New Appearance for Stats" and "New Cards for Stats Insights" feature flags

Case 1:

  1. Fresh install Jetpack (or kill the app after enabling feature flags)
  2. Log in
  3. Insights Update pop up should appear
  4. Increase font size
  5. The header height should be adapting depending on the font size

Regression Notes

  1. Potential unintended areas of impact

None, the changes are isolated

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.

Images & Videos

Before

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2022-12-28.at.13.55.25.mp4

After

Simulator.Screen.Recording.-.iPhone.14.Pro.-.2022-12-28.at.14.55.09.mp4

@staskus staskus added this to the 21.5 milestone Dec 28, 2022
@staskus staskus requested review from guarani and sla8c December 28, 2022 13:05
@staskus staskus changed the title [Stats Revamp] Layout Insights Update prompt header when dynamic text size changes [Stats Revamp] Recalculate the height of Insights Update prompt header when dynamic text size changes Dec 28, 2022
@wpmobilebot
Copy link
Contributor

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 pr19819-58c9ab2 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot
Copy link
Contributor

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 pr19819-58c9ab2 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Looks good! I tested this by updating font size to 200% with Spanish as the device language and the layout adapts correctly now.

None, the changes are isolated

I noticed that CollapsableHeaderViewController is used in other places like the page template picker so I tested there and it seems to work.

@staskus staskus merged commit 16fa3c3 into trunk Dec 30, 2022
@staskus staskus deleted the fix/19720-stats-revamp-insights-update-annoucement-ui-issues branch December 30, 2022 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[iOS] Stats: Insights update announcement UI issues

4 participants