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] Make Landing Screen Revamp Feature Flag Configurable Remotely #17238

Closed

Conversation

ovitrif
Copy link
Contributor

@ovitrif ovitrif commented Sep 30, 2022

Fixes #17236

This PR makes the feature flag to toggle the Revamped Landing Screen for both WordPress and Jetpack apps configurable remotely.

📣 Important Please run both test cases to leave the feature flag disabled.

To test:

1️⃣ Test Case 1: Remote feature flag enabled

  1. Go to Firebase Remote Config, set the default value of the landing_screen_revamp_remote_field feature flag to true and publish your changes.
  2. Launch the WordPress / Jetpack App and make sure you're logged out.
  3. Verify that the new landing screen is displayed.

2️⃣ Test Case 2 - Remote feature flag disabled

  1. Go to Firebase Remote Config, set the default value of the landing_screen_revamp_remote_field feature flag to false and publish your changes.
  2. Launch the WordPress / Jetpack App and make sure you're logged out.

3️⃣ Test Case 3: Local feature flag override

  1. Launch the WordPress / Jetpack app.
  2. Verify that the old landing screen is displayed.
  3. Login.
  4. Go MeApp SettingsDebug settings → check landing_screen_revamp_remote_field on and tap RESTART THE APP at the bottom.
  5. Logout of the app.
  6. Verify that the new landing screen is displayed.

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 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 requested review from mkevins and a team September 30, 2022 16:42
@ovitrif ovitrif self-assigned this Sep 30, 2022
@ovitrif ovitrif added this to the 21.0 milestone Sep 30, 2022
@ovitrif ovitrif requested a review from antonis October 3, 2022 10:24
Copy link
Member

@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 👍
The code looks great and the feature flag is enabled/disabled remotely as expected 🎉

I noticed an issue with the timing the change is propagated in the Jetpack App on my test device (Pixel 5, Android 13, Debug build). I needed to restart the app to get the updated experience depending on the value of the remote field. I feel that this might be a limitation of the remote field approach since the landing screen is the 1st screen of the app and maybe there are race conditions that lead in loading the screen with the previous value.
If that is the case maybe we should stick to the build config approach and not use a remote field at all for this feature 🤔

@ovitrif
Copy link
Contributor Author

ovitrif commented Oct 4, 2022

I noticed an issue with the timing the change is propagated in the Jetpack App on my test device (Pixel 5, Android 13, Debug build). I needed to restart the app to get the updated experience depending on the value of the remote field. I feel that this might be a limitation of the remote field approach since the landing screen is the 1st screen of the app and maybe there are race conditions that lead in loading the screen with the previous value.
If that is the case maybe we should stick to the build config approach and not use a remote field at all for this feature 🤔

I've noticed this issue too; sometimes having to restart the app 2 times (I guess this could be due to the delay it takes to propagate the remote config).

I'm open to stick to the build config approach given the aforementioned issue.

Do you suggest we drop this PR, in that case I'll also remove the 2 issues for Android and iOS 😃 .

Thank you for reviewing and raising this concern @antonis 🙇

@antonis
Copy link
Member

antonis commented Oct 4, 2022

Do you suggest we drop this PR, in that case I'll also remove the 2 issues for Android and iOS 😃 .

Yep 👍
I mean if we cannot trigger the desired landing screen reliably remotely (given that there is an internet connection) maybe we should stick to shipping this with a build flag (probably together with the new splashscreens if we have the time).

@ovitrif
Copy link
Contributor Author

ovitrif commented Oct 4, 2022

Relying on a remote feature flag is not safe for this feature → we're going forward with a build flag (see discussion in comments).

@ovitrif ovitrif closed this Oct 4, 2022
@ovitrif ovitrif deleted the issue/17236-revamp-landing-screen-remote-feature-flag branch November 7, 2022 15:01
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.

[Revamp Landing Screen] Make feature flag configurable remotely
2 participants