From b90a514c79dbc9f71d11fe9fe041f4f0187f14ad Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 8 Oct 2024 09:52:33 +0200 Subject: [PATCH 1/6] refactor(EditorFactory): split createEditor in rich and plain variant Use two functions that do one thing rather than one with a boolean flag. Simplifies the functions use and signature a lot in particular since plain and rich editor need different arguments. Signed-off-by: Max --- src/EditorFactory.js | 38 ++++++++------- src/components/Editor.vue | 89 +++++++++++++++++++---------------- src/mixins/setContent.js | 9 ++-- src/tests/builders.js | 9 ++-- src/tests/helpers.js | 14 ++---- src/tests/markdown.spec.js | 6 +-- src/tests/nodes/Table.spec.js | 6 +-- src/tests/plaintext.spec.js | 6 +-- src/tests/tiptap.spec.js | 6 +-- 9 files changed, 90 insertions(+), 93 deletions(-) diff --git a/src/EditorFactory.js b/src/EditorFactory.js index 586654a4eaf..9bdc9c3445b 100644 --- a/src/EditorFactory.js +++ b/src/EditorFactory.js @@ -32,10 +32,15 @@ const loadSyntaxHighlight = async (language) => { } } -const createEditor = ({ language, onCreate = () => {}, onUpdate = () => {}, extensions, enableRichEditing, session, relativePath, isEmbedded = false }) => { - let defaultExtensions - if (enableRichEditing) { - defaultExtensions = [ +const editorProps = { + scrollMargin: 50, + scrollThreshold: 50, +} + +const createRichEditor = ({ extensions = [], session, relativePath, isEmbedded = false } = {}) => { + return new Editor({ + editorProps, + extensions: [ RichText.configure({ relativePath, isEmbedded, @@ -49,19 +54,19 @@ const createEditor = ({ language, onCreate = () => {}, onUpdate = () => {}, exte ], }), FocusTrap, - ] - } else { - defaultExtensions = [PlainText, CodeBlockLowlight.configure({ lowlight, defaultLanguage: language })] - } + ...extensions, + ], + }) +} +const createPlainEditor = ({ language, extensions = [] } = {}) => { return new Editor({ - onCreate, - onUpdate, - editorProps: { - scrollMargin: 50, - scrollThreshold: 50, - }, - extensions: defaultExtensions.concat(extensions || []), + editorProps, + extensions: [ + PlainText, + CodeBlockLowlight.configure({ lowlight, defaultLanguage: language }), + ...extensions, + ], }) } @@ -69,5 +74,4 @@ const serializePlainText = (doc) => { return doc.textContent } -export default createEditor -export { createEditor, serializePlainText, loadSyntaxHighlight } +export { createRichEditor, createPlainEditor, serializePlainText, loadSyntaxHighlight } diff --git a/src/components/Editor.vue b/src/components/Editor.vue index df97144a5f3..345f820342f 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -98,7 +98,12 @@ import { SyncService, ERROR_TYPE, IDLE_TIMEOUT } from './../services/SyncService import createSyncServiceProvider from './../services/SyncServiceProvider.js' import AttachmentResolver from './../services/AttachmentResolver.js' import { extensionHighlight } from '../helpers/mappings.js' -import { createEditor, serializePlainText, loadSyntaxHighlight } from './../EditorFactory.js' +import { + createRichEditor, + createPlainEditor, + serializePlainText, + loadSyntaxHighlight, +} from './../EditorFactory.js' import { createMarkdownSerializer } from './../extensions/Markdown.js' import markdownit from './../markdownit/index.js' @@ -401,10 +406,15 @@ export default { listenEditorEvents() { this.$editor.on('focus', this.onFocus) this.$editor.on('blur', this.onBlur) + this.$editor.on('create', this.onCreate) + this.$editor.on('update', this.onUpdate) }, + unlistenEditorEvents() { this.$editor.off('focus', this.onFocus) this.$editor.off('blur', this.onBlur) + this.$editor.off('create', this.onCreate) + this.$editor.off('update', this.onUpdate) }, listenSyncServiceEvents() { @@ -510,45 +520,28 @@ export default { .then(() => { const session = this.currentSession if (!this.$editor) { - this.$editor = createEditor({ - language, - relativePath: this.relativePath, - session, - onCreate: ({ editor }) => { - this.$syncService.startSync() - const proseMirrorMarkdown = this.$syncService.serialize(editor.state.doc) - this.emit('create:content', { - markdown: proseMirrorMarkdown, - }) - }, - onUpdate: ({ editor }) => { - // this.debugContent(editor) - const proseMirrorMarkdown = this.$syncService.serialize(editor.state.doc) - this.emit('update:content', { - markdown: proseMirrorMarkdown, - }) - }, - extensions: [ - Autofocus.configure({ - fileId: this.fileId, - }), - Collaboration.configure({ - document: this.$ydoc, - }), - CollaborationCursor.configure({ - provider: this.$providers[0], - user: { - name: session?.userId - ? session.displayName - : (session?.guestName || t('text', 'Guest')), - color: session?.color, - clientId: this.$ydoc.clientID, - }, - }), - ], - enableRichEditing: this.isRichEditor, - isEmbedded: this.isEmbedded, - }) + const extensions = [ + Autofocus.configure({ fileId: this.fileId }), + Collaboration.configure({ document: this.$ydoc }), + CollaborationCursor.configure({ + provider: this.$providers[0], + user: { + name: session?.userId + ? session.displayName + : (session?.guestName || t('text', 'Guest')), + color: session?.color, + clientId: this.$ydoc.clientID, + }, + }), + ] + this.$editor = this.isRichEditor + ? createRichEditor({ + relativePath: this.relativePath, + session, + extensions, + isEmbedded: this.isEmbedded, + }) + : createPlainEditor({ language, extensions }) this.hasEditor = true this.listenEditorEvents() } else { @@ -571,6 +564,22 @@ export default { } }, + onCreate({ editor }) { + this.$syncService.startSync() + const proseMirrorMarkdown = this.$syncService.serialize(editor.state.doc) + this.emit('create:content', { + markdown: proseMirrorMarkdown, + }) + }, + + onUpdate({ editor }) { + // this.debugContent(editor) + const proseMirrorMarkdown = this.$syncService.serialize(editor.state.doc) + this.emit('update:content', { + markdown: proseMirrorMarkdown, + }) + }, + onSync({ steps, document }) { this.hasConnectionIssue = this.$syncService.backend.fetcher === 0 || !this.$providers[0].wsconnected || this.$syncService.pushError > 0 this.$nextTick(() => { diff --git a/src/mixins/setContent.js b/src/mixins/setContent.js index addf6aee02c..5af31521720 100644 --- a/src/mixins/setContent.js +++ b/src/mixins/setContent.js @@ -9,7 +9,7 @@ import { Doc, encodeStateAsUpdate, XmlFragment, applyUpdate } from 'yjs' import { generateJSON } from '@tiptap/core' import { prosemirrorToYXmlFragment } from 'y-prosemirror' import { Node } from '@tiptap/pm/model' -import { createEditor } from '../EditorFactory.js' +import { createRichEditor, createPlainEditor } from '../EditorFactory.js' export default { methods: { @@ -31,9 +31,10 @@ export default { ? markdownit.render(content) + '

' : `

${escapeHtml(content)}
` - const editor = createEditor({ - enableRichEditing: isRichEditor, - }) + const editor = isRichEditor + ? createRichEditor() + : createPlainEditor() + const json = generateJSON(html, editor.extensionManager.extensions) const doc = Node.fromJSON(editor.schema, json) diff --git a/src/tests/builders.js b/src/tests/builders.js index c0e8da5c4a5..ce209b26866 100644 --- a/src/tests/builders.js +++ b/src/tests/builders.js @@ -6,14 +6,11 @@ import { expect } from '@jest/globals'; import { Mark, Node } from '@tiptap/pm/model' import { builders } from 'prosemirror-test-builder' -import createEditor from '../EditorFactory' +import { createRichEditor } from '../EditorFactory' export function getBuilders() { - const editor = createEditor({ - content: '', - enableRichEditing: true - }) + const editor = createRichEditor() return builders(editor.schema, { tr: { nodeType: 'tableRow' }, td: { nodeType: 'tableCell' }, @@ -84,7 +81,7 @@ function createDocumentString(node) { * @param {Node} subject The editor document * @param {Node} expected The expected document * @example - * const editor = createEditor() + * const editor = createRichEditor() * expectDocument(editor.state.doc, table( * tr( * td('foo') diff --git a/src/tests/helpers.js b/src/tests/helpers.js index a1c980439bf..53a44ac18fb 100644 --- a/src/tests/helpers.js +++ b/src/tests/helpers.js @@ -10,7 +10,7 @@ import Document from '@tiptap/extension-document' import Paragraph from '../nodes/Paragraph' import Text from '@tiptap/extension-text' -import createEditor from '../EditorFactory' +import { createRichEditor } from '../EditorFactory' import markdownit from '../markdownit' export function createCustomEditor({ content, extensions }) { @@ -32,9 +32,7 @@ export function createCustomEditor({ content, extensions }) { * @returns {string} */ export function markdownThroughEditor(markdown) { - const tiptap = createEditor({ - enableRichEditing: true - }) + const tiptap = createRichEditor() tiptap.commands.setContent(markdownit.render(markdown)) const serializer = createMarkdownSerializer(tiptap.schema) return serializer.serialize(tiptap.state.doc) @@ -47,9 +45,7 @@ export function markdownThroughEditor(markdown) { * @returns {string} */ export function markdownThroughEditorHtml(html) { - const tiptap = createEditor({ - enableRichEditing: true - }) + const tiptap = createRichEditor() tiptap.commands.setContent(html) const serializer = createMarkdownSerializer(tiptap.schema) return serializer.serialize(tiptap.state.doc) @@ -62,9 +58,7 @@ export function markdownThroughEditorHtml(html) { * @returns {string} */ export function markdownFromPaste(html) { - const tiptap = createEditor({ - enableRichEditing: true - }) + const tiptap = createRichEditor() tiptap.commands.insertContent(html) const serializer = createMarkdownSerializer(tiptap.schema) return serializer.serialize(tiptap.state.doc) diff --git a/src/tests/markdown.spec.js b/src/tests/markdown.spec.js index 6f1b6258a01..f15cec281c7 100644 --- a/src/tests/markdown.spec.js +++ b/src/tests/markdown.spec.js @@ -11,7 +11,7 @@ import { markdownFromPaste } from './helpers.js' import { createMarkdownSerializer } from "../extensions/Markdown"; -import createEditor from "../EditorFactory"; +import { createRichEditor } from "../EditorFactory"; /* * This file is for various markdown tests, mainly testing if input and output stays the same. @@ -212,9 +212,7 @@ describe('Markdown serializer from html', () => { describe('Trailing nodes', () => { test('No extra transaction is added after loading', () => { const source = "# My heading\n\n* test\n* test2" - const tiptap = createEditor({ - enableRichEditing: true, - }) + const tiptap = createRichEditor() tiptap.commands.setContent(markdownit.render(source)) const jsonBefore = tiptap.getJSON() diff --git a/src/tests/nodes/Table.spec.js b/src/tests/nodes/Table.spec.js index 19c843f3494..06dabbea0ef 100644 --- a/src/tests/nodes/Table.spec.js +++ b/src/tests/nodes/Table.spec.js @@ -3,7 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { createEditor } from '../../EditorFactory' +import { createRichEditor } from '../../EditorFactory' import { createMarkdownSerializer } from '../../extensions/Markdown' import { builders } from 'prosemirror-test-builder' @@ -75,9 +75,7 @@ describe('Table', () => { }) function editorWithContent(content) { - const editor = createEditor({ - enableRichEditing: true, - }) + const editor = createRichEditor() editor.commands.setContent(content) return editor } diff --git a/src/tests/plaintext.spec.js b/src/tests/plaintext.spec.js index 20c89bb0438..ccf7f765095 100644 --- a/src/tests/plaintext.spec.js +++ b/src/tests/plaintext.spec.js @@ -3,7 +3,7 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { createEditor, serializePlainText } from './../EditorFactory'; +import { createPlainEditor, serializePlainText } from './../EditorFactory'; import spec from "./fixtures/spec" import xssFuzzVectors from './fixtures/xssFuzzVectors'; @@ -18,9 +18,7 @@ const escapeHTML = (s) => { const plaintextThroughEditor = (markdown) => { const content = '
' + escapeHTML(markdown) + '
' - const tiptap = createEditor({ - enableRichEditing: false - }) + const tiptap = createPlainEditor() tiptap.commands.setContent(content) return serializePlainText(tiptap.state.doc) || 'failed' } diff --git a/src/tests/tiptap.spec.js b/src/tests/tiptap.spec.js index 4b6f4244496..b1adc70ed23 100644 --- a/src/tests/tiptap.spec.js +++ b/src/tests/tiptap.spec.js @@ -3,13 +3,11 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import createEditor from '../EditorFactory' +import { createRichEditor } from '../EditorFactory' import markdownit from '../markdownit' const renderedHTML = ( markdown ) => { - const editor = createEditor({ - enableRichEditing: true - }) + const editor = createRichEditor() editor.commands.setContent(markdownit.render(markdown)) // Remove TrailingNode return editor.getHTML().replace(/

<\/p>$/, '') From 576af091244fef8da0d298c25aae216daed71449 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 8 Oct 2024 09:56:54 +0200 Subject: [PATCH 2/6] fix(lint): use const arrow function for helper This does not require jsdoc comments. Signed-off-by: Max --- cypress/e2e/nodes/Links.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/e2e/nodes/Links.spec.js b/cypress/e2e/nodes/Links.spec.js index 40518b7b937..0bcf40d6f64 100644 --- a/cypress/e2e/nodes/Links.spec.js +++ b/cypress/e2e/nodes/Links.spec.js @@ -27,7 +27,7 @@ describe('test link marks', function() { describe('link bubble', function() { - function clickLink(link, options = {}) { + const clickLink = (link, options = {}) => { cy.getContent() .find(`a[href*="${link}"]`) .click(options) From 2b815e9ebbed905550b81bbfeeca8b602ad791a6 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 8 Oct 2024 10:36:30 +0200 Subject: [PATCH 3/6] fix(plaintext): allow adding multiple empty lines at the end Pressing enter three times exits a CodeBlock element by default. Disable this by setting `exitOnTripleEnter` to false in plaintext editor. Signed-off-by: Max --- src/EditorFactory.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/EditorFactory.js b/src/EditorFactory.js index 9bdc9c3445b..86feee804ca 100644 --- a/src/EditorFactory.js +++ b/src/EditorFactory.js @@ -64,7 +64,11 @@ const createPlainEditor = ({ language, extensions = [] } = {}) => { editorProps, extensions: [ PlainText, - CodeBlockLowlight.configure({ lowlight, defaultLanguage: language }), + CodeBlockLowlight.configure({ + lowlight, + defaultLanguage: language, + exitOnTripleEnter: false, + }), ...extensions, ], }) From 3e689ee2a54a5d73b2aeef88ba3f8263f1ebbd2d Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 8 Oct 2024 11:05:34 +0200 Subject: [PATCH 4/6] refactor(Editor): pull check for $editor up Signed-off-by: Max --- src/components/Editor.vue | 61 +++++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 345f820342f..4b21b6e03e1 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -514,41 +514,40 @@ export default { this.$baseVersionEtag = document.baseVersionEtag this.hasConnectionIssue = false - const language = extensionHighlight[this.fileExtension] || this.fileExtension; + if (this.$editor) { + // $editor already existed. So this is a reconnect. + this.$syncService.startSync() + return + } + const language = extensionHighlight[this.fileExtension] || this.fileExtension; (this.isRichEditor ? Promise.resolve() : loadSyntaxHighlight(language)) .then(() => { const session = this.currentSession - if (!this.$editor) { - const extensions = [ - Autofocus.configure({ fileId: this.fileId }), - Collaboration.configure({ document: this.$ydoc }), - CollaborationCursor.configure({ - provider: this.$providers[0], - user: { - name: session?.userId - ? session.displayName - : (session?.guestName || t('text', 'Guest')), - color: session?.color, - clientId: this.$ydoc.clientID, - }, - }), - ] - this.$editor = this.isRichEditor - ? createRichEditor({ - relativePath: this.relativePath, - session, - extensions, - isEmbedded: this.isEmbedded, - }) - : createPlainEditor({ language, extensions }) - this.hasEditor = true - this.listenEditorEvents() - } else { - // $editor already existed. So this is a reconnect. - this.$syncService.startSync() - } - + const extensions = [ + Autofocus.configure({ fileId: this.fileId }), + Collaboration.configure({ document: this.$ydoc }), + CollaborationCursor.configure({ + provider: this.$providers[0], + user: { + name: session?.userId + ? session.displayName + : (session?.guestName || t('text', 'Guest')), + color: session?.color, + clientId: this.$ydoc.clientID, + }, + }), + ] + this.$editor = this.isRichEditor + ? createRichEditor({ + relativePath: this.relativePath, + session, + extensions, + isEmbedded: this.isEmbedded, + }) + : createPlainEditor({ language, extensions }) + this.hasEditor = true + this.listenEditorEvents() }) }, From 07e4bd0167a6f52d3769fb5a97694fd9b3dc9671 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 8 Oct 2024 11:14:20 +0200 Subject: [PATCH 5/6] refactor(Editor): separate rich and plain creation Signed-off-by: Max --- src/components/Editor.vue | 61 +++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 4b21b6e03e1..73b223a2a99 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -520,35 +520,40 @@ export default { return } - const language = extensionHighlight[this.fileExtension] || this.fileExtension; - (this.isRichEditor ? Promise.resolve() : loadSyntaxHighlight(language)) - .then(() => { - const session = this.currentSession - const extensions = [ - Autofocus.configure({ fileId: this.fileId }), - Collaboration.configure({ document: this.$ydoc }), - CollaborationCursor.configure({ - provider: this.$providers[0], - user: { - name: session?.userId - ? session.displayName - : (session?.guestName || t('text', 'Guest')), - color: session?.color, - clientId: this.$ydoc.clientID, - }, - }), - ] - this.$editor = this.isRichEditor - ? createRichEditor({ - relativePath: this.relativePath, - session, - extensions, - isEmbedded: this.isEmbedded, - }) - : createPlainEditor({ language, extensions }) - this.hasEditor = true - this.listenEditorEvents() + const session = this.currentSession + const extensions = [ + Autofocus.configure({ fileId: this.fileId }), + Collaboration.configure({ document: this.$ydoc }), + CollaborationCursor.configure({ + provider: this.$providers[0], + user: { + name: session?.userId + ? session.displayName + : (session?.guestName || t('text', 'Guest')), + color: session?.color, + clientId: this.$ydoc.clientID, + }, + }), + ] + if (this.isRichEditor) { + this.$editor = createRichEditor({ + relativePath: this.relativePath, + session, + extensions, + isEmbedded: this.isEmbedded, }) + this.hasEditor = true + this.listenEditorEvents() + } else { + const language = extensionHighlight[this.fileExtension] + || this.fileExtension + loadSyntaxHighlight(language) + .then(() => { + this.$editor = createPlainEditor({ language, extensions }) + this.hasEditor = true + this.listenEditorEvents() + }) + } }, From 995642e135d8d42d1fd89a76bb38a6155e08f11c Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 8 Oct 2024 11:47:14 +0200 Subject: [PATCH 6/6] test(plaintext) add regression test for newlines Signed-off-by: Max --- src/tests/plaintext.spec.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/tests/plaintext.spec.js b/src/tests/plaintext.spec.js index ccf7f765095..8fd957236fc 100644 --- a/src/tests/plaintext.spec.js +++ b/src/tests/plaintext.spec.js @@ -82,4 +82,14 @@ describe('html as plain text', () => { expect(plaintextThroughEditor('"\';&.-#><')).toBe('"\';&.-#><') expect(plaintextThroughEditor(xssFuzzVectors)).toBe(xssFuzzVectors) }) +} ) + +describe('regression tests', () => { + test('tripple enter creates new lines at end (#6507)', () => { + const tiptap = createPlainEditor() + tiptap.commands.enter() + tiptap.commands.enter() + tiptap.commands.enter() + expect(serializePlainText(tiptap.state.doc)).toEqual("\n\n\n") + }) })