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

Fix: Quick Start Prompt not shown on API 25 #16238

Merged
merged 1 commit into from
Apr 2, 2022

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Apr 2, 2022

Fixes #16225

This PR adds a quick fix for the issue where we noticed that the order of nested child fragments creation/parent fragment result handling is different between different Android versions (as a result QS prompt was not shown after the Login Epilogue was dismissed) by adding a small delay before passing the result to nested child fragments.

(Internal Slack Ref: p1648794037279879-slack-C02QANACA)

It might not be a real issue as we could verify the issue only on a lower API level emulator but this workaround takes care of the scenario if it actually happens.

To test:

  1. Fresh install the app on an API 25 emulator.
  2. Login with a site eligible for Quick Start.
  3. Notice that the Quick Start prompt is shown after the login epilogue screen.

To be safe, test it on a higher API level emulator/ device as well.

Regression Notes

  1. Potential unintended areas of impact
    Other activity result scenarios like site icon change, stories photo picker should work as intended.

    Note that I also tried passing data from parent to child fragment using a shared parent fragment VM (in this branch) but the site icon change was broken with this approach.

    We might want to find a better way so that we don't have to add even this small delay but considering time constraints and that the delay added is negligible, the workaround serves the purpose.

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

  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.

@ashiagr ashiagr added this to the 19.6 milestone Apr 2, 2022
@ashiagr ashiagr requested a review from zwarm April 2, 2022 11:42
@ashiagr ashiagr self-assigned this Apr 2, 2022
@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

@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

You can test the changes on this Pull Request by downloading the APKs:

Copy link
Contributor

@zwarm zwarm left a comment

Choose a reason for hiding this comment

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

@ashiagr - I have tested in API 25 (with and without the fix) and I no longer see the issue. I also spot tested in API's above 25 and did not encounter any issues.

Thanks!

@zwarm zwarm merged commit 16eadcb into trunk Apr 2, 2022
@zwarm zwarm deleted the issue/16225-workaround-activity-result-tab-fragments branch April 2, 2022 12:18
@ashiagr ashiagr mentioned this pull request Apr 4, 2022
28 tasks
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.

Quick Start Prompt not shown on API 25
2 participants