fix(content-loop): keep editor preview links from navigating away - #1016
Open
wil-gerken wants to merge 8 commits into
Open
fix(content-loop): keep editor preview links from navigating away#1016wil-gerken wants to merge 8 commits into
wil-gerken wants to merge 8 commits into
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates newspack-blocks editor previews for the Content Loop (Homepage Articles) and Content Carousel blocks so preview links keep their real destinations while preventing unmodified clicks from navigating the block editor canvas away from the post being edited.
Changes:
- Introduces a capture-phase click handler (
preventPreviewNavigation) and attaches it to preview wrappers to cancel unmodified link navigation. - Restores “real”
hrefvalues in editor preview markup (titles, thumbnails, tag labels, sponsors, bylines/avatars), matching front-end output more closely. - Updates PHPUnit/Jest coverage to reflect the new contract: editor payloads carry live URLs; navigation prevention happens at the preview container.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/newspack-blocks/tests/test-homepage-posts-block.php | Updates REST payload contract test to assert live author-archive URLs are preserved. |
| plugins/newspack-blocks/src/shared/js/utils.test.js | Updates sponsor markup tests to expect real sponsor URLs and adds “no URL => no anchor” assertions. |
| plugins/newspack-blocks/src/shared/js/utils.js | Updates sponsor formatter output to render real links when a sponsor URL exists. |
| plugins/newspack-blocks/src/shared/js/inert-preview.test.js | Adds Jest coverage for the click-cancel predicate (anchors vs non-anchors, modified vs unmodified). |
| plugins/newspack-blocks/src/shared/js/inert-preview.js | Adds shared capture handler to prevent editor-canvas navigation from preview links. |
| plugins/newspack-blocks/src/blocks/homepage-articles/edit.tsx | Restores real href usage in preview markup and attaches onClickCapture handler to wrapper. |
| plugins/newspack-blocks/src/blocks/carousel/edit.js | Restores real href usage in preview markup and attaches onClickCapture handler to wrapper. |
| plugins/newspack-blocks/includes/class-newspack-blocks-api.php | Stops rewriting byline/avatar links to # and makes primary category link real when available. |
Suppressed comments (1)
plugins/newspack-blocks/src/shared/js/utils.js:38
- Same as
formatSponsorLogos:sponsor.sponsor_urlis used verbatim as anhref, but the underlying meta is not URL-sanitized. Please ensure only safe URL schemes (and/or relative URLs) are allowed before rendering a clickable anchor in the editor.
<span className="author" key={ sponsor.id }>
{ sponsor.sponsor_url ? <a href={ sponsor.sponsor_url }>{ sponsor.sponsor_name }</a> : sponsor.sponsor_name }
</span>,
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All Submissions:
Changes proposed in this Pull Request:
The problem
Some links in Content Loop and Content Carousel previews could still navigate the editor away from the post being edited:
This could leave the front-end page loaded inside the editing canvas, with the admin bar across the top. Clicking Edit there opens a second editor inside the first.
The fix
Previous fixes disabled known links one group at a time, and in two different ways: a placeholder
href="#"written into the block's own markup, and separately a rewrite of the REST payload the preview renders (#992). This change handles navigation in one place instead: a capture-phase click handler on each block's preview wrapper stops unmodified link clicks from navigating the editor, regardless of where the link came from.That now covers:
Because navigation is handled at the container, those links can keep their real destinations instead of using
href="#". That restores two useful behaviors:.swiper-wrapper { pointer-events: none }already stops mouse clicks from reaching links inside slides. The handler covers keyboard activation there.One browser-controlled path remains: right-clicking a link and choosing Open Link can still navigate the editor, because that action does not fire a page click event.
Links with no destination now render unlinked, matching the front end.
Newspack_Blocks::get_post_link()returnsfalsefor a post type that is not public and has no external URL, andsponsor_urlis empty whenever a sponsor has no link set.Ref NPPM-3241. Follows NPPM-3165 (#992).
How to test the changes in this Pull Request:
Set up the fixtures on
main:Reproduce the problem on
main. Skip this half if you only have the branch checked out; verification starts at step 8.Verify the fix on this branch:
#.Other information:
typescript:check, PHPCS, and ESLint all pass.Technical details — implementation, prior art, and deferred coverage
Why the wrapper, not each link
The preview renders anchors this block does not author:
RawHTMLfor byline, avatars, categories, excerpt, full content, and the article meta footer, plus whatever thenewspack_blocks_post_bylineandnewspack_blocks_categoriesfilters inject. Handlers attached to the anchors this block builds cannot reach those, and neither could the per-field rewriting. A site-specific plugin that hooksnewspack_blocks_categoriesand appends an anchor with a live URL is enough to defeat it, sinceneutralize_editor_links()ran on two payload fields and not that one.WordPress core takes the same approach in its Latest Posts block: keep a real
href, cancel the click. Core attaches the handler to each anchor, which works because its preview is entirely JSX. Ours also renders server-built and filter-injected markup, so the handler goes one level up.Removed
Newspack_Blocks::neutralize_editor_links()and both call sites, and ten hardcodedhref="#"anchors acrossedit.tsx,carousel/edit.js,shared/js/utils.js, and the primary-category builder inclass-newspack-blocks-api.php. Thejsx-a11y/anchor-is-validdisable came out of the three JavaScript files, since no placeholder anchors remain..swiper-wrapper { pointer-events: none }is deliberately untouched: it quiets Swiper drag, not links.Modifier guard
The handler returns early on
metaKey,ctrlKey,shiftKey, andaltKey, the same check link-intercepting routers use. One gap: the Super key on Windows and Linux setsmetaKeywhile carrying no link behavior of its own, so in principle that chord navigates. In practice the operating system claims it before the click reaches the page, and React Router'sisModifiedEventcarries the same edge case.Front-end output is unchanged
newspack_blocks_format_byline(),newspack_blocks_format_avatars(),templates/article.php, andcarousel/view.phpare untouched.newspack_blocks_get_primary_category()runs only inposts_endpoint(); the front end builds its category markup throughnewspack_blocks_format_categories().Deferred end-to-end coverage
Three payload tests asserting
href="#"are gone. One test replaces them, asserting the payload carries the live author-archive URL, the contract the editor now relies on.An end-to-end spec covering the canvas is written and verified: it passes against this fix and fails against a build with the handler removed. It is not in this PR. The nightly suite runs specs from
mainagainst a site on the stable release channel, so a spec expecting this fix would fail there every night until the fix reaches stable. The spec ships in a follow-up PR once the release carrying this change reaches that site.Until then, one claim is unpinned: nothing in the shipping suite proves the handler is attached to the wrapper. Removing
onClickCaptureleaves every Jest and PHPUnit test green.🤖 Generated with Claude Code