-
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
Combine "preview" and "publish" Tips copy #16131
Conversation
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.
The snapshot test in the post-preview-button
component fails. We need to update the snapshots
This command updates this snapshot:
npm run test-unit -- --updateSnapshot --testPathPattern /packages/editor/src/components/post-preview-button/test
Updated the tests, thanks @Soean |
I can't seem to update the post-publish-button tests? I get a bunch of errors when I try to run |
Okay yeah actually removing that fourth tip broke it, oops. @talldan any idea how I'd remove a tip? |
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.
Copy-wise: looks good to me!
Position-wise: it feels like it's floating in midair (both the blue dot, and the tip itself). I wouldn't go bananas trying to get it to a perfect position, but maybe if we could at least centre the dot between the two buttons in question?
@melchoyce: 71aa3a0 removes the fourth tip and c51ce65 positions the third tip in the middle of the two buttons. I didn't spend any time making the position pixel perfect. It's difficult to have tips appear exactly where you want them for all languages and viewport sizes, but if you'd like to fiddle with it, you could try giving the new wrapper |
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 just tested this. While its not lined up to pixel perfection, I don't think that's a big deal.
The only suggestion I have is the tip says,
"...use the “Preview” button to see how it will look to readers."
That was a bit awkward for me. Can we say,
"...use the "Preview" button to see how it will look for others."
The use of "others" here is a bit vague. Would it makes sense to say:
Or just:
|
I proposed on #9256, but adding here as well.
|
I really like this copy, since it short and sweet. Maybe remove ‘your work from the third sentence to make it even shorter, and make that one ‘When you’re ready to publish or schedule, use the “Publish” button.’ |
Sounds good!
|
c51ce65
to
f05b9a1
Compare
I rebased this and updated the copy to be what @0aveRyan suggested above. |
While there are tip changes coming that will deprecate this current rendition, this looks fine. I don't have a problem merging it until the new improved tips are implemented. |
Let's close this out as we're intending to get rid of tips altogether, per #16315. |
...To account for the preview button being disabled.
See #8061.