Skip to content

Blogging Prompt Dashboard Card: show a real prompt#18534

Merged
ScoutHarris merged 1 commit intotrunkfrom
feature/18429-dashboard_card_show_prompt
May 9, 2022
Merged

Blogging Prompt Dashboard Card: show a real prompt#18534
ScoutHarris merged 1 commit intotrunkfrom
feature/18429-dashboard_card_show_prompt

Conversation

@ScoutHarris
Copy link
Contributor

Ref: #18429

The dashboard prompt card now fetches one prompt and updates the card accordingly.

To test:

  • Enable bloggingPrompts feature flag.
  • Verify the example card on the Feature Introduction has not changed.
  • Go to Home tab > prompt card.
  • Verify a prompt is displayed.
Feature Introduction Card Dashboard Card
fi_card dashboard_card

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 added this to the 19.9 milestone May 6, 2022
@ScoutHarris ScoutHarris self-assigned this May 6, 2022
@ScoutHarris ScoutHarris marked this pull request as ready for review May 6, 2022 23:17
@ScoutHarris ScoutHarris requested a review from wargcm May 6, 2022 23:18
@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 pr18534-60de999 on your iPhone

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

@wpmobilebot
Copy link
Contributor

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 pr18534-60de999 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.

Looks good! Only left a note and had one question I wasn't sure about.

self.blog = blog
self.presenterViewController = viewController
refreshStackView()
fetchPrompt()
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not a big deal and just something to note - I noticed this gets called every time the device goes from portrait to landscape. I didn't investigate too much but it looks like the dashboard might be getting recreated and that eventually ends up calling this on a view load.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed this gets called every time the device goes from portrait to landscape. I didn't investigate too much but it looks like the dashboard might be getting recreated and that eventually ends up calling this on a view load.

Thanks for noting this. It's not typical behavior so it didn't occur to me.

Probably not a big deal

If we were only going to get prompts via fetching, this is something we'd definitely not want to do. But once prompts are cached, it won't be so offensive. In the end, we should fetch only if we don't have a cached prompt. So the fetch should happen once per prompt. In theory. 😄

bloggingPromptsService.fetchTodaysPrompt(success: { [weak self] (prompt) in
self?.prompt = prompt
}, failure: { (error) in
DDLogError("Failed fetching blogging prompt: \(String(describing: error))")
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have any expected behavior for what happens on failure?

At the moment, it looks like it just displays an empty prompts card:

Screen Shot 2022-05-09 at 12 14 47 PM

Copy link
Contributor Author

@ScoutHarris ScoutHarris May 9, 2022

Choose a reason for hiding this comment

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

Oh that's a good point. Thanks! I'll show an error message in place of the title in a follow up PR.

@ScoutHarris
Copy link
Contributor Author

Thanks @wargcm !

@ScoutHarris ScoutHarris merged commit 50814c6 into trunk May 9, 2022
@ScoutHarris ScoutHarris deleted the feature/18429-dashboard_card_show_prompt branch May 9, 2022 19:25
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