🐛 Fixed unsaved changes confirmation on Lexical schema change (#20687)

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.
This commit is contained in:
Ronald Langeveld 2024-08-05 19:58:58 +07:00 committed by GitHub
parent e378252d36
commit c8ba9e8027
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
10 changed files with 141 additions and 51 deletions

View File

@ -95,7 +95,9 @@
@placeholder={{@bodyPlaceholder}} @placeholder={{@bodyPlaceholder}}
@cardConfig={{@cardOptions}} @cardConfig={{@cardOptions}}
@onChange={{@onBodyChange}} @onChange={{@onBodyChange}}
@updateSecondaryInstanceModel={{@updateSecondaryInstanceModel}}
@registerAPI={{this.registerEditorAPI}} @registerAPI={{this.registerEditorAPI}}
@registerSecondaryAPI={{this.registerSecondaryEditorAPI}}
@cursorDidExitAtTop={{if this.feature.editorExcerpt this.focusExcerpt this.focusTitle}} @cursorDidExitAtTop={{if this.feature.editorExcerpt this.focusExcerpt this.focusTitle}}
@updateWordCount={{@updateWordCount}} @updateWordCount={{@updateWordCount}}
@updatePostTkCount={{@updatePostTkCount}} @updatePostTkCount={{@updatePostTkCount}}

View File

@ -15,6 +15,7 @@ export default class GhKoenigEditorLexical extends Component {
uploadUrl = `${ghostPaths().apiRoot}/images/upload/`; uploadUrl = `${ghostPaths().apiRoot}/images/upload/`;
editorAPI = null; editorAPI = null;
secondaryEditorAPI = null;
skipFocusEditor = false; skipFocusEditor = false;
@tracked titleIsHovered = false; @tracked titleIsHovered = false;
@ -232,6 +233,12 @@ export default class GhKoenigEditorLexical extends Component {
this.args.registerAPI(API); this.args.registerAPI(API);
} }
@action
registerSecondaryEditorAPI(API) {
this.secondaryEditorAPI = API;
this.args.registerSecondaryAPI(API);
}
// focus the editor when the editor canvas is clicked below the editor content, // focus the editor when the editor canvas is clicked below the editor content,
// otherwise the browser will defocus the editor and the cursor will disappear // otherwise the browser will defocus the editor and the cursor will disappear
@action @action

View File

@ -853,6 +853,7 @@
post=this.post post=this.post
editorAPI=this.editorAPI editorAPI=this.editorAPI
toggleSettingsMenu=this.toggleSettingsMenu toggleSettingsMenu=this.toggleSettingsMenu
secondaryEditorAPI=this.secondaryEditorAPI
}} }}
@close={{this.closePostHistory}} @close={{this.closePostHistory}}
@modifier="total-overlay post-history" /> @modifier="total-overlay post-history" />

View File

@ -669,34 +669,43 @@ export default class KoenigLexicalEditor extends Component {
const multiplayerDocId = cardConfig.post.id; const multiplayerDocId = cardConfig.post.id;
const multiplayerUsername = this.session.user.name; const multiplayerUsername = this.session.user.name;
const KGEditorComponent = ({isInitInstance}) => {
return (
<div style={isInitInstance ? {visibility: 'hidden', position: 'absolute'} : {}}>
<KoenigComposer
editorResource={this.editorResource}
cardConfig={cardConfig}
enableMultiplayer={enableMultiplayer}
fileUploader={{useFileUpload, fileTypes}}
initialEditorState={this.args.lexical}
multiplayerUsername={multiplayerUsername}
multiplayerDocId={multiplayerDocId}
multiplayerEndpoint={multiplayerEndpoint}
onError={this.onError}
darkMode={this.feature.nightShift}
isTKEnabled={true}
>
<KoenigEditor
editorResource={this.editorResource}
cursorDidExitAtTop={isInitInstance ? null : this.args.cursorDidExitAtTop}
placeholderText={isInitInstance ? null : this.args.placeholderText}
darkMode={isInitInstance ? null : this.feature.nightShift}
onChange={isInitInstance ? this.args.updateSecondaryInstanceModel : this.args.onChange}
registerAPI={isInitInstance ? this.args.registerSecondaryAPI : this.args.registerAPI}
/>
<WordCountPlugin editorResource={this.editorResource} onChange={isInitInstance ? () => {} : this.args.updateWordCount} />
<TKCountPlugin editorResource={this.editorResource} onChange={isInitInstance ? () => {} : this.args.updatePostTkCount} />
</KoenigComposer>
</div>
);
};
return ( return (
<div className={['koenig-react-editor', 'koenig-lexical', this.args.className].filter(Boolean).join(' ')}> <div className={['koenig-react-editor', 'koenig-lexical', this.args.className].filter(Boolean).join(' ')}>
<ErrorHandler config={this.config}> <ErrorHandler config={this.config}>
<Suspense fallback={<p className="koenig-react-editor-loading">Loading editor...</p>}> <Suspense fallback={<p className="koenig-react-editor-loading">Loading editor...</p>}>
<KoenigComposer <KGEditorComponent />
editorResource={this.editorResource} <KGEditorComponent isInitInstance={true} />
cardConfig={cardConfig}
enableMultiplayer={enableMultiplayer}
fileUploader={{useFileUpload, fileTypes}}
initialEditorState={this.args.lexical}
multiplayerUsername={multiplayerUsername}
multiplayerDocId={multiplayerDocId}
multiplayerEndpoint={multiplayerEndpoint}
onError={this.onError}
darkMode={this.feature.nightShift}
isTKEnabled={true}
>
<KoenigEditor
editorResource={this.editorResource}
cursorDidExitAtTop={this.args.cursorDidExitAtTop}
placeholderText={this.args.placeholder}
darkMode={this.feature.nightShift}
onChange={this.args.onChange}
registerAPI={this.args.registerAPI}
/>
<WordCountPlugin editorResource={this.editorResource} onChange={this.args.updateWordCount} />
<TKCountPlugin editorResource={this.editorResource} onChange={this.args.updatePostTkCount} />
</KoenigComposer>
</Suspense> </Suspense>
</ErrorHandler> </ErrorHandler>
</div> </div>

View File

@ -33,6 +33,7 @@
@lexical={{this.selectedRevision.lexical}} @lexical={{this.selectedRevision.lexical}}
@cardConfig={{this.cardConfig}} @cardConfig={{this.cardConfig}}
@registerAPI={{this.registerSelectedEditorApi}} @registerAPI={{this.registerSelectedEditorApi}}
@registerSecondaryAPI={{this.registerSecondarySelectedEditorApi}}
/> />
</div> </div>
</div> </div>

View File

@ -31,6 +31,7 @@ export default class ModalPostHistory extends Component {
super(...arguments); super(...arguments);
this.post = this.args.model.post; this.post = this.args.model.post;
this.editorAPI = this.args.model.editorAPI; this.editorAPI = this.args.model.editorAPI;
this.secondaryEditorAPI = this.args.model.secondaryEditorAPI;
this.toggleSettingsMenu = this.args.model.toggleSettingsMenu; this.toggleSettingsMenu = this.args.model.toggleSettingsMenu;
} }
@ -101,6 +102,11 @@ export default class ModalPostHistory extends Component {
this.selectedEditor = api; this.selectedEditor = api;
} }
@action
registerSecondarySelectedEditorApi(api) {
this.secondarySelectedEditor = api;
}
@action @action
registerComparisonEditorApi(api) { registerComparisonEditorApi(api) {
this.comparisonEditor = api; this.comparisonEditor = api;
@ -130,6 +136,7 @@ export default class ModalPostHistory extends Component {
updateEditor: () => { updateEditor: () => {
const state = this.editorAPI.editorInstance.parseEditorState(revision.lexical); const state = this.editorAPI.editorInstance.parseEditorState(revision.lexical);
this.editorAPI.editorInstance.setEditorState(state); this.editorAPI.editorInstance.setEditorState(state);
this.secondaryEditorAPI.editorInstance.setEditorState(state);
}, },
closePostHistoryModal: () => { closePostHistoryModal: () => {
this.closeModal(); this.closeModal();

View File

@ -297,6 +297,11 @@ export default class LexicalEditorController extends Controller {
this._timedSaveTask.perform(); this._timedSaveTask.perform();
} }
@action
updateSecondaryInstanceModel(lexical) {
this.set('post.secondaryLexicalState', JSON.stringify(lexical));
}
@action @action
updateTitleScratch(title) { updateTitleScratch(title) {
this.set('post.titleScratch', title); this.set('post.titleScratch', title);
@ -423,6 +428,11 @@ export default class LexicalEditorController extends Controller {
this.editorAPI = API; this.editorAPI = API;
} }
@action
registerSecondaryEditorAPI(API) {
this.secondaryEditorAPI = API;
}
@action @action
clearFeatureImage() { clearFeatureImage() {
this.post.set('featureImage', null); this.post.set('featureImage', null);
@ -1221,7 +1231,6 @@ export default class LexicalEditorController extends Controller {
_timedSaveTask; _timedSaveTask;
/* Private methods -------------------------------------------------------*/ /* Private methods -------------------------------------------------------*/
_hasDirtyAttributes() { _hasDirtyAttributes() {
let post = this.post; let post = this.post;
@ -1229,8 +1238,7 @@ export default class LexicalEditorController extends Controller {
return false; return false;
} }
// if the Adapter failed to save the post isError will be true // If the Adapter failed to save the post, isError will be true, and we should consider the post still dirty.
// and we should consider the post still dirty.
if (post.get('isError')) { if (post.get('isError')) {
this._leaveModalReason = {reason: 'isError', context: post.errors.messages}; this._leaveModalReason = {reason: 'isError', context: post.errors.messages};
return true; return true;
@ -1245,37 +1253,32 @@ export default class LexicalEditorController extends Controller {
return true; return true;
} }
// titleScratch isn't an attr so needs a manual dirty check // Title scratch comparison
if (post.titleScratch !== post.title) { if (post.titleScratch !== post.title) {
this._leaveModalReason = {reason: 'title is different', context: {current: post.title, scratch: post.titleScratch}}; this._leaveModalReason = {reason: 'title is different', context: {current: post.title, scratch: post.titleScratch}};
return true; return true;
} }
// scratch isn't an attr so needs a manual dirty check // Lexical and scratch comparison
let lexical = post.get('lexical'); let lexical = post.get('lexical');
let scratch = post.get('lexicalScratch'); let scratch = post.get('lexicalScratch');
// additional guard in case we are trying to compare null with undefined let secondaryLexical = post.get('secondaryLexicalState');
if (scratch || lexical) {
if (scratch !== lexical) {
// lexical can dynamically set direction on loading editor state (e.g. "rtl"/"ltr") per the DOM context
// and we need to ignore this as a change from the user; see https://github.com/facebook/lexical/issues/4998
const scratchChildNodes = scratch ? JSON.parse(scratch).root?.children : [];
const lexicalChildNodes = lexical ? JSON.parse(lexical).root?.children : [];
// // nullling is typically faster than delete let lexicalChildNodes = lexical ? JSON.parse(lexical).root?.children : [];
scratchChildNodes.forEach(child => child.direction = null); let scratchChildNodes = scratch ? JSON.parse(scratch).root?.children : [];
lexicalChildNodes.forEach(child => child.direction = null); let secondaryLexicalChildNodes = secondaryLexical ? JSON.parse(secondaryLexical).root?.children : [];
if (JSON.stringify(scratchChildNodes) === JSON.stringify(lexicalChildNodes)) { lexicalChildNodes.forEach(child => child.direction = null);
return false; scratchChildNodes.forEach(child => child.direction = null);
} secondaryLexicalChildNodes.forEach(child => child.direction = null);
this._leaveModalReason = {reason: 'lexical is different', context: {current: lexical, scratch}}; // Compare initLexical with scratch
return true; let isSecondaryDirty = secondaryLexical && scratch && JSON.stringify(secondaryLexicalChildNodes) !== JSON.stringify(scratchChildNodes);
}
}
// new+unsaved posts always return `hasDirtyAttributes: true` // Compare lexical with scratch
let isLexicalDirty = lexical && scratch && JSON.stringify(lexicalChildNodes) !== JSON.stringify(scratchChildNodes);
// New+unsaved posts always return `hasDirtyAttributes: true`
// so we need a manual check to see if any // so we need a manual check to see if any
if (post.get('isNew')) { if (post.get('isNew')) {
let changedAttributes = Object.keys(post.changedAttributes()); let changedAttributes = Object.keys(post.changedAttributes());
@ -1286,15 +1289,26 @@ export default class LexicalEditorController extends Controller {
return changedAttributes.length ? true : false; return changedAttributes.length ? true : false;
} }
// we've covered all the non-tracked cases we care about so fall // We've covered all the non-tracked cases we care about so fall
// back on Ember Data's default dirty attribute checks // back on Ember Data's default dirty attribute checks
let {hasDirtyAttributes} = post; let {hasDirtyAttributes} = post;
if (hasDirtyAttributes) { if (hasDirtyAttributes) {
this._leaveModalReason = {reason: 'post.hasDirtyAttributes === true', context: post.changedAttributes()}; this._leaveModalReason = {reason: 'post.hasDirtyAttributes === true', context: post.changedAttributes()};
return true;
} }
return hasDirtyAttributes; // If either comparison is not dirty, return false, because scratch is always up to date.
if (!isSecondaryDirty || !isLexicalDirty) {
return false;
}
// If both comparisons are dirty, consider the post dirty
if (isSecondaryDirty && isLexicalDirty) {
this._leaveModalReason = {reason: 'initLexical and lexical are different from scratch', context: {secondaryLexical, lexical, scratch}};
return true;
}
return false;
} }
_showSaveNotification(prevStatus, status, delayed) { _showSaveNotification(prevStatus, status, delayed) {

View File

@ -136,6 +136,9 @@ export default Model.extend(Comparable, ValidationEngine, {
scratch: null, scratch: null,
lexicalScratch: null, lexicalScratch: null,
titleScratch: null, titleScratch: null,
//This is used to store the initial lexical state from the
// secondary editor to get the schema up to date in case its outdated
secondaryLexicalState: null,
// For use by date/time pickers - will be validated then converted to UTC // For use by date/time pickers - will be validated then converted to UTC
// on save. Updated by an observer whenever publishedAtUTC changes. // on save. Updated by an observer whenever publishedAtUTC changes.

View File

@ -73,6 +73,7 @@
@body={{readonly this.post.lexicalScratch}} @body={{readonly this.post.lexicalScratch}}
@bodyPlaceholder={{concat "Begin writing your " this.post.displayName "..."}} @bodyPlaceholder={{concat "Begin writing your " this.post.displayName "..."}}
@onBodyChange={{this.updateScratch}} @onBodyChange={{this.updateScratch}}
@updateSecondaryInstanceModel={{this.updateSecondaryInstanceModel}}
@headerOffset={{editor.headerHeight}} @headerOffset={{editor.headerHeight}}
@scrollContainerSelector=".gh-koenig-editor" @scrollContainerSelector=".gh-koenig-editor"
@scrollOffsetBottomSelector=".gh-mobile-nav-bar" @scrollOffsetBottomSelector=".gh-mobile-nav-bar"
@ -97,6 +98,7 @@
}} }}
@postType={{this.post.displayName}} @postType={{this.post.displayName}}
@registerAPI={{this.registerEditorAPI}} @registerAPI={{this.registerEditorAPI}}
@registerSecondaryAPI={{this.registerSecondaryEditorAPI}}
@savePostTask={{this.savePostTask}} @savePostTask={{this.savePostTask}}
/> />
@ -136,6 +138,7 @@
@updateSlugTask={{this.updateSlugTask}} @updateSlugTask={{this.updateSlugTask}}
@savePostTask={{this.savePostTask}} @savePostTask={{this.savePostTask}}
@editorAPI={{this.editorAPI}} @editorAPI={{this.editorAPI}}
@secondaryEditorAPI={{this.secondaryEditorAPI}}
@toggleSettingsMenu={{this.toggleSettingsMenu}} @toggleSettingsMenu={{this.toggleSettingsMenu}}
/> />
{{/if}} {{/if}}

View File

@ -208,7 +208,8 @@ describe('Unit: Controller: lexical-editor', function () {
titleScratch: 'this is a title', titleScratch: 'this is a title',
status: 'published', status: 'published',
lexical: initialLexicalString, lexical: initialLexicalString,
lexicalScratch: initialLexicalString lexicalScratch: initialLexicalString,
secondaryLexicalState: initialLexicalString
})); }));
// synthetically update the lexicalScratch as if the editor itself made the modifications on loading the initial editorState // synthetically update the lexicalScratch as if the editor itself made the modifications on loading the initial editorState
@ -225,5 +226,47 @@ describe('Unit: Controller: lexical-editor', function () {
isDirty = controller.get('hasDirtyAttributes'); isDirty = controller.get('hasDirtyAttributes');
expect(isDirty).to.be.true; expect(isDirty).to.be.true;
}); });
it('dirty is false if secondaryLexical and scratch matches, but lexical is outdated', async function () {
const initialLexicalString = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Sample content","type": "extended-text","version": 1}],"direction": null,"format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
const lexicalScratch = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Sample content","type": "extended-text","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
const secondLexicalInstance = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Here's some new text","type": "extended-text","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
let controller = this.owner.lookup('controller:lexical-editor');
controller.set('post', EmberObject.create({
title: 'this is a title',
titleScratch: 'this is a title',
status: 'published',
lexical: initialLexicalString,
lexicalScratch: lexicalScratch,
secondaryLexicalState: secondLexicalInstance
}));
let isDirty = controller.get('hasDirtyAttributes');
expect(isDirty).to.be.false;
});
it('dirty is true if secondaryLexical and lexical does not match scratch', async function () {
const initialLexicalString = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Sample content","type": "extended-text","version": 1}],"direction": null,"format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
const lexicalScratch = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Sample content1234","type": "extended-text","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
const secondLexicalInstance = `{"root":{"children":[{"children": [{"detail": 0,"format": 0,"mode": "normal","style": "","text": "Here's some new text","type": "extended-text","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "paragraph","version": 1}],"direction": "ltr","format": "","indent": 0,"type": "root","version": 1}}`;
let controller = this.owner.lookup('controller:lexical-editor');
controller.set('post', EmberObject.create({
title: 'this is a title',
titleScratch: 'this is a title',
status: 'published',
lexical: initialLexicalString,
lexicalScratch: lexicalScratch,
secondaryLexicalState: secondLexicalInstance
}));
controller.send('updateScratch',JSON.parse(lexicalScratch));
let isDirty = controller.get('hasDirtyAttributes');
expect(isDirty).to.be.true;
});
}); });
}); });