Repository navigation
Premium Analytics: link Most viewed post titles through the router - #50891
Conversation
The Most viewed widget linked post titles with a plain href. The href carried the SPA path in the p query param. A click reloaded wp-admin and restarted the whole dashboard. Add PostTitleLink to the widgets toolkit. A row with a post ID gets a router link to the post detail page. A row without one falls back to the public URL and takes the external icon. Other rows stay plain text. An internal link carries no icon, because the icon marks a destination outside the app. Use the component in the Most viewed widget and in the Posts and pages report. The report link dropped the report window before this change, so the detail page reseeded default dates. Add a View post link to the post detail header. The lists that link there now carry no outbound link, so this link is the only path to the live post. Delete usePostDetailHrefBuilder. The widget was its only caller.
|
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! |
Code Coverage SummaryCoverage changed in 4 files.
2 files are newly checked for coverage.
|
The View post link sat inside the summary card. Move it to the Page actions slot, the same header position that reports use for Download and the dashboard uses for Customize. Match the Download button treatment: solid variant, neutral tone, compact size, and no icon. The button renders as an anchor, so it keeps a real href to the live post. wp-admin colours bare anchors outside any cascade layer, and the @wordpress/ui button styles live inside layer wp-ui. Unlayered rules beat layered rules whatever the specificity, so the admin link colour overrode the button foreground. Restore the button colour from the page stylesheet.
The coverage bot flagged the post summary hook: the new public-URL selector added six uncovered lines to a file at zero coverage. The page stage had no test at all, so the header action was uncovered too. Add tests for usePostSummary and for the page stage, following the video detail stage test. The hook reaches full coverage, and the stage reaches 83 percent of statements. The stage test found two defects in the header action. Base UI logged a console error, because a button rendered as an anchor needs nativeButton set to false. With that set, Base UI then marked the anchor with a button role, so a screen reader announced a button for an element that opens a new tab. Set an explicit link role to match the behaviour.
chihsuan
left a comment
There was a problem hiding this comment.
Thanks for the thorough fix! @Nikschavan
I left three inline comments about regressions or edge cases worth addressing:
- Preserve ellipsis behavior for drill-down rows with long labels.
- Keep a fallback path to the live post when core-data cannot resolve a post type, particularly CPTs with
show_in_rest: false. - Add report-specific layout styles for external-link icons.
Once those are resolved, this should be good to go.
| href={ href } | ||
| variant="unstyled" | ||
| openInNewTab | ||
| rel="noopener noreferrer" |
There was a problem hiding this comment.
BTW just a nit but you don't need to add these in modern browsers anymore. Link and ExternalLink components don't add them anymore either in Gutenberg.
| // The header action is a Button rendered as an anchor, so it can carry a real | ||
| // href to the live post. @wordpress/ui styles the button inside `@layer wp-ui`, | ||
| // and wp-admin colours bare anchors outside any cascade layer — unlayered wins | ||
| // over layered whatever the specificity, so admin's link blue overrode the | ||
| // button foreground. Re-apply the button's own colour from here (unlayered). | ||
| .viewPost:any-link { | ||
| color: var(--wp-ui-button-foreground-color); | ||
| } |
There was a problem hiding this comment.
Would this be fixed by latest WP dependencies update or is this some kind of bug we should look into at UI library? It looks like an odd fix to need to do here.
What it kinda sounds is happening that you have Button with href, and you can now replace that with the new LinkButton component, and solve odd bugs like these.
There was a problem hiding this comment.
Right, this is an anchor inside a Button, so LinkButton will be the right fit. After the dependency bump in #50509, I will do a follow-up PR to remove this override and use the LinkButton, thank you!
Fixes WOOA7S-1826
Proposed changes
hrefthat encoded the SPA path in thepquery param. A click reloaded wp-admin and restarted the whole dashboard. Every other surface in the package navigates through the router.PostTitleLinktoolkit component, modelled on the existingVideoTitleLink. A row with a post ID gets a router link to/post/{id}. A row without one falls back to the public URL and takes the external-link icon. Anything else stays plain text.search, so opening a post detail page reset the selected date range to defaults. The shared component carries the range.href, withnativeButton={ false }androle="link"so it does not announce as a button.usePostDetailHrefBuilder. It existed to avoid router context, but the widget already renders a router link throughReportLink, so that property was gone before this change.Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions