-
Notifications
You must be signed in to change notification settings - Fork 3k
Site Editor: Add get parameter to hide admin bar in classic theme site preview #8475
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: Add get parameter to hide admin bar in classic theme site preview #8475
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN:
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
I'm wondering whether the JS code that disables interactive elements should also be moved to this PR and output as an inline JS. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
I think it’s a good idea because otherwise the elements can be interactive for a moment before the |
@stokesman Thanks for the feedback! I'm not familiar with the approach using By the way, I came up with a different approach that doesn't implement any custom JS code and simply wraps the This approach is used in various places, including the pattern preview. |
This approach didn't work 😅 Because the |
296a180
to
23e11ce
Compare
Perhaps this can be addressed in a follow-up? |
The inline script would be something like this: add_action( 'wp_print_footer_scripts', function() {
echo '<script>', <<<JS
if ( window.parent ) {
window.parent.postMessage( 'preview dom available' );
}
JS, '</script>';
} ); The only important part of that is that it appears after any other elements we’ll want to manipulate through DOM methods. Then the site-editor would listen for the message with something like this: const messageEffect = useRefEffect(
( node ) => {
const onMessage = ( event ) => {
if (
event.origin === siteUrl &&
event.data === 'preview dom available'
) {
// Make interactive elements unclickable…
}
};
const { defaultView } = node.ownerDocument;
defaultView.addEventListener( 'message', onMessage );
return () =>
defaultView.removeEventListener( 'message', onMessage );
},
[ siteUrl ]
);
// attaching the ref callback to the iframe would follow… That has to be done through the ref callback since it seems React doesn’t do |
@stokesman We found that some plugins load code into the preview. To solve this problem, it seems best to define |
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.
Thanks @t-hamano! One suggestion, but this approach looks great if it solves the problems you're seeing with the previews.
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.
To prevent this being used in an unintended manner, I'd suggest
current_user_can()
for theme editing caps, or whatever cap is most appropriate for being able to view the storybook- Using a nonce for the value of the
wp_site_preview
query string parameter.
The capability check makes sense, but I don't know if nonce is necessary. Unlike theme previews, this flag just renders the site front-end like iframe (which is already accessible when public); it doesn't override anything else. |
I would be happy if this PR is committed before Beta 3. I think the following two suggestions could be considered in a follow-up:
|
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'm happy with this. Thanks, @t-hamano!
Fair point, that works for me. |
Trac ticket: https://core.trac.wordpress.org/ticket/63076
This PR hides the admin bar if the get parameterThis PR setswp_site_preview=1
is present.IFRAME_REQUEST
constant if the get parameterwp_site_preview=1
is present.This PR is needed to hide the admin bar from the classic theme site preview in the site editor.
This PR cannot yet be tested in the site editor, but can be tested on the frontend.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.