Skip to content

Comments

Blogging Prompts Dashboard Card: hide prompt card when prompt skipped#18642

Merged
ScoutHarris merged 5 commits intotrunkfrom
feature/18429-skip_this_prompt
May 18, 2022
Merged

Blogging Prompts Dashboard Card: hide prompt card when prompt skipped#18642
ScoutHarris merged 5 commits intotrunkfrom
feature/18429-skip_this_prompt

Conversation

@ScoutHarris
Copy link
Contributor

@ScoutHarris ScoutHarris commented May 17, 2022

Ref: #18429, #18250

When Skip this prompt is selected from the prompt dashboard:

  • The prompt information is stored in User Defaults.
  • The card is removed from the Dashboard.

Bonus: the card will now use the cached prompt if there is one.

To test:

  • Enable bloggingPrompts feature flag.
  • Select the My Site Home tab.
  • On the prompt card menu, select Skip this prompt.
  • Verify the card is removed from the Dashboard.
  • Select a different site.
  • Verify the prompt card appears.
prompt no_prompt

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.

@ScoutHarris ScoutHarris requested review from dvdchr and wargcm May 17, 2022 21:08
@ScoutHarris ScoutHarris marked this pull request as ready for review May 17, 2022 21:08
@wpmobilebot
Copy link
Contributor

wpmobilebot commented May 17, 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 pr18642-1dfa019 on your iPhone

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

Copy link
Contributor

@wargcm wargcm left a comment

Choose a reason for hiding this comment

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

Hey @ScoutHarris 👋, I ran into a few issues while testing different scenarios:

Prompt for other sites being skipped Unable to skip prompt on other sites Skipped prompt reappears
  • Launch app
  • Site 1 appears, change to a different site
  • Skip site 2's prompt
  • Site 2's prompt remains on the dashboard
  • Switch to site 1
  • Site 1's prompt is now gone
  • Launch app
  • Skip site 1's prompt
  • Switch to site 2
  • Site 2's prompt can't be skipped
  • Launch app
  • Skip site 1's prompt
  • Switch to site 2
  • Kill and relaunch the app
  • Skip site 2's prompt
  • Switch to site 1
  • Prompt is shown again

@ScoutHarris
Copy link
Contributor Author

ScoutHarris commented May 18, 2022

Hey @wargcm .

Thanks for the testing! Much appreciated.

The problem was twofold:

  1. I failed to correctly manage skipped prompts by siteID in UserDefaults. 🤦
  2. I was using prompt.siteID in UD. However, the prompt doesn't update when the site changes, so it didn't always match the displayed blog's siteID. UD is now using blog.dotcomID instead of prompt.siteID.

Ready for another go if you please. You'll want to reinstall the app to purge the defective user defaults. Thank 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 pr18642-1dfa019 on your iPhone

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

Copy link
Contributor

@wargcm wargcm left a comment

Choose a reason for hiding this comment

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

@ScoutHarris Each of those have been fixed and I don't see any other issues. Looks good! :shipit:

@ScoutHarris
Copy link
Contributor Author

Thanks @wargcm !

@ScoutHarris ScoutHarris merged commit 5a6c648 into trunk May 18, 2022
@ScoutHarris ScoutHarris deleted the feature/18429-skip_this_prompt branch May 18, 2022 19:35
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.

3 participants