Site editor: Fix opening of save panel when using the save shortcut
#59647
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] Bug
|
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. |
jasmussen
approved these changes
Mar 6, 2024
|
Size Change: +40 B (0%) Total Size: 1.71 MB
ℹ️ View Unchanged
|
|
edit: I pushed the update. |
youknowriad
added
the
Backport to WP Beta/RC
|
Ok, so there's a few confusing things when testing the PR. I just want to confirm that this is the right behavior that we want (also maybe we should compare with 6.4 to see what's the expected behavior)
It feels like there's a lot of inconsistencies and I'm not entirely sure if it's on purpose or not. cc @jasmussen @SaxonF |
|
I agree there are lots of inconsistencies but we have similar ones in post editor and I'm not sure if we should handle them separately or here(was leaning on follow up). For example in post editor if we have edits in more entities, the shortcut saves the main edited entity but not the other ones.
I think that's useful for when navigating in a different entity while in view mode. Maybe we could add one more check tied to the view mode to save directly if we're in the same page. We could do that here.
This is also related to: #38714, which suggests a preference for saving all entities at once(not only on shortcut use though). Design input is needed for sure about the way to move forward. |
|
Yeah, my main thing here is to check what we were doing in 6.4. If it's the same thing as this PR, let's ship and consider these separately. |
|
I have unfortunately not been that big of a part of the original discussion around the behavior, so it's very likely I'm missing nuance. My expectation would be that ⌘S only ever saves the page, because the expectation is that the shortcut saves the document I'm working on. If it opens the multi entity saving panel, it hasn't actually saved anything yet. Similarly, if it simply saves all the multi entities listed, without asking, then you're likely to get in trouble if you accidentally changed the site title without knowing, thinking you saved the page heading. To that end, I'd do either of the following two:
Which of the two to go with, I'd appreciate if someone with more historical knowledge of the shortcut can respond. |
|
What was the behavior in 6.4? |
|
Testing 6.4.3 just now: whether you have only local, or both local and global, in the site editor, ⌘S opens the multi entity saving panel. If you're in the post or page editor, ⌘S saves, and does so immediately. Saves the draft, or updates the page. If you're in the post or page editor and insert a site title, then change the site title, ⌘S still just saves or updates the local page, but does not open the multi entity saving panel — the "dot" to indicate there are such entities just remains. |
youknowriad
approved these changes
Mar 7, 2024
There was a problem hiding this comment.
Ok so it seems there's inconsistencies in here but also in 6.4 (post editor), so let's just merge this one as a fix for 6.5 and work towards @jasmussen's proposals for later.
This is what I would do. It's how the save button works currently and the state of a page may depend on other entities, like global styles for example. If we only save the page that isn't an accurate representation of what a user is seeing in front of them. |
getdave
pushed a commit
that referenced
this pull request
Mar 11, 2024
…#59647) Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: SaxonF <saxonafletcher@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org> Co-authored-by: colorful-tones <colorful-tones@git.wordpress.org> Co-authored-by: sunil25393 <sunil25393@git.wordpress.org>
|
I just cherry-picked this PR to the pick/wp-65-rc-2 branch to get it included in the next release: 00fcb56 |
getdave
added
Backported to WP Core
getdave
pushed a commit
that referenced
this pull request
Mar 12, 2024
…#59647) Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: SaxonF <saxonafletcher@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org> Co-authored-by: colorful-tones <colorful-tones@git.wordpress.org> Co-authored-by: sunil25393 <sunil25393@git.wordpress.org>
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?
Fixes: #59472
This PR adjusts the checks for when to open the save panel in site editor when using the
savekeyboard shortcut.Right now when using the shortcut, the changes for the edited post are automatically saved by the
editorglobal shortcut. So if we have changes only for the edited post there is no need to open the save panel at all.Additionally the current logic creates another bug which is if we make changes only to the edited post and use the shortcut, if we make more changes afterwards we cannot open the save panel by clicking the
savebutton because the value we keep inedit sitestore is wrong.Testing Instructions
saveshortcutsavebuttonviewmodeTrunk
Screen.Recording.2024-03-06.at.5.06.04.PM.mov
This PR
thisPR.mov