Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fixed a bug causing new drafts to only save if the title is populated #20769

Merged
merged 1 commit into from
Aug 16, 2024
Merged

🐛 Fixed a bug causing new drafts to only save if the title is populated #20769

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions ghost/admin/app/components/gh-koenig-editor-lexical.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,7 @@
@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: 0 additions & 7 deletions ghost/admin/app/components/gh-koenig-editor-lexical.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export default class GhKoenigEditorLexical extends Component {
uploadUrl = `${ghostPaths().apiRoot}/images/upload/`;

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

@tracked titleIsHovered = false;
Expand Down Expand Up @@ -233,12 +232,6 @@ 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: 0 additions & 1 deletion ghost/admin/app/components/gh-post-settings-menu.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,7 +853,6 @@
post=this.post
editorAPI=this.editorAPI
toggleSettingsMenu=this.toggleSettingsMenu
secondaryEditorAPI=this.secondaryEditorAPI
}}
@close={{this.closePostHistory}}
@modifier="total-overlay post-history" />
Expand Down
57 changes: 24 additions & 33 deletions ghost/admin/app/components/koenig-lexical-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -678,43 +678,34 @@ export default class KoenigLexicalEditor extends Component {
const multiplayerDocId = cardConfig.post.id;
const multiplayerUsername = this.session.user.name;

const KGEditorComponent = ({isInitInstance}) => {
return (
<div data-secondary-instance={isInitInstance ? true : false} style={isInitInstance ? {width: 0, height: 0, overflow: 'hidden'} : {}}>
<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>}>
<KGEditorComponent />
<KGEditorComponent isInitInstance={true} />
<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>
</Suspense>
</ErrorHandler>
</div>
Expand Down
1 change: 0 additions & 1 deletion ghost/admin/app/components/modal-post-history.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
@lexical={{this.selectedRevision.lexical}}
@cardConfig={{this.cardConfig}}
@registerAPI={{this.registerSelectedEditorApi}}
@registerSecondaryAPI={{this.registerSecondarySelectedEditorApi}}
/>
</div>
</div>
Expand Down
7 changes: 0 additions & 7 deletions ghost/admin/app/components/modal-post-history.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ 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 @@ -102,11 +101,6 @@ 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 @@ -136,7 +130,6 @@ 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: 28 additions & 42 deletions ghost/admin/app/controllers/lexical-editor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,6 @@ 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 @@ -428,11 +423,6 @@ 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 @@ -1231,14 +1221,16 @@ 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 @@ -1253,32 +1245,37 @@ export default class LexicalEditorController extends Controller {
return true;
}

// Title scratch comparison
// titleScratch isn't an attr so needs a manual dirty check
if (post.titleScratch !== post.title) {
this._leaveModalReason = {reason: 'title is different', context: {current: post.title, scratch: post.titleScratch}};
return true;
}

// Lexical and scratch comparison
// scratch isn't an attr so needs a manual dirty check
let lexical = post.get('lexical');
let scratch = post.get('lexicalScratch');
let secondaryLexical = post.get('secondaryLexicalState');

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);

// Compare initLexical with scratch
let isSecondaryDirty = secondaryLexical && scratch && JSON.stringify(secondaryLexicalChildNodes) !== JSON.stringify(scratchChildNodes);
// 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;
}

// Compare lexical with scratch
let isLexicalDirty = lexical && scratch && JSON.stringify(lexicalChildNodes) !== JSON.stringify(scratchChildNodes);
this._leaveModalReason = {reason: 'lexical is different', context: {current: lexical, scratch}};
return true;
}
}

// New+unsaved posts always return `hasDirtyAttributes: true`
// 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 @@ -1289,26 +1286,15 @@ 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;
}

// 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;
return hasDirtyAttributes;
}

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

Expand Down Expand Up @@ -138,7 +136,6 @@
@updateSlugTask={{this.updateSlugTask}}
@savePostTask={{this.savePostTask}}
@editorAPI={{this.editorAPI}}
@secondaryEditorAPI={{this.secondaryEditorAPI}}
@toggleSettingsMenu={{this.toggleSettingsMenu}}
/>
{{/if}}
Expand Down
45 changes: 1 addition & 44 deletions ghost/admin/tests/unit/controllers/editor-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,7 @@ describe('Unit: Controller: lexical-editor', function () {
titleScratch: 'this is a title',
status: 'published',
lexical: initialLexicalString,
lexicalScratch: initialLexicalString,
secondaryLexicalState: initialLexicalString
lexicalScratch: initialLexicalString
}));

// synthetically update the lexicalScratch as if the editor itself made the modifications on loading the initial editorState
Expand All @@ -226,47 +225,5 @@ 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;
});
});
});
Loading