-
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
Prevent focus loss on reusable block edition #27950
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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.
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.
Howdy @youknowriad 👋 I arrived at this
editEntityRecord
call when debugging thePostPreviewButton
component and how it autosaves the post before showing the preview.The trouble is that when the
isEditedPostAutosaveable
selector is called, it callshasChangedContent
, which always returnstrue
even if I didn't edit the post at all.It's because this
editEntityRecord
call always adds an edit when the editor is being initialized (more precisely,EditorProvider
being mounted).hasChangedContent
checks if there are'blocks' in edits
which is always true. Theblocks
field never goes away.What do you think would be a good way to fix this? Is the entity's
blocks
field also initialized anywhere else, outsideEditorProvider
? Seems to me the initial value ofblock
should be set on the unedited post entity, to live alongside the originalcontent
it is derived from. Not in the edits.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.
Isn't there a flag maybe we can pass to
editEntityRecord
to mark the change as "non dirty" or something like that? (we had some kind of initial changes in the past).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.
Currently there isn't. And
edits.blocks
is not really an edit -- it's a computed property from the originalentity.content
and there is no changed content. Ideally the*.blocks
field would always live on the same object as*.content
, as they are two different representations of the same underlying data.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.
We do have a notion of
transientEdits
which is what edits.blocks is about but subsequent changes to "blocks" should be considered real edits (mark the post dirty, create undo levels...). Could the first time a "transition edit" be set considered a non-dirty edit?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.
Yes,
blocks
andselection
edits are considered transient and should never trigger save. Any real edit will also editcontent
, setting the value to a serialization function to it. So, we can completely ignoreblocks
and rely oncontent
edits to be always present.I'm fixing the
hasChangedContent
selector in #45090 to ignoreblocks
and look only atcontent
.