-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 Traffic: Ensure deep links and widget routing work #22695
Stats Traffic: Ensure deep links and widget routing work #22695
Conversation
📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
|
📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
|
WordPress/Classes/ViewRelated/Stats/SiteStatsDashboardViewController.swift
Outdated
Show resolved
Hide resolved
…cations-widgets-work-with-traffic-tab
55e3b72
to
b58d8ce
Compare
@guarani ready for review after merging with the latest changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @staskus!
I think this is ready to merge as-is.
Widgets
- Home Screen Widgets
- ✅ Today's Views
- ✅ Today's Views, Visitors, Likes, and Comments
- ✅ This Week's Views (Small)
- ✅ This Week's Views (Large)
- ✅ All-Time Views (Small)
- Lock Screen Widgets
- ✅ Today's Views
- ✅ Today's Views & Visitors
- ✅ Today's Likes & Comments
- ✅ All-Time Views
- ✅ All-Time Views & Visitors
- ✅ All-Time Posts & Most Views
Links
- The "Stats at your fingertips":
⚠️ (not a blocker) this opens the Insights screen, but opening the Traffic tab could make more sense for users to "See your real-time stats anytime, anywhere".
- WordPress.com/stats
- ❌ https://wordpress.com/stats/day doesn't open "By day", it's opening the most recently opened period instead
- This is an existing issue (tested with the current production version 24.2.1, I'll make a separate issue for it (update: Deep link to Stats day not working #22714)
- ✅ https://wordpress.com/stats/week
- ✅ https://wordpress.com/stats/month
- ✅ https://wordpress.com/stats/year
- ❌ https://wordpress.com/stats/day doesn't open "By day", it's opening the most recently opened period instead
- ✅ https://wordpress.com/stats/insights/testsite.wordpress.com
- ✅ Tested Tab Switching Regression scenario
Yes, good observation. Looks like |
Ensure deep links and widget routing work
https://github.com/wordpress-mobile/jetpack-issue-repo/issues/16
https://github.com/wordpress-mobile/jetpack-issue-repo/issues/17
Problem
Routing is supported by setting
StatsPeriodType
in user preferences and then reading from it when determining which tab to display when opening Stats.This solution worked well when
/days
/months
/years
/insights
routes mapped well to tabs displayed on the Stats Dashboard.However, with the introduction of
Traffic
tab, we need to separate the concept of the selected tab (Insights / Traffic), and the selected period unit (Days/Weeks/Months/Years) while at the same time keep supporting the previous configuration with feature flag disabled.Solution
StatsPeriodType
toStatsTabType
to correctly reflect that thisenum
is used to represent tabs displayed on Stats DashboardSiteStatsDashboardPreferences
StatsTabType
during the routing process, also setStatsPeriodUnit
, mapping/days to .day
,/months to .month
...getSelectedPeriodUnit
) which was preselected during the routing process. When the feature flag is disabled,getSelectedPeriodUnit
is not used since the selected tab is equal to the selected unit as well.To test:
Widgets
Links
Tab Switching regression
Videos
Stats Traffic feature flag enabled
Routing.Stats.Traffic.mp4
Stats Traffic feature flag disabled
Routing.Before.mp4
Regression Notes
Breaking existing routing functionality
PR submission checklist:
RELEASE-NOTES.txt
if necessary.Testing checklist: