Repository navigation
Premium Analytics: Show detail page errors in a notice - #53086
Conversation
Use the @wordpress/ui Notice for the author, post and video detail pages' error and not-found states, and decide Retry through describeError on all three, so access denied no longer offers a Retry that cannot help.
|
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. 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. |
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Code Coverage SummaryCoverage changed in 2 files.
2 files are newly checked for coverage.
|
The post summary passed the raw query error through while gating isError on a missing post, so a failed background refetch left an error beside a loaded post. Also inline the single-use intent alias and note that the notice link must be childless.
Nikschavan
left a comment
There was a problem hiding this comment.
Thank you, the changes look good. I left two questions inline, one about Notice announcements upstream and one about intent in the widget SDK.
What I checked: I opened a video details page on a local site with this branch built, using an image attachment ID for the not-found state. I then made the single-video request fail in the browser, first with a 500 and then with a 403. The 500 shows a red notice with Retry, and the screen reader announces it assertively without the Retry label. Retry recovers once the request succeeds. The 403 shows an info notice with no Retry, announced politely. The layout holds at 1280 and at 390 wide, and the console stays clean.
| Not found | Load failure | Access denied |
|---|---|---|
![]() |
![]() |
![]() |
| export function DetailPageNotice( { intent, description, actions, link }: DetailPageNoticeProps ) { | ||
| return ( | ||
| // The default announcement (children) would trail the action labels. | ||
| <Notice.Root intent={ intent } spokenMessage={ description }> |
There was a problem hiding this comment.
Notice no longer announces anything on Gutenberg trunk: #82737 removed spokenMessage and politeness, and the announcement guidelines now ask the app to call speak() and to pick the politeness by urgency rather than by intent. It is still unreleased, so this works today, but the next @wordpress/ui bump drops the announcement these three pages now depend on. Calling speak( description, … ) from an effect here would keep the same live regions and the tests as they are. Would it make sense to switch now, since this is the first new caller of the prop?
|
|
||
| export interface DescribedError extends WidgetStateError { | ||
| /** `error` when something failed; `info` when the request answered and the answer is a fact, such as no access. */ | ||
| intent: 'error' | 'info'; |
There was a problem hiding this comment.
describeError is re-exported from the widget SDK (sdk/src/index.ts#L20), so intent becomes part of the widget contract, while the only reader is DetailPageNotice and WIDGET_API_VERSION stays at 1.2.0. Is the plan for SDK widgets to use intent too, or could it stay on the detail-page side?
There was a problem hiding this comment.
Good point, no plan for widgets to use it. Moved it to a detail-page helper in c9e1b33.



Fixes WOOA7S-2180
Proposed changes
The shared error mapping the cards use now also names the notice type, so the detail pages and the cards decide Retry the same way.
Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
Setup
jp build plugins/jetpack --deps.fake_detail_err.mu-plugin snippet
Load failures
wp/v2/users, then open an author from Top authors. Confirm a red notice reads "We couldn't load this author. Please try again in a moment." with a Retry button.stats/videoon a video opened from the Videos report, and withstats/poston a post opened on All time.Access denied
&fake_detail_err=403to the page URL. Confirm a blue notice reads "You don't have access to this data." with no Retry. On trunk, the same page offers Retry.&fake_detail_err=404. Confirm a blue notice reads "This site doesn't share author profiles." with no Retry.Not found
/author/99999. Confirm a blue notice reads "We couldn't find this author." with Back to Authors./video/<id>with an image's attachment ID. Confirm "We couldn't find this video." with Back to Videos, which opens the Videos report on the same range.Accessibility
Before / after
Video details with the video request failing. Only the message below the header changes; the header and date button do not.