OpenSourceArk/Ghost
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixed draft posts not autosaving without title (#20774)

Browse Source 
ref [ENG-661](https://linear.app/tryghost/issue/ENG-661/) ONC-253

- Reverts the revert of
93cbb94b90
of the intial bug fix.
- Updated hasDirtyAttributes logic to ensure the dirty state changes
when typing a draft, despite not title.
- Updated tests and added tests missing from the hasDirtyAttributes
logic
This commit is contained in:
Ronald Langeveld 2024-08-19 12:21:35 +07:00 committed by GitHub
parent f4794a5e18
commit aad438ba0b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
11 changed files with 256 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
ghost/admin/app/components/gh-koenig-editor-lexical.hbs
View File

@ -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}}

7
ghost/admin/app/components/gh-koenig-editor-lexical.js
View File

@ -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

1
ghost/admin/app/components/gh-post-settings-menu.hbs
View File

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

57
ghost/admin/app/components/koenig-lexical-editor.js
View File

@ -678,34 +678,43 @@ 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>}>
<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>

1
ghost/admin/app/components/modal-post-history.hbs
View File

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

7
ghost/admin/app/components/modal-post-history.js
View File

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

64
ghost/admin/app/controllers/lexical-editor.js
View File

@ -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);
@ -1229,8 +1239,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,53 +1254,58 @@ 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);
// Compare lexical with scratch
let isLexicalDirty = lexical && scratch && JSON.stringify(lexicalChildNodes) !== JSON.stringify(scratchChildNodes);
// If either comparison is not dirty, return false, because scratch is always up to date.
if (!isSecondaryDirty || !isLexicalDirty) {
return false;
}
// new+unsaved posts always return `hasDirtyAttributes: true`
// 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;
}
// 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());
let changedAttributes = Object.keys(post.changedAttributes() || {});
if (changedAttributes.length) {
this._leaveModalReason = {reason: 'post.changedAttributes.length > 0', context: post.changedAttributes()};
}
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;

3
ghost/admin/app/models/post.js
View File

@ -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.

3
ghost/admin/app/templates/lexical-editor.hbs
View File

@ -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}}

156
ghost/admin/tests/unit/controllers/editor-test.js
View File

@ -195,6 +195,60 @@ describe('Unit: Controller: lexical-editor', function () {
});
describe('hasDirtyAttributes', function () {
it('detects new post with changed attributes as dirty (autosave)', 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 updated","type": "extended-text","version": 1}],"direction": null,"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({
isNew: true,
title: '',
titleScratch: '',
status: 'draft',
lexical: initialLexicalString,
lexicalScratch: lexicalScratch,
secondaryLexicalState: initialLexicalString
}));
let isDirty = controller.get('hasDirtyAttributes');
expect(isDirty).to.be.true;
});
it('does not detect new post as dirty when there are no changes', 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}}`;
let controller = this.owner.lookup('controller:lexical-editor');
controller.set('post', EmberObject.create({
isNew: true,
title: '',
titleScratch: '',
status: 'draft',
lexical: initialLexicalString,
lexicalScratch: initialLexicalString,
secondaryLexicalState: initialLexicalString
}));
let isDirty = controller.get('hasDirtyAttributes');
expect(isDirty).to.be.false;
});
it('marks isNew post as dirty when lexicalScratch differs from lexical and secondaryLexical', 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 scratch","type": "extended-text","version": 1}],"direction": null,"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({
isNew: true,
title: '',
titleScratch: '',
status: 'draft',
lexical: initialLexicalString,
lexicalScratch: lexicalScratch,
secondaryLexicalState: initialLexicalString,
changedAttributes: () => ({title: ['', 'New Title']})
}));
let isDirty = controller.get('hasDirtyAttributes');
expect(isDirty).to.be.true;
});
it('Changes in the direction field in the lexical string are not considered dirty', async function () {
let controller = this.owner.lookup('controller:lexical-editor');
@ -208,7 +262,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 +280,104 @@ 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;
});
it('dirty is false if no Post', async function () {
let controller = this.owner.lookup('controller:lexical-editor');
controller.set('post', null);
let isDirty = controller.get('hasDirtyAttributes');
expect(isDirty).to.be.false;
});
it('returns true if current tags differ from previous tags', async function () {
let controller = this.owner.lookup('controller:lexical-editor');
controller.set('post', EmberObject.create({
tags: [{name: 'test'}],
tagsString: 'test',
changedAttributes: () => ({tags: [[{name: 'test'}], [{name: 'changed'}]]})
}));
let isDirty = controller.get('hasDirtyAttributes');
expect(isDirty).to.be.true;
});
it('returns false when the post is new but has no changed attributes', async function () {
let controller = this.owner.lookup('controller:lexical-editor');
controller.set('post', EmberObject.create({
isNew: true,
title: '',
titleScratch: '',
status: 'draft',
lexical: '',
lexicalScratch: '',
secondaryLexicalState: '',
changedAttributes: () => ({}) // no changes
}));
let isDirty = controller.get('hasDirtyAttributes');
expect(isDirty).to.be.false;
});
it('skips new post check if post is not new', async function () {
let controller = this.owner.lookup('controller:lexical-editor');
controller.set('post', EmberObject.create({
isNew: false,
title: 'Sample Title',
titleScratch: 'Sample Title',
status: 'draft',
lexical: '',
lexicalScratch: '',
secondaryLexicalState: '',
changedAttributes: () => ({})
}));
let isDirty = controller.get('hasDirtyAttributes');
// The test passes if no errors occur and it doesn't return true for new post condition
expect(isDirty).to.be.false;
});
});
});

9
ghost/core/test/e2e-browser/admin/publishing.spec.js
View File

@ -298,11 +298,12 @@ test.describe('Publishing', () => {
test('Renders secondary hidden lexical editor', async ({sharedPage: adminPage}) => {
await adminPage.goto('/ghost');
await createPostDraft(adminPage, {title: 'Secondary lexical editor test', body: 'This is my post body.'});
// Check if the secondary lexical editor exists but is hidden.
expect(await adminPage.locator('[data-secondary-instance="true"]')).toBeHidden();
const secondaryLexicalEditor = adminPage.locator('[data-secondary-instance="true"]');
// Check if the secondary lexical editor exists
await expect(secondaryLexicalEditor).toHaveCount(1);
// Check if it is hidden
await expect(secondaryLexicalEditor).toBeHidden();
});
});