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.

Sign up for GitHub

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

Jump to bottom

Stats Traffic: Create an empty Traffic tab behind the feature flag #22381

Merged
merged 5 commits into from
Jan 12, 2024
Merged

Stats Traffic: Create an empty Traffic tab behind the feature flag #22381

merged 5 commits into from
Jan 12, 2024

Conversation

staskus
Copy link
Contributor

@staskus staskus commented Jan 12, 2024

Fixes #22380

The goal of PR is to create a mechanism to support:

  • Insights, Days, Weeks, Months, Years tabs when stats_traffic_tab feature flag is disabled
  • Traffic, Insights when stats_traffic_tab feature flag is enabled

While continuing to support existing mechanisms of remembering previous tab or period selection.

To test:

Regression

  1. Disable the feature flag
  2. Open Stats
  3. Confirm Insights, Days, Weeks, Months, Years tabs appear as before
  4. Select Insights, Close and Open Stats
  5. Confirm Insights is selected
  6. Select Weeks, Close and Open Stats
  7. Confirm Weeks is selected
  8. Switch between Tabs and Date periods
  9. Confirm mechanism works

Stats Traffic tab

  1. Enable feature flag
  2. Open Stats
  3. Confirm Traffic, Insights tabs appear
  4. Select Insights
  5. Confirm Insights appear as before, Close and Open Stats
  6. Confirm Insights is still selected
  7. Select Traffic, Close and Open Stats
  8. Confirm Traffic is selected

Stats Traffic tab after feature flag update update

  1. Disable feature flag
  2. Open Stats
  3. Select Days/Weeks/Months/Years tab
  4. Enable feature flag, Close and Open Stats
  5. Confirm Traffic tab is open
  6. Disable feature flag, Close and Open Stats
  7. Select Insights tab
  8. Enable feature flag, Close and Open Stats
  9. Confirm Insights tab is open
Simulator.Screen.Recording.-.iPhone.15.-.2024-01-12.at.12.18.52.mp4

Widgets and Deep Links regression

  1. Disable feature flag
  2. Add Today/This Week/All Time widgets
  3. Confirm they open Days/Weeks/Insights
  4. Open WordPress.com/stats
  5. Tap on banners
  6. Confirm they open appropriate tab
Simulator.Screen.Recording.-.iPhone.15.-.2024-01-12.at.12.21.27.mp4

Regression Notes

  1. Potential unintended areas of impact

Breaking current Stats tab switching functionality

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

Manual testing

  1. What automated tests I added (or what prevented me from doing so)

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.

UI Changes testing checklist:

  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • VoiceOver.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • iPhone and iPad.
  • Multi-tasking: Split view and Slide over. (iPad)

@staskus
Add Traffic StatsPeriodType to SiteStatsDashboard
e2c8138
@staskus
Display specific Stats tabs depending on Stats Traffic feature flag
d15b045 
- Replace allPeriod with displayedPeriods within StatsPeriodType that depends on a feature flag
- To support different Stats tabs configurations:
-- Replace usage of StatsPeriodType.rawValue to usage of StatsPeriodType. displayedPeriods.firstIndex
-- Replace usage of StatsPeriodType(rawValue:..) initializer with StatsPeriodType.displayedPeriods[]
-- Assume first displayedPeriod is a default tab
@staskus
Update displayedPeriods only when whole Stats view updates
b64b3b3 
Avoid issues when feature flag updates when we have Stats already opened.
@staskus staskus added this to the 24.1 milestone Jan 12, 2024
@staskus staskus requested a review from guarani January 12, 2024 12:25
@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jan 12, 2024 •
edited
Loading

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr22381-115ca18
Version24.0
Bundle IDorg.wordpress.alpha
Commit115ca18
App Center BuildWPiOS - One-Offs #8438
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

@wpmobilebot
Copy link
Contributor

wpmobilebot commented Jan 12, 2024 •
edited
Loading

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr22381-115ca18
Version24.0
Bundle IDcom.jetpack.alpha
Commit115ca18
App Center Buildjetpack-installable-builds #7461
 Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

guarani
guarani approved these changes Jan 12, 2024
View reviewed changes
Copy link
Contributor

@guarani guarani left a comment

Choose a reason for hiding this comment

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

Nice! All the cases work perfectly.

WordPress/Classes/ViewRelated/Stats/SiteStatsDashboardViewController.swift Outdated
@@ -46,6 +56,7 @@ fileprivate extension StatsPeriodType {
case .weeks: return .statsPeriodWeeksAccessed
case .months: return .statsPeriodMonthsAccessed
case .years: return .statsPeriodYearsAccessed
case .traffic: return .statsPeriodDaysAccessed // TODO
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: This could return .noStat

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 didn't know about this case, thanks!

@staskus staskus enabled auto-merge January 12, 2024 18:05
@staskus staskus merged commit a9a56ef into trunk Jan 12, 2024
21 checks passed
@staskus staskus deleted the task/22380-stats-traffic-create-an-empty-traffic-tab-behind-the-feature-flag branch January 12, 2024 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guarani guarani guarani approved these changes

Assignees
No one assigned
Labels
Stats [Type] Task
Projects
None yet
Milestone
24.1
Development

Successfully merging this pull request may close these issues.

Stats Traffic: Create an empty Traffic tab behind the feature flag
3 participants
@staskus @wpmobilebot @guarani