-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Hide the "View" button when previewing a theme #50986
Hide the "View" button when previewing a theme #50986
Conversation
<PreviewOptions | ||
deviceType={ deviceType } | ||
setDeviceType={ setPreviewDeviceType } | ||
/* translators: button label text should, if possible, be under 16 characters. */ | ||
viewLabel={ __( 'View' ) } | ||
> |
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.
These preview options work on within the editor, so I think we should leave them as they are useful here.
> | ||
<MenuGroup> | ||
<MenuItem | ||
href={ homeUrl } |
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 think adding the isPreviewingTheme()
check here and appending the ?theme_preview
URL query would work well to resolve #50712. I think #50713 can be addressed later as a separate PR. The View and Previewing the site links are legitimately useful as is, even if the preview link isn't (yet) appended to all links on the site when previewing.
I think the first comment by @jeryj is accurate, the mobile and tablet options work well no reason to hide those as well. I think though that hiding the menu group that opens a new tab is good untill we figure out a way to persist theme_preview while browsing the live website. |
I don't see how this could ever be possible |
Can we check:
Another way would be to add a filter for all content and block content if the theme preview param is there. Then append the GET param to all links that match the site url? |
I found a way here: #51412 |
I'm closing this PR since #51412 will come as a solution soon, and the interim solution (this PR) is unnecessary. Thank you for all your reviews. |
What?
Hide "View" when previewing a block theme.
Close: #50919
Why?
While we can show a preview on the front of the site with
?theme_preview
URL query, I thought we better fix these issues (#50713, #50712) before adding the URL query to the link.So, I'm proposing to hide it until previewing the front of the site works well.
How?
isPreviewing()
and show/hide "View"Testing Instructions
Screenshots or screencast
When previewing;
When not previewing;