-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Stats Traffic date picker #22400
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 date picker #22400
Conversation
|
| App Name | WordPress Alpha |
|
| Configuration | Release-Alpha | |
| Build Number | pr22400-18b7d4b | |
| Version | 24.3 | |
| Bundle ID | org.wordpress.alpha | |
| Commit | 18b7d4b | |
| App Center Build | WPiOS - One-Offs #8997 |
|
| App Name | Jetpack Alpha |
|
| Configuration | Release-Alpha | |
| Build Number | pr22400-18b7d4b | |
| Version | 24.3 | |
| Bundle ID | com.jetpack.alpha | |
| Commit | 18b7d4b | |
| App Center Build | jetpack-installable-builds #8035 |
|
👋 @staskus, a heads-up: the date picker is working functionally, but visually isn't quite there yet. It also needs localization, testing, etc. |
829b97e to
4719ef5
Compare
staskus
left a comment
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.
Shorten the date format, e.g. "Jan 14, 2024 - Jan 20, 2024" is too long
- Agree. I think we should follow what is shown on designs IqhXWz3Iir7RMb5XH5gGfZ-fi-97_5069:
- I also noticed that By Month, and By Year uses the date range instead of a single date, that could be fixed as well and would allow dates to fit:
- Nit-pick. I think the space between chevron and date text could be a bit larger (
Length.Padding.single) according to designs
WordPress/Classes/ViewRelated/Stats/Traffic/StatsTrafficDatePickerViewModel.swift
Outdated
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Traffic/StatsTrafficDatePickerViewModel.swift
Outdated
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Traffic/StatsTrafficDatePickerViewModel.swift
Outdated
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Traffic/StatsTrafficDatePickerViewModel.swift
Outdated
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Traffic/StatsTrafficDatePickerViewModel.swift
Outdated
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Traffic/StatsTrafficDatePickerViewModel.swift
Outdated
Show resolved
Hide resolved
|
👋 Thanks for the review comments, @staskus! I should have considered re-using some of the existing logic, so I'll see what can be done there. |
|
I'm going to bump this to the next release because we'll be code freezing 24.3 today and it seems like this has not been approved yet. It doesn't seem the case, but if it's very important that this makes it into this release, let me know and we'll organize a new beta once ready. |
|
👋 Hey @staskus, this should be working now. Thanks for all the great feedback; I think I've incorporated it all in. I added analytics, fixed the design and date formats, connected the date picker to the site's date, and refactored to use the existing date methods. I haven't gone back and re-tested it all yet. If you have time, I'd appreciate your testing of this, but if not, I'll come back here and re-test everything tomorrow. |
staskus
left a comment
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, looks and works well! Only left a couple of comments.
Day selection
- Go back and forth between different days
- You shouldn't be able to go to future dates
Week selection
Switch to "By week"
- Go back and forth between different calendar weeks (Monday to Sunday, inclusive)
- The current week may include future days, but it should prevent you from going to the next week after that
Month selection
- Switch to "By month"
- Go back and forth between different calendar months (e.g. Jan 1-31st)
- The current month may include future days, but it should prevent you from going to the next month after that
Year selection
- Switch to "By year"
- Go back and forth between different calendar years (e.g. Jan 1 to Dec 31, 2024)
The current year may include future days, but it should prevent you from going to the next year after that
Switching periods
- Switch to "By week"
- Go to a given week
- Switch to "By day"
- The date the picker lands on should be the date you were in beforehand
Other tests
- Analytics
- RTL Language
- Dark mode
- Larger fonts
- Different orientations
- Voiceover
WordPress/Classes/ViewRelated/Stats/Period Stats/SiteStatsPeriodTableViewController.swift
Outdated
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/SiteStatsDashboardViewController.swift
Outdated
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Traffic/StatsTrafficDatePickerView.swift
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Traffic/StatsTrafficDatePickerView.swift
Outdated
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Traffic/StatsTrafficDatePickerView.swift
Outdated
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Period Stats/SiteStatsPeriodTableViewController.swift
Show resolved
Hide resolved
WordPress/Classes/ViewRelated/Stats/Traffic/StatsTrafficDatePickerView.swift
Show resolved
Hide resolved
Co-authored-by: Povilas Staskus <[email protected]>
Co-authored-by: Povilas Staskus <[email protected]>
|
I found a corner-case issue that combines these two requirements:
|
|
Thanks for catching that. I've made a separate issue for it: https://github.com/wordpress-mobile/jetpack-issue-repo/issues/19 Enabling auto-merge now |


Fixes #22382
Add the Stats Traffic date picker.
To test
Enable the feature flag: swipe right on any screen (h/t to @staskus for telling me about this), tap Feature Flags, enable
Stats Traffic Tab. Then open the Stats Traffic tab.Day selection
Week selection
Month selection
Year selection
Switching periods
Regression Notes
The changes here are mostly isolated to the Traffic tab.
Not applicable.
I manually tested the new date picker and added unit tests.
PR submission checklist:
RELEASE-NOTES.txtif necessary.UI Changes testing checklist: