Effects: Restore new post setup as non-dirtying change#4836
Conversation
aduth
added
[Type] Bug
Avoids "Undo" present on new post
aduth
force-pushed
the
fix/save-auto-draft-empty-title
branch
from
7dce790 to
daac48c
Compare
aduth
force-pushed
the
fix/save-auto-draft-empty-title
branch
from
daac48c to
88f922e
Compare
|
The added E2E tests here illustrated a pain point, which is that Cypress provides Mocha + Chai in its testing environment, conflicting with ESLint rules we have configured for Jest (extended via |
| // `.eslintrc.json`, the exceptions being that since Cypress is preconfigured | ||
| // with Mocha & Chai, we need to broadly disable the Jest rules, which is | ||
| // otherwise difficult to do. This file could be made more minimal once all of | ||
| // the Gutenberg-specific rules are migrated to a common WordPress config. |
There was a problem hiding this comment.
I wonder if there's a way to override the root config instead of copying it
There was a problem hiding this comment.
Agreed, that would be the best solution. I would leave it as it is. We are going to tackle it together with @ntwb as part of @wordpress/packages soon.
There was a problem hiding this comment.
The previous iteration of the configuration file is how to override the root config (i.e. ESLint will look in all parent directories until it finds a config with root: true). The problem is that by opting into many Jest presets, there are many rules and env values we'd need to disable individually.
There was a problem hiding this comment.
It's more complicated than I would expect.
| cy.get( '.editor-history__redo:not( :disabled )' ).should( 'not.exist' ); | ||
| } ); | ||
|
|
||
| it( 'Should not prompt to confirm unsaved changes', ( done ) => { |
There was a problem hiding this comment.
I was afraid this was a false positive because I thought Cypress would remove all popovers but confirmed it's breaking correctly when needed.
|
|
||
| cy.reload(); | ||
| } ); | ||
| } ); |
3 participants
Regression introduced in #3983
This pull request seeks to resolve an issue where saving a new post which has an empty title will set the title as "Auto Draft".
When starting a new post, a temporary post is created with "Auto Draft" as the assigned title. Obviously, we do not want to display this title to the user, so we artificially set the title to an empty string during editor initialization.
In the case that the user never sets a title of their own, we must also include the empty string value in the save request. This relies on a complex exchange whereby:
To resolve this, we can add additional reset types to the
withChangeDetectionhigher-order reducer applied forstate.editor. TheRESET_POSTaction must occur beforeSETUP_NEW_POSTso the post setup is considered a change, butSETUP_NEW_POSTmust also reset change detection.In the course of implementing this, I also discovered that
SETUP_NEW_POSTshould also be a reset type for change history, as prior to this change, "Undo" would be offered as an option for a new post.Next steps:
I'd encourage exploring two tasks in the future, as this flow is rather fragile:
Add integration tests for editor initialization, e.g. "After the editor finishes loading, there should be a title change, but not dirty, and with no history". I was going to add these here, but given how the editor store is shared, it wasn't clear how we isolate/reset/tear down this in unit tests (cc @youknowriad @gziolo).Edit: I guess these could be e2e tests?Added in 7dce790.Testing instructions: