Add publish flow in site editor #61136
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ntsekouras
added
[Type] Enhancement
ntsekouras
requested review from
draganescu,
Mamaduka,
youknowriad,
scruffian,
jameskoster and
jasmussen
|
Size Change: -481 B (0%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
ntsekouras
force-pushed
the
add/publish-flow-in-site-editor
branch
from
aabdee0 to
5a25cb7
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
ntsekouras
commented
Apr 26, 2024
|
Gave it a quick review in post and site editors, doing all the test items. Here are GIFs of those two cases:
Overall, this worked well. One hiccup of note was scheduling, if I simply choose the "Scheduled" option in the status dropdown, I'm not prompted for a date. So presumably I'm scheduling for now — that was a bit surprising in the flow. This could be something to revisit, but it doesn't seem a product of this PR. I'm also increasingly feeling like maybe that status chip should look like the other items gridded below the excerpt, i.e. it could say "Status" on the left, and then as a blue link, or even the chip if need be, could sit on the right, in the same column as the other meta items. In practice its placement below the excerpt doesn't feel as intentional as I had hoped, and it felt almost non-interactive. The idea is good, but we can probably do small tweaks to move this. |
|
One more thing, the more we unify these two editors, the more I'm finding that it trips up my flow that we are not consistent in the inspector behavior between them. In the post and regular page editors, when you click a block, the inspector shifts to the block tab:
In the site editor, this behavior is the same, until you click the Page tab at which point the page tab becomes "sticky": It's not clear how to make that tab "unsticky" again, or why it became sticky in the first place. Can we revisit this? |
|
good step
|
Mamaduka
reviewed
Apr 26, 2024
Comment on lines
+194
to
+208
|
|
||
| // Local state for save panel. | ||
| // Note 'truthy' callback implies an open panel. | ||
| const [ entitiesSavedStatesCallback, setEntitiesSavedStatesCallback ] = | ||
| useState( false ); | ||
|
|
||
| const closeEntitiesSavedStates = useCallback( | ||
| ( arg ) => { | ||
| if ( typeof entitiesSavedStatesCallback === 'function' ) { | ||
| entitiesSavedStatesCallback( arg ); | ||
| } | ||
| setEntitiesSavedStatesCallback( false ); | ||
| }, | ||
| [ entitiesSavedStatesCallback ] | ||
| ); |
There was a problem hiding this comment.
I think the edit-site package has selectors/actions for this. See #45200. We should probably put this into the editor store and reuse it in both editors.
There was a problem hiding this comment.
Do you think we can do it in a follow up to minimize the risk of introducing any regressions? I'd love to minimize the changes per PR, as there are more needed for every step that gets us closer. 😄
|
Thanks for testing @jasmussen!
If you choose from the status dialog Regarding the other design suggestions, I think they can be discussed separately and be handled in separate PRs.
This is exactly the same in both editors, if we explicitly select the
@Marc-pi this should also be handled separately and would probably require a new issue (if none similar exists). Could you create an issue? |
|
Seems to be working well for me. A couple of observations:
It seems a bit strange to see a specific message combined with a generic one after saving. Curious what others thing, but I'm inclined to suggest the "Site updated" one is probably adequate on its own. |
youknowriad
reviewed
Apr 26, 2024
youknowriad
reviewed
Apr 26, 2024
packages/editor/src/components/post-layout-actions-panel/index.js
Outdated
Show resolved
Hide resolved
@ntsekouras , here you are #47446 |
ntsekouras
force-pushed
the
add/publish-flow-in-site-editor
branch
from
5a25cb7 to
283d870
Compare
I agree with that and it's happening when we save multiple entities. This is also the case for post editor and we might want to look into that separately. |
This is still an issue for me, would appreciate if others could check it 🤔 |
Can you make sure that you have enabled the panel in the preferences? |
It wasn't there before but I see it now and toggling it on works :) Not a big deal, but I wonder if this preference should be synced across post/site editors. I had it enabled in the post editor, but it wasn't enabled in the site editor. This seems a little unexpected. |
youknowriad
reviewed
Apr 30, 2024
| } | ||
| /> | ||
| ) } | ||
| { _isPreviewingTheme && <SaveButton size="compact" /> } |
There was a problem hiding this comment.
Would love a review from @scruffian or @draganescu to test the theme activation flows
There was a problem hiding this comment.
For me it tested all right!
Edit: I did find the activate button disappears when browsing a previewing theme. Testing more.
There was a problem hiding this comment.
I found the regression above on trunk as well so not this PR.
youknowriad
approved these changes
Apr 30, 2024
jameskoster
approved these changes
Apr 30, 2024
draganescu
approved these changes
Apr 30, 2024
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Part of: #59878
Related: #59878
This PR continues the work on unifying the publishing flow between post and site editors and adds to the site editor the
publish/savebutton that is used in post editor. This means we have the same publishing flow between the two editors. Additionally the updatedPost Statuscomponent is reused too.Because of some extra use cases in site editor and trying to keep smaller change sets, this first iteration conditionally renders the reused
save/publishbutton and panel and the existing one in site editor.The extra use cases in site editor include:
Eventually all these should be consolidated by probably passing some extra props down to the reusable components or having an internal API to render something differently and will be explored separately. Also after unifying the flow we should update the designs based on: #59878 (comment)
Testing Instructions
Screenshots or screencast
Screen.Recording.2024-04-26.at.11.19.20.AM.mov