🐛 Fixed unsaved changes confirmation on Lexical schema change #20687
Conversation
ronaldlangeveld
changed the title
| placeholderText={this.args.placeholder} | ||
| darkMode={this.feature.nightShift} | ||
| onChange={isInitInstance ? e => handleInitInstance(e) : this.args.onChange} | ||
| registerAPI={this.args.registerAPI} |
There was a problem hiding this comment.
We'll want to make sure this doesn't get registered for the secondary instance otherwise we won't have a stable reference to the main editor
| <WordCountPlugin editorResource={this.editorResource} onChange={this.args.updateWordCount} /> | ||
| <TKCountPlugin editorResource={this.editorResource} onChange={this.args.updatePostTkCount} /> |
There was a problem hiding this comment.
Same for both of these, we don't want the secondary instance interacting with any of the outside world aside from the main content onChange handler
| if (this.args.initLexical && isInitInstance && !hasInitialised) { | ||
| this.args.initLexical(data); | ||
| setHasInitialised(true); |
There was a problem hiding this comment.
Why do we need hasInitialised here? One of the issues Steve ran into is that the changes made when opening the editor come in batches so we can't guarantee that only the first onChange call is the one we want - if we could rely on that then we could use the first call's data from the main editor instance rather than needing a secondary instance
| @@ -404,6 +404,12 @@ export default class LexicalEditorController extends Controller { | |||
| this.set('postTkCount', count); | |||
| } | |||
|
|
|||
| @action | |||
| initLexical(data) { | |||
| this.post.set('lexical', JSON.stringify(data)); | |||
There was a problem hiding this comment.
I don't think we should be updating the post's lexical data otherwise we're in the same position as before but we're masking it by setting hasDirtyAttributes to false which may not be correct! Aside from indirectly masking the problem it also means the model immediately has dirty attributes so we can no longer use that as part of our dirty checks.
The idea of having the secondary instance was so we can add a new property that contains the modified-on-init data that we can compare against inside our _hasDirtyAttributes() function
| if (post.get('isError')) { | ||
| this._leaveModalReason = {reason: 'isError', context: post.errors.messages}; | ||
| return true; | ||
| } | ||
|
|
||
| // post.tags is an array so hasDirtyAttributes doesn't pick up | ||
| // changes unless the array ref is changed | ||
| // Post tags comparison |
There was a problem hiding this comment.
Can we switch back to the comments that explain why this is necessary rather than a comment that just tells you what the code is doing?
…TryGhost#20687)" This reverts commit c8ba9e8.
refs ENG-661 Fixes a long-standing issue where an outdated Lexical schema in the database triggered the unsaved changes confirmation dialog incorrectly. Implemented a secondary hidden Lexical instance that loads the state from the database, renders it, and uses this updated state to compare with the live editor's scratch. This ensures the unsaved changes prompt appears only when there are real changes from the user.
ref [ENG-661](https://linear.app/tryghost/issue/ENG-661/) ref [ONC-253](https://linear.app/tryghost/issue/ONC-253/) ref [PLG-174](https://linear.app/tryghost/issue/PLG-174/) - restored the original but reverted fix for unsaved changes modal from #20687 - updated code to remove some incorrect early-falsy-return logic in `editorController.hasDirtyAttributes` that prevented save of unsaved changes on the underlying model (e.g. excerpt) - updated unit tests so they are testing real post model instances and therefore are testing what we expect them to test - added acceptance tests to ensure autosave is working for title and excerpt fields --------- Co-authored-by: Ronald Langeveld <[email protected]>
ref [ENG-661](https://linear.app/tryghost/issue/ENG-661/) ref [ONC-253](https://linear.app/tryghost/issue/ONC-253/) ref [PLG-174](https://linear.app/tryghost/issue/PLG-174/) - restored the original but reverted fix for unsaved changes modal from #20687 - updated code to remove some incorrect early-falsy-return logic in `editorController.hasDirtyAttributes` that prevented save of unsaved changes on the underlying model (e.g. excerpt) - updated unit tests so they are testing real post model instances and therefore are testing what we expect them to test - added acceptance tests to ensure autosave is working for title and excerpt fields --------- Co-authored-by: Ronald Langeveld <[email protected]>
refs ENG-661
Fixes a long-standing issue where an outdated Lexical schema in the database triggered the unsaved changes confirmation dialog incorrectly.
Implemented a secondary hidden Lexical instance that loads the state from the database, renders it, and uses this updated state to compare with the live editor's scratch.
This ensures the unsaved changes prompt appears only when there are real changes from the user.