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

Stats: restore selected site ID when widget Stats modal is dismissed #13195

Merged
merged 5 commits into from Jan 8, 2020

Conversation

@ScoutHarris
Copy link
Contributor

ScoutHarris commented Jan 7, 2020

Fixes #13194

The problem was that the site ID used for stats was being overwritten by the site ID from the widget. To resolve this, the selected site ID is restored when the widget's Stats modal is dismissed. The Stats view is then refreshed for the correct site ID.

To test:

  • Add the Today and/or the All-Time widget to the Today view.
  • In the app:
    • Select a site. Go to Stats > Widgets > Use this site.
    • Select a different site, and go to Stats.
  • Close the app.
  • Return to the Today view and tap a widget.
  • The app is launched, and a modal is displayed with the Stats for the widget site.
  • Tap Done to close the modal.
  • Verify the Stats view then displays the Stats for the selected site (not the widget site).

PR submission checklist:

  • I have considered adding unit tests where possible.

  • 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 14.0 milestone Jan 7, 2020
@ScoutHarris ScoutHarris requested a review from bjtitus Jan 7, 2020
@ScoutHarris ScoutHarris self-assigned this Jan 7, 2020
@peril-wordpress-mobile

This comment has been minimized.

Copy link

peril-wordpress-mobile bot commented Jan 7, 2020

You can trigger an installable build for these changes by visiting CircleCI here.

@bjtitus
bjtitus approved these changes Jan 8, 2020
Copy link
Contributor

bjtitus left a comment

:shipit:

Seems to behave as expected.

Out of scope: I wonder if we could remove the flash of the old stats by triggering a query refresh earlier. Also still trying to wrap my head around the singleton usage. I feel like the siteID should be set for each view controller instance, but I need to investigate a little further to understand the whole Store system.

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

ScoutHarris commented Jan 8, 2020

Thanks @bjtitus .

I wonder if we could remove the flash of the old stats by triggering a query refresh earlier.

Yea, I wasn't real happy with it showing the widget stats then the selected site stats, but I wanted to get a fix out in the next release. I'll look at this a bit closer separately.

@ScoutHarris ScoutHarris merged commit 61308ba into develop Jan 8, 2020
7 checks passed
7 checks passed
Hound No violations found. Woof!
Peril All green. Nice work.
Details
ci/circleci: Build Tests Your tests passed on CircleCI!
Details
ci/circleci: Installable Build/Hold Your job is on hold on CircleCI!
Details
ci/circleci: UI Tests (iPad Air 3rd generation) Your tests passed on CircleCI!
Details
ci/circleci: UI Tests (iPhone 11) Your tests passed on CircleCI!
Details
ci/circleci: Unit Tests Your tests passed on CircleCI!
Details
@ScoutHarris ScoutHarris deleted the fix/13194-stats_displayed_after_modal_dismiss branch Jan 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.