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 Content Migration Flow] Add Notifications Screen to the flow #17435

Merged
merged 7 commits into from
Nov 16, 2022

Conversation

ovitrif
Copy link
Contributor

@ovitrif ovitrif commented Nov 7, 2022

Fixes Partially #17372 (doesn't disable notifications in WP which is wip in #17371 nor navigates to next step).

Depends on #17425 so that PR should be reviewed & merged first, then this one is ready for review.
The [Status] Not Ready for Merge label can be removed once the PR it depends on is merged.

This PR implements the UI for the notifications step in the WordPress to Jetpack migration flow.
It also adds crossfade transition between the migration flow screens.

🏁 All these changes are behind a temporary feature flag.

🔔 Notice: Once the feature flag is enabled the migration flow will always be presented. The Jetpack app build has to be uninstalled (or "Clear Data" should be done) in order to revert back to the normal functionality. Further development on the data layer will make sure the migration is not attempted again once it's successful. To be partially handled in #17406.

To test:

  1. Open the Jetpack app & login
  2. Go to MeApp SettingsDebug Settings
  3. Enable the JetpackMigrationFlowFeatureConfig & restart the app.
  4. Tap the Continue button on the "Welcome to Jetpack!" screen
    • Note: Temporarily the migration progress is simulated by showing for 2 seconds a processing state.
  5. Expect to see the "Notifications now come from Jetpack" screen
    • The Continue button has no functionality in this PR, that's handled in #17440.

Previews

Light Dark

Regression Notes

  1. Potential unintended areas of impact
    None

  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)
    None - PR adds only UI and UI state changes.

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.

@ovitrif ovitrif added this to the 21.2 milestone Nov 7, 2022
@ovitrif ovitrif requested a review from a team November 7, 2022 15:25
@ovitrif ovitrif changed the title Issue/17372 jp migration notifications screen [Jetpack Content Migration Flow] Add Notifications Screen to the flow Nov 7, 2022
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 7, 2022

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

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Nov 7, 2022

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

@ovitrif ovitrif requested review from antonis and mkevins November 7, 2022 20:26
@ovitrif ovitrif force-pushed the issue/17372-jp-migration-notifications-screen branch from 4da4453 to a0a8ed6 Compare November 9, 2022 12:23
@ovitrif ovitrif force-pushed the issue/17368-jp-migration-welcome-screen-design-updates branch from 2116a45 to 809738d Compare November 9, 2022 13:21
@ovitrif ovitrif force-pushed the issue/17372-jp-migration-notifications-screen branch from a0a8ed6 to e7dd931 Compare November 9, 2022 13:43
Base automatically changed from issue/17368-jp-migration-welcome-screen-design-updates to trunk November 10, 2022 11:12
@peril-wordpress-mobile
Copy link

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

@ovitrif ovitrif modified the milestones: 21.2, 21.3 Nov 10, 2022
Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Great work @ovitrif 👍
I've tested the app on a Pixel 5 (Android 13) both with the JETPACK_MIGRATION_FLOW true and false and everything worked as expected. The code changes also LGTM 🎉

@antonis antonis merged commit 166c03d into trunk Nov 16, 2022
@antonis antonis deleted the issue/17372-jp-migration-notifications-screen branch November 16, 2022 12:51
@ovitrif ovitrif linked an issue Nov 17, 2022 that may be closed by this pull request
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.

Android - Permissions Screen
3 participants