From c8ba9e8027bd8269f8fb0ff3303da6d1e34b9696 Mon Sep 17 00:00:00 2001 From: Ronald Langeveld Date: Mon, 5 Aug 2024 19:58:58 +0700 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fixed=20unsaved=20changes=20conf?= =?UTF-8?q?irmation=20on=20Lexical=20schema=20change=20(#20687)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../components/gh-koenig-editor-lexical.hbs | 2 + .../components/gh-koenig-editor-lexical.js | 7 ++ .../app/components/gh-post-settings-menu.hbs | 1 + .../app/components/koenig-lexical-editor.js | 57 +++++++++------- .../app/components/modal-post-history.hbs | 1 + .../app/components/modal-post-history.js | 7 ++ ghost/admin/app/controllers/lexical-editor.js | 66 +++++++++++-------- ghost/admin/app/models/post.js | 3 + ghost/admin/app/templates/lexical-editor.hbs | 3 + .../tests/unit/controllers/editor-test.js | 45 ++++++++++++- 10 files changed, 141 insertions(+), 51 deletions(-) diff --git a/ghost/admin/app/components/gh-koenig-editor-lexical.hbs b/ghost/admin/app/components/gh-koenig-editor-lexical.hbs index af3524b998..3ad4aae7a0 100644 --- a/ghost/admin/app/components/gh-koenig-editor-lexical.hbs +++ b/ghost/admin/app/components/gh-koenig-editor-lexical.hbs @@ -95,7 +95,9 @@ @placeholder={{@bodyPlaceholder}} @cardConfig={{@cardOptions}} @onChange={{@onBodyChange}} + @updateSecondaryInstanceModel={{@updateSecondaryInstanceModel}} @registerAPI={{this.registerEditorAPI}} + @registerSecondaryAPI={{this.registerSecondaryEditorAPI}} @cursorDidExitAtTop={{if this.feature.editorExcerpt this.focusExcerpt this.focusTitle}} @updateWordCount={{@updateWordCount}} @updatePostTkCount={{@updatePostTkCount}} diff --git a/ghost/admin/app/components/gh-koenig-editor-lexical.js b/ghost/admin/app/components/gh-koenig-editor-lexical.js index 2319ae9c8a..be28f73a5f 100644 --- a/ghost/admin/app/components/gh-koenig-editor-lexical.js +++ b/ghost/admin/app/components/gh-koenig-editor-lexical.js @@ -15,6 +15,7 @@ export default class GhKoenigEditorLexical extends Component { uploadUrl = `${ghostPaths().apiRoot}/images/upload/`; editorAPI = null; + secondaryEditorAPI = null; skipFocusEditor = false; @tracked titleIsHovered = false; @@ -232,6 +233,12 @@ export default class GhKoenigEditorLexical extends Component { 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, // otherwise the browser will defocus the editor and the cursor will disappear @action diff --git a/ghost/admin/app/components/gh-post-settings-menu.hbs b/ghost/admin/app/components/gh-post-settings-menu.hbs index 1d01dcc5bc..ee91b2af80 100644 --- a/ghost/admin/app/components/gh-post-settings-menu.hbs +++ b/ghost/admin/app/components/gh-post-settings-menu.hbs @@ -853,6 +853,7 @@ post=this.post editorAPI=this.editorAPI toggleSettingsMenu=this.toggleSettingsMenu + secondaryEditorAPI=this.secondaryEditorAPI }} @close={{this.closePostHistory}} @modifier="total-overlay post-history" /> diff --git a/ghost/admin/app/components/koenig-lexical-editor.js b/ghost/admin/app/components/koenig-lexical-editor.js index 4365e4e26c..8f91477f74 100644 --- a/ghost/admin/app/components/koenig-lexical-editor.js +++ b/ghost/admin/app/components/koenig-lexical-editor.js @@ -669,34 +669,43 @@ export default class KoenigLexicalEditor extends Component { const multiplayerDocId = cardConfig.post.id; const multiplayerUsername = this.session.user.name; + const KGEditorComponent = ({isInitInstance}) => { + return ( +
+ + + {} : this.args.updateWordCount} /> + {} : this.args.updatePostTkCount} /> + +
+ ); + }; + return (
Loading editor...

}> - - - - - + +
diff --git a/ghost/admin/app/components/modal-post-history.hbs b/ghost/admin/app/components/modal-post-history.hbs index 3989c29db5..f7f8ab1261 100644 --- a/ghost/admin/app/components/modal-post-history.hbs +++ b/ghost/admin/app/components/modal-post-history.hbs @@ -33,6 +33,7 @@ @lexical={{this.selectedRevision.lexical}} @cardConfig={{this.cardConfig}} @registerAPI={{this.registerSelectedEditorApi}} + @registerSecondaryAPI={{this.registerSecondarySelectedEditorApi}} /> diff --git a/ghost/admin/app/components/modal-post-history.js b/ghost/admin/app/components/modal-post-history.js index 05aba6646e..4dfa987e2e 100644 --- a/ghost/admin/app/components/modal-post-history.js +++ b/ghost/admin/app/components/modal-post-history.js @@ -31,6 +31,7 @@ export default class ModalPostHistory extends Component { super(...arguments); this.post = this.args.model.post; this.editorAPI = this.args.model.editorAPI; + this.secondaryEditorAPI = this.args.model.secondaryEditorAPI; this.toggleSettingsMenu = this.args.model.toggleSettingsMenu; } @@ -101,6 +102,11 @@ export default class ModalPostHistory extends Component { this.selectedEditor = api; } + @action + registerSecondarySelectedEditorApi(api) { + this.secondarySelectedEditor = api; + } + @action registerComparisonEditorApi(api) { this.comparisonEditor = api; @@ -130,6 +136,7 @@ export default class ModalPostHistory extends Component { updateEditor: () => { const state = this.editorAPI.editorInstance.parseEditorState(revision.lexical); this.editorAPI.editorInstance.setEditorState(state); + this.secondaryEditorAPI.editorInstance.setEditorState(state); }, closePostHistoryModal: () => { this.closeModal(); diff --git a/ghost/admin/app/controllers/lexical-editor.js b/ghost/admin/app/controllers/lexical-editor.js index 71c6331437..2b8342afd9 100644 --- a/ghost/admin/app/controllers/lexical-editor.js +++ b/ghost/admin/app/controllers/lexical-editor.js @@ -297,6 +297,11 @@ export default class LexicalEditorController extends Controller { this._timedSaveTask.perform(); } + @action + updateSecondaryInstanceModel(lexical) { + this.set('post.secondaryLexicalState', JSON.stringify(lexical)); + } + @action updateTitleScratch(title) { this.set('post.titleScratch', title); @@ -423,6 +428,11 @@ export default class LexicalEditorController extends Controller { this.editorAPI = API; } + @action + registerSecondaryEditorAPI(API) { + this.secondaryEditorAPI = API; + } + @action clearFeatureImage() { this.post.set('featureImage', null); @@ -1221,7 +1231,6 @@ export default class LexicalEditorController extends Controller { _timedSaveTask; /* Private methods -------------------------------------------------------*/ - _hasDirtyAttributes() { let post = this.post; @@ -1229,8 +1238,7 @@ export default class LexicalEditorController extends Controller { return false; } - // if the Adapter failed to save the post isError will be true - // and we should consider the post still dirty. + // If the Adapter failed to save the post, isError will be true, and we should consider the post still dirty. if (post.get('isError')) { this._leaveModalReason = {reason: 'isError', context: post.errors.messages}; return true; @@ -1245,37 +1253,32 @@ export default class LexicalEditorController extends Controller { return true; } - // titleScratch isn't an attr so needs a manual dirty check + // Title scratch comparison if (post.titleScratch !== post.title) { this._leaveModalReason = {reason: 'title is different', context: {current: post.title, scratch: post.titleScratch}}; return true; } - // scratch isn't an attr so needs a manual dirty check + // Lexical and scratch comparison let lexical = post.get('lexical'); let scratch = post.get('lexicalScratch'); - // additional guard in case we are trying to compare null with undefined - 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 : []; + let secondaryLexical = post.get('secondaryLexicalState'); - // // nullling is typically faster than delete - scratchChildNodes.forEach(child => child.direction = null); - lexicalChildNodes.forEach(child => child.direction = null); + let lexicalChildNodes = lexical ? JSON.parse(lexical).root?.children : []; + let scratchChildNodes = scratch ? JSON.parse(scratch).root?.children : []; + let secondaryLexicalChildNodes = secondaryLexical ? JSON.parse(secondaryLexical).root?.children : []; - if (JSON.stringify(scratchChildNodes) === JSON.stringify(lexicalChildNodes)) { - return false; - } + lexicalChildNodes.forEach(child => child.direction = null); + scratchChildNodes.forEach(child => child.direction = null); + secondaryLexicalChildNodes.forEach(child => child.direction = null); - this._leaveModalReason = {reason: 'lexical is different', context: {current: lexical, scratch}}; - return true; - } - } + // Compare initLexical with scratch + 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 if (post.get('isNew')) { let changedAttributes = Object.keys(post.changedAttributes()); @@ -1286,15 +1289,26 @@ export default class LexicalEditorController extends Controller { 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 let {hasDirtyAttributes} = post; - if (hasDirtyAttributes) { 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) { diff --git a/ghost/admin/app/models/post.js b/ghost/admin/app/models/post.js index 1ffb06d8d0..835d24d0a2 100644 --- a/ghost/admin/app/models/post.js +++ b/ghost/admin/app/models/post.js @@ -136,6 +136,9 @@ export default Model.extend(Comparable, ValidationEngine, { scratch: null, lexicalScratch: 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 // on save. Updated by an observer whenever publishedAtUTC changes. diff --git a/ghost/admin/app/templates/lexical-editor.hbs b/ghost/admin/app/templates/lexical-editor.hbs index bd9d5d51e7..b7ee9a4a74 100644 --- a/ghost/admin/app/templates/lexical-editor.hbs +++ b/ghost/admin/app/templates/lexical-editor.hbs @@ -73,6 +73,7 @@ @body={{readonly this.post.lexicalScratch}} @bodyPlaceholder={{concat "Begin writing your " this.post.displayName "..."}} @onBodyChange={{this.updateScratch}} + @updateSecondaryInstanceModel={{this.updateSecondaryInstanceModel}} @headerOffset={{editor.headerHeight}} @scrollContainerSelector=".gh-koenig-editor" @scrollOffsetBottomSelector=".gh-mobile-nav-bar" @@ -97,6 +98,7 @@ }} @postType={{this.post.displayName}} @registerAPI={{this.registerEditorAPI}} + @registerSecondaryAPI={{this.registerSecondaryEditorAPI}} @savePostTask={{this.savePostTask}} /> @@ -136,6 +138,7 @@ @updateSlugTask={{this.updateSlugTask}} @savePostTask={{this.savePostTask}} @editorAPI={{this.editorAPI}} + @secondaryEditorAPI={{this.secondaryEditorAPI}} @toggleSettingsMenu={{this.toggleSettingsMenu}} /> {{/if}} diff --git a/ghost/admin/tests/unit/controllers/editor-test.js b/ghost/admin/tests/unit/controllers/editor-test.js index 088f22391d..acf7a5e95b 100644 --- a/ghost/admin/tests/unit/controllers/editor-test.js +++ b/ghost/admin/tests/unit/controllers/editor-test.js @@ -208,7 +208,8 @@ describe('Unit: Controller: lexical-editor', function () { titleScratch: 'this is a title', status: 'published', 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 @@ -225,5 +226,47 @@ describe('Unit: Controller: lexical-editor', function () { isDirty = controller.get('hasDirtyAttributes'); 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; + }); }); });