-
Notifications
You must be signed in to change notification settings - Fork 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
Add the sticky notice when previewing a theme #82629
Conversation
This PR modifies the release build for wpcom-block-editor To test your changes on WordPress.com, run To deploy your changes after merging, see the documentation: PCYsg-l4k-p2 |
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
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.
👍 LGTM and works as expected.
We should use Back to themes
rather than Back to Themes
to be consistent with other screens.
About the copy of the notice
I wonder if we can still make the copy of the notice more helpful for users. Some suggestions:
- Use "previewing" rather than "live-previewing" because the button doesn't mention "Live" and users might not get the meaning of "live" in this context.
- I think users will appreciate to read that changes done while previewing won't affect their site.
- Maybe we can use the word "customize" in some way?
How can we make this copy more helpful? cc: @Automattic/lego
Current: "You are currently live-previewing the Smithland theme."
Suggestion: "You are currently previewing the Smithland theme on your site. You can customize it without affecting your site."
Here is a demo:
This is a bit tricky, because we can add new pages which does affect the site :') Maybe: "You are currently previewing the Smithland theme on your site. You can try customizing the styles without affecting your site." |
Just to add some context, it is related to WordPress/gutenberg#53935. 📝
@fushar @miksansegundo CC: @Automattic/lego |
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
const theme = ( select( 'core' ) as any ).getTheme( currentlyPreviewingTheme() ); | ||
return { | ||
previewingTheme: theme?.name?.rendered || 'new', |
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.
Should || 'new'
be || currentlyPreviewingTheme()
?
@okmttdhr Just confirming, have we tested this on Atomic sites 😄 |
This comment was marked as resolved.
This comment was marked as resolved.
Nice work thinking through copy ideas everyone!!! I did a little adaptation of the suggestions: You are previewing the {theme_name} theme. You can try out your own style customizations, which will only be saved if you activate this theme. |
Yes! Sandboxing |
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.
This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/9245807 Thank you @okmttdhr for including a screenshot in the description! This is really helpful for our translators. |
This reverts commit a6d1837.
Translation for this Pull Request has now been finished. |
Proposed Changes
Related
wp-calypso/apps/editing-toolkit/editing-toolkit-plugin/wpcom-global-styles/notices.js
Line 164 in 2a55443
wpcom-block-editor
.Testing Instructions
Preview & Customize
Pre-merge Checklist