-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Improve pre-publish panel styling #7879
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.
This looks/reads better. I have a few questions/thoughts about it though.
@@ -44,22 +45,25 @@ | |||
|
|||
.editor-post-publish-panel__link { | |||
color: $blue-medium-700; | |||
font-weight: 400; |
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.
Any chance we have variables for these, or are they hard-coded everywhere? 🤷♂️
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.
Nope! Hard-coded everywhere. There aren't that many font-weights technically available, although an audit could be worth doing at some point during the high-shine polish stage.
|
||
.components-panel__body-toggle { | ||
font-weight: 400; | ||
//font-weight: 400; |
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.
🔥
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.
🙈
@@ -26,7 +26,7 @@ function PostPublishPanelPrepublish( { | |||
return ( | |||
<div className="editor-post-publish-panel__prepublish"> | |||
<div><strong>{ hasPublishAction ? __( 'Are you ready to publish?' ) : __( 'Are you ready to submit for review?' ) }</strong></div> | |||
<p>{ hasPublishAction ? __( 'Here, you can do a last-minute check up of your settings below, before you publish.' ) : __( 'When you\'re ready, submit your work for review, and an Editor will be able to approve it for you.' ) }</p> | |||
<p>{ hasPublishAction ? __( 'Double-check your settings, then use the button to publish your post.' ) : __( 'When you\'re ready, submit your work for review, and an Editor will be able to approve it for you.' ) }</p> |
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 button" is a bit vague. Maybe the "Publish" button
? But that's awkward. Why not:
Double-check your settings, then click "Publish".
Short and to-the-point.
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.
"Click" is too desktop-centric, hence the vague wording—I'd like to stick with "use" for lack of a better term.
Interesting ideas @sarahmonster. I do think having this section look so close to the sidebar for document/block maybe is a step too far. Could we scale back have it reflect the design over be exact? Maybe we have the top section white for example. I think this is better as whilst it is a sidebar it isn't the same type of element. The content is different. |
Making the top section white makes everything very low contrast and makes it hard to single out where the user should be looking. The most important info (eg: what to do next and why this modal appeared) is highlighted by the darker background in the new design. Comparing the two screenshots my eyes immediately know where to look in the one with the dark grey background over the white, where my eyes just dart all over the place. |
For reference, this is what it looks like with the header section in white: For readability and usability, I think the most important thing here is to retain the visual separation between panels. I think the trouble here is that this sidebar pattern looks and functions almost exactly the same way as that other (pretty ubiquitous) sidebar pattern, with a few notable deviations. This makes it feel as though the difference here is accidental, rather than deliberate. There's also an odd visual thing where you end up with a very pronounced crosshair of grid lines in the top right of the screen, which commands focus. If we don't want it to feel like the same element, a better solution might be to consider using a different element here, rather than a well-established pattern with a slight deviation. A drop-down from the interim publish button could work, and it's part of my suggestions for improvements in #7602. |
I actually think the white sidebar works. The problem with a drop down is extending, what is someone puts a check in there, or 10? |
It would scroll, just like it would need to if 10 checkboxes were in there on mobile. Though putting ten checkboxes in there is probably a UX bug 😉 I think the main issue with a dropdown that extends out of the publish button actually is that it's still not very attention-grabbing unless we blur/darken the rest of the editor. Which brings me to...
While it looks visually "clean", I don't think it draws any attention to itself. I already find the pre-publish sidebar modal a bit strange as I expect the post to publish when I click publish–but if it isn't we should be clear about what to focus on. If you want to keep the sidebar white, how about we grey out the editor? |
I still feel for the format we have and patterns it being white makes sense. However as in #7602 other explorations may totally change this area. If we want an update to it right now then white would be the right pattern. |
As mentioned: fine with it being white, but having more visual draw when it's the last step between writing and publishing is better than what we have now, where a user's eyes wander while they wonder why "Publish" didn't publish 😄 All white plus the greyed out editor (as you shouldn't be making editor changes at this point anyway) would be a good idea I think (and probably easy to implement and work well with stuff mentioned in #7879). |
25eb5a1
to
50b84cb
Compare
I just realised this PR went stale, but in reviewing #8235 realised it's super confusing having the panels without any delineating border. In the interests of pushing this through, I've switched the background to white. It'll likely change more as we iterate through the changes in #7602, but for the moment this is an improvement on what we currently have. |
I think originally @jasmussen and @youknowriad shaped how this panel looks and behaves in general. If we want to make it look closer to other sidebars, we should consider refactoring it in a way that avoids maintaining two different solutions for sidebars. |
Just to clarify, since there's been a bit of back-and-forth: this isn't an attempt to make it look more "sidebar-like", we've just adjusted the text strings and added back the separators between panels within the sidebar itself. This isn't intended to be a final solution, but rather a smaller iterative change to make this panel easier to use whilst we work on larger improvements to the overall flow. Longer term, it'll likely be moving to using a different pattern entirely that's even less sidebar-ish than its current incarnation. 😄 For extensive discussion and preliminary mockups, see #7602. |
Cool, thanks for clarifications @sarahmonster 👍 I just wanted to avoid having code duplication which ends up looking the same for the end user. Nothing to worry about in such case and sadly no space to remove some code 😃 |
I regret to inform you I am 100% planning on adding more code before we're all finished here. 😬 |
On this one, designwise, I'll defer to Tammie, I know she has lots of ideas around this one. |
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.
Code here looks good and I think this looks way better.
50b84cb
to
56abd87
Compare
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.
Let's try and get this in as a first step :)
Let's do it. :party-sheep:! |
Thanks for the improvements here! |
Before
This resolves #7878 and resolves #7873, by clarifying the microcopy used in the prompt here, and adjusting the styling to be more consistent with other sidebars in the UI.
After
How has this been tested?
Types of changes
Style & copy tweak.
Checklist: