Skip to content

Commit

Permalink
🐛 Fixed unsaved changes confirmation on Lexical schema change (#20687)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@ronaldlangeveld
ronaldlangeveld authored Aug 5, 2024
1 parent e378252 commit c8ba9e8
Show file tree
Hide file tree
Showing 10 changed files with 143 additions and 53 deletions.
2 changes: 2 additions & 0 deletions ghost/admin/app/components/gh-koenig-editor-lexical.hbs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
7 changes: 7 additions & 0 deletions ghost/admin/app/components/gh-koenig-editor-lexical.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export default class GhKoenigEditorLexical extends Component {
uploadUrl = `${ghostPaths().apiRoot}/images/upload/`;

editorAPI = null;
secondaryEditorAPI = null;
skipFocusEditor = false;

@tracked titleIsHovered = false;
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions ghost/admin/app/components/gh-post-settings-menu.hbs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,7 @@
post=this.post
editorAPI=this.editorAPI
toggleSettingsMenu=this.toggleSettingsMenu
secondaryEditorAPI=this.secondaryEditorAPI
}}
@close={{this.closePostHistory}}
@modifier="total-overlay post-history" />
Expand Down
57 changes: 33 additions & 24 deletions ghost/admin/app/components/koenig-lexical-editor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<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 (
<div className={['koenig-react-editor', 'koenig-lexical', this.args.className].filter(Boolean).join(' ')}>
<ErrorHandler config={this.config}>
<Suspense fallback={<p className="koenig-react-editor-loading">Loading editor...</p>}>
<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={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>
<KGEditorComponent />
<KGEditorComponent isInitInstance={true} />
</Suspense>
</ErrorHandler>
</div>
Expand Down
1 change: 1 addition & 0 deletions ghost/admin/app/components/modal-post-history.hbs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
@lexical={{this.selectedRevision.lexical}}
@cardConfig={{this.cardConfig}}
@registerAPI={{this.registerSelectedEditorApi}}
@registerSecondaryAPI={{this.registerSecondarySelectedEditorApi}}
/>
</div>
</div>
Expand Down
7 changes: 7 additions & 0 deletions ghost/admin/app/components/modal-post-history.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
70 changes: 42 additions & 28 deletions ghost/admin/app/controllers/lexical-editor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1221,16 +1231,14 @@ export default class LexicalEditorController extends Controller {
_timedSaveTask;

/* Private methods -------------------------------------------------------*/

_hasDirtyAttributes() {
let post = this.post;

if (!post) {
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;
Expand All @@ -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 : [];

// // nullling is typically faster than delete
scratchChildNodes.forEach(child => child.direction = null);
lexicalChildNodes.forEach(child => child.direction = null);

if (JSON.stringify(scratchChildNodes) === JSON.stringify(lexicalChildNodes)) {
return false;
}
let secondaryLexical = post.get('secondaryLexicalState');

this._leaveModalReason = {reason: 'lexical is different', context: {current: lexical, scratch}};
return true;
}
}
let lexicalChildNodes = lexical ? JSON.parse(lexical).root?.children : [];
let scratchChildNodes = scratch ? JSON.parse(scratch).root?.children : [];
let secondaryLexicalChildNodes = secondaryLexical ? JSON.parse(secondaryLexical).root?.children : [];

lexicalChildNodes.forEach(child => child.direction = null);
scratchChildNodes.forEach(child => child.direction = null);
secondaryLexicalChildNodes.forEach(child => child.direction = null);

// new+unsaved posts always return `hasDirtyAttributes: true`
// Compare initLexical with scratch
let isSecondaryDirty = secondaryLexical && scratch && JSON.stringify(secondaryLexicalChildNodes) !== JSON.stringify(scratchChildNodes);

// 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());
Expand All @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions ghost/admin/app/models/post.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 3 additions & 0 deletions ghost/admin/app/templates/lexical-editor.hbs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -97,6 +98,7 @@
}}
@postType={{this.post.displayName}}
@registerAPI={{this.registerEditorAPI}}
@registerSecondaryAPI={{this.registerSecondaryEditorAPI}}
@savePostTask={{this.savePostTask}}
/>

Expand Down Expand Up @@ -136,6 +138,7 @@
@updateSlugTask={{this.updateSlugTask}}
@savePostTask={{this.savePostTask}}
@editorAPI={{this.editorAPI}}
@secondaryEditorAPI={{this.secondaryEditorAPI}}
@toggleSettingsMenu={{this.toggleSettingsMenu}}
/>
{{/if}}
Expand Down
45 changes: 44 additions & 1 deletion ghost/admin/tests/unit/controllers/editor-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
});
});
});

0 comments on commit c8ba9e8

Please sign in to comment.