-
Notifications
You must be signed in to change notification settings - Fork 798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Site Editor Compatibility: Podcast Player block issues in editor and front end views #19578
Merged
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
113c09a
[not verified] For TT1 block-based theme at least, we have to wait un…
ramonjd 79333a8
[not verified] # This is a combination of 3 commits.
ramonjd 68eda5a
Using the block element to check whether it's sitting inside an ifram…
ramonjd 269d941
Remove unused method
ramonjd a04c762
Switch from useEffect to ref callback for getting podcast player elem…
48dacca
Making element removal optional.
ramonjd 611b1a9
This updates the iframe check to also look for the block editor ifram…
ramonjd a27e87c
Returning an array from maybeCopyElementsToSiteEditorContext and not …
ramonjd File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
4 changes: 4 additions & 0 deletions
4
projects/plugins/jetpack/changelog/fix-site-editor-podcast-player-block
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
Significance: patch | ||
Type: compat | ||
|
||
Ensure compatibility with the Site Editor by injecting required media assets into the Site Editor canvas, and loading frontend scripts onDomReady |
This file contains 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
This file contains 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
67 changes: 67 additions & 0 deletions
67
projects/plugins/jetpack/extensions/shared/block-editor-asset-loader.js
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
/** | ||
* Returns the current document and window contexts for `elementRef`. | ||
* Use to retrieve the correct context for elements that may be within an iframe. | ||
* | ||
* @param {HTMLElement} elementRef - The element whose context we want to return. | ||
* @returns {Object} - The current document (`currentDoc`) and window (`currentWindow`) contexts. | ||
*/ | ||
export function getLoadContext( elementRef ) { | ||
const currentDoc = elementRef.ownerDocument; | ||
const currentWindow = currentDoc.defaultView || currentDoc.parentWindow; | ||
|
||
return { currentDoc, currentWindow }; | ||
} | ||
|
||
/** | ||
* Returns whether a given element is contained within an iframe. | ||
* Useful to check if a block sits inside the Site Editor. | ||
* | ||
* @param {HTMLElement} elementRef - The element whose context we want to return. | ||
* @returns {boolean} - Whether `elementRef` is contained within an iframe. | ||
*/ | ||
export function isElementInIframe( elementRef ) { | ||
const { currentWindow } = getLoadContext( elementRef ); | ||
return currentWindow.self !== currentWindow.top; | ||
} | ||
|
||
/** | ||
* This function will check if the current element (e.g., a block) sits inside an Iframe (e.g., the Site Editor) | ||
* and tries to move elements from the parent window to the iframe. | ||
* | ||
* It's a temporary work-around to inject the styles we need for the media player into the site editor. | ||
* For use until Gutenberg offers a standardized way of including enqueued/3rd-party assets. | ||
* Target usage is the Podcast Playerblock: projects/plugins/jetpack/extensions/blocks/podcast-player/. | ||
* | ||
* @param {Array} elementSelectors - An array of selectors, e.g., [ '#conan', '#robocop' ] | ||
* @param {HTMLElement} elementRef - The current element. | ||
* @param {boolean} shouldRemoveSource - Optional. Whether to remove the source element in the parent frame. | ||
* @returns {Array} - An array of successfully migrated selectors; | ||
*/ | ||
export function maybeCopyElementsToSiteEditorContext( elementSelectors, elementRef, shouldRemoveSource = false ) { | ||
// Check to see if we're in an iframe, e.g., the Site Editor. | ||
// If not, do nothing. | ||
if ( ! elementRef || ( ! elementSelectors && ! elementSelectors.length ) || ! isElementInIframe( elementRef ) ) { | ||
return; | ||
} | ||
|
||
const { currentDoc, currentWindow } = getLoadContext( elementRef ); | ||
const parentDoc = currentWindow?.parent?.document; | ||
let results = []; | ||
|
||
if ( currentDoc && parentDoc ) { | ||
results = elementSelectors.filter( selector => { | ||
const parentElementToCopy = parentDoc.querySelector( selector ); | ||
const isElementAlreadyPresentInCurrentWindow = !! currentDoc.querySelector( selector ); | ||
if ( parentElementToCopy && ! isElementAlreadyPresentInCurrentWindow ) { | ||
currentDoc.head.appendChild( parentElementToCopy.cloneNode() ); | ||
if ( shouldRemoveSource ) { | ||
parentElementToCopy.remove(); | ||
} | ||
return true; | ||
} | ||
return false; | ||
} ); | ||
|
||
return results; | ||
} | ||
} |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might need to add an additional check either here or within
getLoadContext
that we're in the correct iframe (the site editor) and not within another version of an iframed editor (e.g. the post/page editor iframed withinwordpress.com/post/<blog_name>/<post_id>
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wow, thanks for double checking that. 🤕
I did check in WPCOM when I was initially targeting
'iframe[name="editor-canvas"]'
I'll try to reinstate that test.
THANK YOU
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I re-added an iframe name check. Not ideal, but the whole thing is (theoretically) transitory.
I could try to check in the back end and add a global JS variable if it might help future endeavours.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also test that the iframe passes the same-origin-policy before we try to start injecting things into the header and bork it all again. :D