-
Notifications
You must be signed in to change notification settings - Fork 2k
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: Release new date filtering to production #97060
Conversation
Jetpack Cloud live (direct link)
Automattic for Agencies live (direct link)
|
Here is how your PR affects size of JS and CSS bundles shipped to the user's browser: Sections (~43 bytes added 📈 [gzipped])
Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to. Legend What is parsed and gzip size?Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory. Generated by performance advisor bot at iscalypsofastyet.com. |
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
In the 'Yesterday' view, I'm unable to click on the Visitors/ Likes/ Comments tabs, see video: Screen.Recording.2024-12-04.at.09.49.40.mov |
@grbicsanja good catch! this is expected atm, because we don't have single day data to show for the chart. We removed the hovering effect if that's the case, which @ederrengifo suggested. |
Thanks @kangzj! Can we instead use the empty state that we usually show for other use cases? Is there a specific reason this view wouldn't work here? |
I initially suggested to communicate that the chart is unable within the tooltip, but it seems an empty state could do a be better job since it's a pattern we already use when there is no data to show. We would need to adjust the message though @kangzj cc @grbicsanja |
Here's the adjusted message: No hourly data available This will be the second use case. Visitors, comments, and likes were recorded that day, but hourly data is unavailable. The first use case is an actual empty state where no data was recorded. cc: @kangzj @ederrengifo |
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.
I found a couple of issues:
- When I first load a site, I see lines go beyond the container. It fixes after refreshing the page. Could be something local?
- I notice we are still showing the visitors filter when selecting Today but visitors are not available in the graph. Should we just hide it for that date range?
62150a7
to
2e4515b
Compare
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.
Let's do this 🕺
Thanks for the final design review @ederrengifo and the improvements pointed out 👍
Addressed
This is due to some interim changes detecting whether a site is internal or not, and will not happen on production. |
Related to https://github.com/Automattic/red-team/issues/274
Proposed Changes
stats/new-date-filtering
for all environments.Why are these changes being made?
Testing Instructions
Pre-merge Checklist