Framework: Add notices #1437
Framework: Add notices #1437
Conversation
youknowriad
added
Design
Framework
|
Visually looks good! What happens if you close the sidebar? |
|
What about rendering these next to the "Saved" status in the header? |
|
@jasmussen If you close the sidebar, they move to the right |
I missed the "post saved" in the screenshot. I don't think we need that, as it's implied by the save status that Matías refers to. But as discussed in #967, we do have a few edgecases where it'd be good to have notices:
For these, the save area on the left doesn't scale well enough to mobile, hence the need for notices. |
|
@jasmussen I mean rendering the notice next to the label instead of next to the sidebar. |
|
Ah rendering it on the left side of the screen, below? That's cool with me too 👍 |
|
Below, or even on the header itself.
|
|
That's the thing, it has to scale to mobile. I'd rather we find a more permanent place for it. |
|
I think top left is a good start, then. |
|
@mtias top left on top of the other UI (fixed position) or not? |
|
The same top position as it is now, but to the left of the editor instead of the right. |
| } else if ( visibility === 'private' ) { | ||
| publishStatus = 'private'; | ||
|
|
||
| componentDidUpdate( prevProps ) { |
There was a problem hiding this comment.
Mmm, got confused a bit by this. Why do we need publish-button to be handling all this? It sounds like just a side-effect of core editor actions, no matter who triggers the actions.
There was a problem hiding this comment.
This is a good question.
It sounds like just a side-effect of core editor actions, no matter who triggers the actions.
Maybe, it would make things easier to write for sure. If we see these actions like actions that could be triggered from different places (imagine other WP pages, for example a settings triggering bulk post update or something), this could cause bugs where notices are shown while we don't expect them or we want to make their content specific to the context.
We experienced this kind of issues with the site settings actions in Calypso.
There was a problem hiding this comment.
Any thoughts on this?
There was a problem hiding this comment.
I'm having a hard time wrapping my head around the question here. Is it mostly a question for Matías or is there a design question in here?
There was a problem hiding this comment.
I think it's more a technical question here
|
I think we should remove the "Saved" notice. We should have one on Publish, but it should say "Post published!" Same with updating and scheduling. |
|
I'm going to drop all the notices from the current PR. That way, we can merge this PR with only the "framework" part and add the publish/update notices in a follow-up PR |
|
Ok! Going to merge this soon. It does nothing right now aside preparing us to trigger notices. |
|
|
||
| return ( | ||
| <div className="components-notice-list"> | ||
| { notices.reverse().map( ( notice ) => ( |
There was a problem hiding this comment.
@youknowriad: FYI, Array#reverse reverses in place. I think this has worked so far because getNotices returns a new instance every time via lodash#values and the prop isn't used elsewhere, but be aware. :)
There was a problem hiding this comment.
Oh, good catch @mcsf Thanks, I'll fix that 👍
Sync RichText with web counterparts
refs #967
This PR adds the notices "framework", I mean it answers these questions:
For this, I've implemented a "Post save success" and "Saving failed" notices, but don't take too much attention to the business logic. Showing more specific "Post published", "Post scheduled"... notices is left for a follow-up PR.
This probably needs some design/responsiveness polish. Feel free to polish :)