Repository navigation
Premium Analytics: Show report load errors in a notice - #53141
Conversation
Replace the report error state with the notice the detail pages use, so the message spans the report instead of sitting at the card's left edge. Report records now pass their error through, so access denied drops Retry there too. Access denied is an error notice on every page, matching the widgets.
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Premium Analytics plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage Summary2 files are newly checked for coverage.
|
…lt intent Update six report page tests still asserting the removed 'Unable to load' titles, add a PageNotice test for its assertive default, drop redundant mock assertions, state the intent rule instead of its history, and split the changelog entry into two sentences.
louwie17
left a comment
There was a problem hiding this comment.
Thanks @chihsuan, this tested well and code looks good! Tested on a local Docker site, blocking the Regions and Tags requests shows the new notice across the report with Retry, Retry loads the table again once unblocked, and a 403 on Regions shows the error notice without Retry. Nice to see the two error components folded into one PageNotice too.
LGTM 🚀
Fixes UNI-846
Proposed changes
The report message now names the report ("We couldn't load locations. Please try again in a moment.") instead of a heading over a generic line. The detail page notice moved into the widgets toolkit as one shared notice, and the old report error state is removed.
Related product discussion/links
speak()before@wordpress/uidropsspokenMessage, as asked on Premium Analytics: Show detail page errors in a notice #53086.Does this pull request change what data or activity we track or use?
No.
Testing instructions
Setup
jp build plugins/jetpack --deps.Load failures
location-views/regionand reload. Confirm a red notice spans the report and reads "We couldn't load locations. Please try again in a moment." with a Retry button.stats/tags). Confirm the same notice, naming tags and categories.Access denied
fake_detail_err=403. Confirm the same red notice with no Retry.Screenshots
The Regions report with its request blocked.