fix: update subtitle block for compatibility with iframed Post Editor#425
fix: update subtitle block for compatibility with iframed Post Editor#425jason10lee wants to merge 9 commits intotrunkfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the newspack-block-theme/article-subtitle block’s editor-side behavior so the subtitle field continues to appear and function when the Post Editor canvas is rendered inside an iframe (Gutenberg iframe mode).
Changes:
- Switch editor asset loading from
enqueue_block_editor_assetstoenqueue_block_assetswith awp_should_load_block_editor_scripts_and_styles()guard. - Update the subtitle block’s editor implementations (site editor + post editor), including iframe-aware DOM targeting in the post editor injection.
- Add
apiVersion: 3/$schemametadata to the block definition.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| includes/class-core.php | Loads theme editor JS via enqueue_block_assets (guarded) to better support iframe editor contexts. |
| includes/blocks/subtitle-block/site-editor.js | Wraps subtitle output with useBlockProps() and sets apiVersion: 3 in registration data. |
| includes/blocks/subtitle-block/post-editor.js | Adds iframe-aware document selection + retry loop to inject the editable subtitle beneath the title in iframe mode. |
| includes/blocks/subtitle-block/class-subtitle-block.php | Moves subtitle block editor asset enqueueing to enqueue_block_assets and updates site-editor script deps. |
| includes/blocks/subtitle-block/block.json | Adds block.json schema reference and sets apiVersion to 3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…gainst naughty subtitles
adekbadek
left a comment
There was a problem hiding this comment.
Works as described, left some suggestions.
As suggested by @adekbadek. Co-authored-by: Adam Cassis <adam.cassis@automattic.com>
rbcorrales
left a comment
There was a problem hiding this comment.
LGTM. Left a nit on the mount effect closure.
| const editorDoc = getEditorCanvas(); | ||
| const titleWrapperEl = editorDoc.querySelector( '.edit-post-visual-editor__post-title-wrapper' ); | ||
| if ( titleWrapperEl ) { | ||
| appendSubtitleToTitleDOMElement( subtitle, editorDoc, saveSubtitle ); |
There was a problem hiding this comment.
nit (but also a possible race condition): the mount effect here captures the initial subtitle in its closure, so if post meta comes back while the retry loop is still looking for the title wrapper, the element ends up with a stale value and the sync effect has already fired and missed it. A ref would keep tryAppend reading the latest value.
You could add this before line 87:
const subtitleRef = useRef( subtitle );
useEffect( () => {
subtitleRef.current = subtitle;
}, [ subtitle ] );Then on line 96:
| appendSubtitleToTitleDOMElement( subtitle, editorDoc, saveSubtitle ); | |
| appendSubtitleToTitleDOMElement( subtitleRef.current, editorDoc, saveSubtitle ); |
All Submissions:
Changes proposed in this Pull Request:
This PR updates the following block for compatibility with the iframed Post Editor:
newspack-block-theme/article-subtitleAddresses NPPM-2588: Newspack Block Theme (iframe Editor Compatibility).
How to test the changes in this Pull Request:
Prerequisites
newspack-block-theme.Part 1: Post Editor — Verify the bug (before the PR)
Make sure
newspack-block-themeis on the trunk branch (the state before this PR)./wp-admin/post.php?post=<POST_ID>&action=edit).<iframe>tag if so. (Unlike other tests, the Post Editor will probably already be in iframe mode due to how the subtitle block is injected.)/?p=<POST_ID>). The subtitle should still render correctly.Part 2: Post Editor — Verify the fix (after the PR)
Other information:
Additional fixes
This PR addresses a few issues beyond the migration that either made testing difficult, or involved code we were changing already. I've been urged by Claude to list them here:
$post_subtitlewas output as raw HTML insprintf(). Now wrapped inesc_html(). Fixed to avoid breaking test site when uploading odd test subtitles. (class-subtitle-block.php:55)innerHTMLused for plain text in post editor — The contenteditable input handler, comparison, and assignment all usedinnerHTMLwheretextContentis appropriate. Now usestextContentthroughout. Belt-and-suspenders fix with the above. (post-editor.js:57, 64-65)style.idcaused duplicate style tags — The<style>element was never assignedSUBTITLE_STYLE_ID, so thegetElementByIddeduplication check always returnednull, injecting a new<style>tag on every call. Cosmetic, but caused confusion. (post-editor.js:38)ifblock didn't prevent execution from continuing to the post-editor check. Now usesif/elseifto make mutual exclusivity explicit. (class-subtitle-block.php:71-79)apiVersion: 3in site-editor.js Set explicitly then immediately overwritten by...metadataspread fromblock.json. Tests as unneeded, and flagged by AI reviewers. (site-editor.js:22)