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

Jetpack Focus: Display phase two/three overlays for Stats, Reader, and Notifications #19703

Merged
merged 20 commits into from
Dec 7, 2022

Conversation

hassaanelgarem
Copy link
Contributor

Closes #19593

Description

This PR displays the feature-specific overlay for phases two and three for Stats, Reader, and Notification.

Notes

To facilitate testing, these changes have been pushed and will be reverted before merging, hence the "Do not merge" label.

  • 3c91594: Disable overlay frequency logic
  • 7994448: Override remote deadline date to be Dec 31st
  • fa56eae: Override remote blog post URLs for phases 2 and 3. The URLs provided are placeholders since the actual blog posts are not ready yet.

Screenshots

Phase Stats Reader Notifications
Phase Two Light phase-two-stats-light phase-two-reader-light phase-two-notifs-light
Phase Two Dark phase-two-stats-dark phase-two-reader-dark phase-two-notifs-dark
Phase Three Light phase-three-stats-light phase-three-reader-light phase-three-notifs-light
Phase Three Dark phase-three-stats-dark phase-three-reader-dark phase-three-notifs-dark

Testing Instructions

Phase Two

  1. Open the app
  2. Make sure that only the "Jetpack Features Removal Phase Two" flag is enabled from the debug menu
  3. Navigate to stats
  4. Make sure the overlay is displayed with the correct content and matches the design
  5. Turn on dark mode
  6. Make sure the overlay matches the dark mode design
  7. Repeat the above steps for Reader and Notifications

Phase Three

  1. Open the app
  2. Make sure that only the "Jetpack Features Removal Phase Three" flag is enabled from the debug menu
  3. Navigate to stats
  4. Make sure the overlay is displayed with the correct content and matches the design
  5. Turn on dark mode
  6. Make sure the overlay matches the dark mode design
  7. Repeat the above steps for Reader and Notifications

Regression Notes

  1. Potential unintended areas of impact
    N/A

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

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

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.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Dec 1, 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 pr19703-eebdca3 on your iPhone

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

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Dec 1, 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 pr19703-eebdca3 on your iPhone

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

Copy link
Contributor

@momo-ozawa momo-ozawa left a comment

Choose a reason for hiding this comment

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

Works as described

@JB184351
Copy link
Contributor

JB184351 commented Dec 5, 2022

Was looking on the files added in this PR and noticed there weren't any tests added, do we have any Unit Tests for the Remote config?

@hassaanelgarem
Copy link
Contributor Author

@JB184351 Remote config logic was added in a previous PR along with its tests.
Relevant PRs: #19483 #19686

@JB184351
Copy link
Contributor

JB184351 commented Dec 5, 2022

@JB184351 Remote config logic was added in a previous PR along with its tests. Relevant PRs: #19483 #19686

Oh great, I must have missed it, thanks for confirming that.

@JB184351
Copy link
Contributor

JB184351 commented Dec 6, 2022

Testing I did for this PR. Wanted to test the Day Frequency logic by adjusting the tie but looks like the only way to change the time configuration was in the JetpackOverlayFrequencyTrackerTests class/case. Unless I'm missing something there? I did the following for Phases 2 & 3.

  1. Dark and Light Mode
  2. Portrait & Landscape Modes
  3. Tested on iPhone 13 Mini, iPhone 14 Pro & iPad Mini (6th gen) (Physical Devices I have available for testing)
  4. RTL support
  5. Voice Over
  6. Dynamic Text Size
  7. The link from the Learn more at jetpack.com
  8. Putting the app in the background & opening it again.

Copy link
Contributor

@JB184351 JB184351 left a comment

Choose a reason for hiding this comment

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

LGTM!

@hassaanelgarem
Copy link
Contributor Author

Wanted to test the Day Frequency logic

@JB184351 There were no changes to the frequency logic since #19571 so there's no need to retest this 👍

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.

Jetpack Focus: Display feature-specific overlays for later phases
4 participants