From 7758d331aa067dd163d73d5dfc78bf8a8946e854 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 11 Jun 2024 10:13:29 +0200 Subject: [PATCH 1/9] refactor(heading): move css to prosemirror.scss Signed-off-by: Max --- src/css/prosemirror.scss | 25 +++++++++++++++++++++++++ src/nodes/Heading/HeadingView.vue | 31 ------------------------------- 2 files changed, 25 insertions(+), 31 deletions(-) diff --git a/src/css/prosemirror.scss b/src/css/prosemirror.scss index 946cdc54ac2..b5343eece81 100644 --- a/src/css/prosemirror.scss +++ b/src/css/prosemirror.scss @@ -87,9 +87,34 @@ div.ProseMirror { } > h1,h2,h3,h4,h5,h6 { + position: relative; + &:first-child { margin-top: 0; } + + .heading-anchor[contenteditable="false"] { + // Shrink clickable area of anchor permalinks to not overlay the heading + width: 1em; + opacity: 0; + padding: 0; + left: -1em; + bottom: 0; + font-size: max(1em, 16px); + position: absolute; + text-decoration: none; + transition-duration: .15s; + transition-property: opacity; + transition-timing-function: cubic-bezier(.4,0,.2,1); + } + + &:hover .heading-anchor { + opacity: 0.5!important; + } + + &:focus-visible { + outline: none; + } } a { diff --git a/src/nodes/Heading/HeadingView.vue b/src/nodes/Heading/HeadingView.vue index d47c7d21a8e..804ef334032 100644 --- a/src/nodes/Heading/HeadingView.vue +++ b/src/nodes/Heading/HeadingView.vue @@ -81,34 +81,3 @@ export default { }, } - - From dbc163a8fe759fd6e10393bda0fa1d18506c40c3 Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 12 Jun 2024 10:11:01 +0200 Subject: [PATCH 2/9] fix: use decorations for heading anchors This avoids transactions that actually change the document state. Fixes #5861. Signed-off-by: Max --- cypress/e2e/sections.spec.js | 14 +-- src/components/Editor/TableOfContents.vue | 9 +- src/css/prosemirror.scss | 2 +- src/extensions/RichText.js | 2 +- src/nodes/Heading.js | 34 ++++++ src/nodes/Heading/HeadingView.vue | 83 --------------- src/nodes/Heading/extractor.js | 67 ------------ src/nodes/Heading/index.js | 69 ------------ src/plugins/extractHeadings.js | 48 +++++++++ src/plugins/headingAnchor.js | 122 ++++++++++++++++++++++ src/{nodes/Heading => plugins}/slug.js | 0 src/tests/plugins/extractHeadings.spec.js | 55 ++++++++++ 12 files changed, 270 insertions(+), 235 deletions(-) create mode 100644 src/nodes/Heading.js delete mode 100644 src/nodes/Heading/HeadingView.vue delete mode 100644 src/nodes/Heading/extractor.js delete mode 100644 src/nodes/Heading/index.js create mode 100644 src/plugins/extractHeadings.js create mode 100644 src/plugins/headingAnchor.js rename src/{nodes/Heading => plugins}/slug.js (100%) create mode 100644 src/tests/plugins/extractHeadings.spec.js diff --git a/cypress/e2e/sections.spec.js b/cypress/e2e/sections.spec.js index 2aa5a327965..bca3b00a5b7 100644 --- a/cypress/e2e/sections.spec.js +++ b/cypress/e2e/sections.spec.js @@ -47,7 +47,7 @@ describe('Content Sections', () => { cy.openFile(fileName, { force: true }) cy.getContent().type('# Heading 1{enter}') cy.getContent() - .find('h1') + .find('h1 > a') .should('have.attr', 'id') .and('equal', 'h-heading-1') cy.getContent() @@ -56,7 +56,7 @@ describe('Content Sections', () => { .and('equal', '#h-heading-1') cy.getContent().type('{backspace}{backspace}2{enter}') cy.getContent() - .find('h1') + .find('h1 > a') .should('have.attr', 'id') .and('equal', 'h-heading-2') cy.getContent() @@ -70,13 +70,13 @@ describe('Content Sections', () => { cy.visitTestFolder() cy.openFile('anchors.md') cy.getContent() - .get('h2[id="h-bottom"]') + .get('h2 > a[id="h-bottom"]') .should('not.be.inViewport') cy.getContent() .find('a[href="#h-bottom"]:not(.heading-anchor)') .click() cy.getContent() - .get('h2[id="h-bottom"]') + .get('h2 > a[id="h-bottom"]') .should('be.inViewport') }) @@ -87,15 +87,15 @@ describe('Content Sections', () => { cy.getContent() .type('# Heading 1{enter}') cy.getContent() - .find('h1') + .find('h1 > a') .should('have.attr', 'id') .and('equal', 'h-heading-1') cy.getContent() - .find('h1 [data-node-view-content]') + .find('h1') .click({ force: true, position: 'center' }) cy.getActionEntry('headings').click() cy.get('.v-popper__wrapper .open').getActionEntry('headings-h3').click() - cy.getContent().find('h3') + cy.getContent().find('h3 > a') .should('have.attr', 'id') .and('equal', 'h-heading-1') }) diff --git a/src/components/Editor/TableOfContents.vue b/src/components/Editor/TableOfContents.vue index 306ef67526a..61d681beb1e 100644 --- a/src/components/Editor/TableOfContents.vue +++ b/src/components/Editor/TableOfContents.vue @@ -2,7 +2,7 @@
  • - - - - - - diff --git a/src/nodes/Heading/extractor.js b/src/nodes/Heading/extractor.js deleted file mode 100644 index 63247dbdd32..00000000000 --- a/src/nodes/Heading/extractor.js +++ /dev/null @@ -1,67 +0,0 @@ -import store from '../../store/index.js' -import { slugify } from './slug.js' -import { v4 as uuidv4 } from 'uuid' - -const setHeadings = (val) => store.dispatch('text/setHeadings', val) - -const extractHeadings = (editor) => { - const counter = new Map() - const headings = [] - const tr = editor.state.tr - - const getId = text => { - const id = slugify(text) - - if (counter.has(id)) { - const next = counter.get(id) - - // increment counter - counter.set(id, next + 1) - - return `h-${id}--${next}` - } - - // define counter - counter.set(id, 1) - - return 'h-' + id - } - - editor.state.doc.descendants((node, position) => { - if (node.type.name === 'heading') { - const text = node.textContent - const id = getId(text) - const uuid = node.attrs.uuid ?? uuidv4() - - if (node.attrs.id !== id || !node.attrs.uuid) { - const attrs = { - ...node.attrs, - uuid, - id, - } - - tr.setNodeMarkup(position, undefined, attrs) - } - - headings.push(Object.freeze({ - level: node.attrs.level, - position, - text, - id, - uuid, - })) - } - }) - - tr.setMeta('addToHistory', false) - tr.setMeta('preventUpdate', true) - - editor.view.dispatch(tr) - - setHeadings(headings) -} - -export { - extractHeadings, - setHeadings, -} diff --git a/src/nodes/Heading/index.js b/src/nodes/Heading/index.js deleted file mode 100644 index bb5adac9ac2..00000000000 --- a/src/nodes/Heading/index.js +++ /dev/null @@ -1,69 +0,0 @@ -import TipTapHeading from '@tiptap/extension-heading' -import { VueNodeViewRenderer } from '@tiptap/vue-2' -import debounce from 'debounce' -import { extractHeadings } from './extractor.js' -import HeaderViewVue from './HeadingView.vue' - -const onUpdate = debounce(({ editor }) => { - if (editor.view && editor.state && !editor.isDestroyed) { - // Only run if editor still exists (prevent dangling debounced extractHeadings function) - extractHeadings(editor) - } -}, 900, { immediate: true }) - -const Heading = TipTapHeading.extend({ - addAttributes() { - return { - ...this.parent(), - id: { - default: undefined, - rendered: true, - }, - uuid: { - default: undefined, - rendered: false, - }, - } - }, - - addOptions() { - return { - ...this.parent?.(), - linkSymbol: '#', - } - }, - - addKeyboardShortcuts() { - return this.options.levels.reduce((items, level) => ({ - ...items, - [`Mod-Shift-${level}`]: () => this.editor.commands.toggleHeading({ level }), - }), {}) - }, - - addNodeView() { - return VueNodeViewRenderer(HeaderViewVue, { - update: ({ oldNode, newNode, updateProps }) => { - if (newNode.type.name !== this.name) return false - // Make sure to redraw node as the vue renderer will not show the updated children - if (newNode.attrs !== oldNode.attrs) return false - updateProps() - return true - }, - }) - }, - - onCreate() { - extractHeadings(this.editor) - - if (this.parent) { - this.parent() - } - }, - - onUpdate: (event) => { - onUpdate(event) - }, - -}) - -export default Heading diff --git a/src/plugins/extractHeadings.js b/src/plugins/extractHeadings.js new file mode 100644 index 00000000000..a59f9277869 --- /dev/null +++ b/src/plugins/extractHeadings.js @@ -0,0 +1,48 @@ +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { slugify } from './slug.js' + +/** + * Extract heading data structure from doc + * + * @param {Document} doc - the prosemirror doc + * @return {Array} headings found in the doc + */ +export default function extractHeadings(doc) { + const counter = new Map() + const headings = [] + + const getId = text => { + const id = slugify(text) + if (counter.has(id)) { + const next = counter.get(id) + // increment counter + counter.set(id, next + 1) + return `h-${id}--${next}` + } + // define counter + counter.set(id, 1) + return 'h-' + id + } + + doc.descendants((node, offset) => { + if (node.type.name !== 'heading') { + return + } + const text = node.textContent + // ignore empty headings + if (!text) return + const id = getId(text) + headings.push(Object.freeze({ + level: node.attrs.level, + text, + id, + offset, + })) + }) + + return headings +} diff --git a/src/plugins/headingAnchor.js b/src/plugins/headingAnchor.js new file mode 100644 index 00000000000..c6886f7bd57 --- /dev/null +++ b/src/plugins/headingAnchor.js @@ -0,0 +1,122 @@ +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { Plugin, PluginKey } from '@tiptap/pm/state' +import { Decoration, DecorationSet } from '@tiptap/pm/view' +import extractHeadings from './extractHeadings.js' + +export const headingAnchorPluginKey = new PluginKey('headingAnchor') + +/** + * Heading anchor decorations ProseMirror plugin + * Add anchor tags with a unique id to all headings. + * + * @return {Plugin} + */ +export default function headingAnchor() { + return new Plugin({ + + key: headingAnchorPluginKey, + + state: { + init(_, { doc }) { + const headings = extractHeadings(doc) + return { + headings, + decorations: anchorDecorations(doc, headings), + } + }, + apply(tr, value, _oldState, newState) { + if (!tr.docChanged) { + return value + } + const headings = extractHeadings(newState.doc) + // if the headings are the same, keep the decorations + if (sameHeadings(headings, value.headings)) { + return { + headings, + decorations: value.decorations.map(tr.mapping, tr.doc), + } + } + return { + headings, + decorations: anchorDecorations(newState.doc, headings), + } + }, + }, + + props: { + decorations(state) { + return this.getState(state).decorations + }, + }, + }) +} + +/** + * Check if the headings provided have the same ids. + * + * This is enough to ensure no updates are needed + * as the id includes a slugified version of the text + * and the level. + * + * @param {Array} current - array of headings + * @param {Array} other - headings to compare against + * + * @return {boolean} + */ +function sameHeadings(current, other) { + if (current.length !== other.length) return false + return current.every((heading, i) => heading.id === other[i].id) +} + +/** + * Create anchor decorations for the given headings + * @param {Document} doc - prosemirror doc + * @param {Array} headings - headings structure in the doc + * @return {DecorationSet} + */ +function anchorDecorations(doc, headings) { + const decorations = headings.map(decorationForHeading) + return DecorationSet.create(doc, decorations) +} + +/** + * Create a decoration for the given heading + * @param {object} heading to decorate + * @return {Decoration} + */ +function decorationForHeading(heading) { + return Decoration.widget(heading.offset + 1, anchorForHeading(heading)) +} + +/** + * Create an anchor element for the given heading + * @param {object} heading to generate anchor for + * @return {HTMLElement} + */ +function anchorForHeading(heading) { + const el = document.createElement('a') + const symbol = document.createTextNode('#') + el.appendChild(symbol) + el.setAttribute('id', heading.id) + el.setAttribute('aria-hidden', 'true') + el.className = 'heading-anchor' + el.setAttribute('href', `#${heading.id}`) + el.setAttribute('title', window.t('text', 'Link to this section')) + el.setAttribute('contenteditable', 'false') + el.addEventListener('click', handleClick) + return el +} + +/** + * Handle click on an anchor - scroll into view and change location hash. + * @param {PointerEvent} event click that triggered the function + */ +function handleClick(event) { + event.stopPropagation() + event.target.scrollIntoView() + window.location.hash = event.target.getAttribute('href') +} diff --git a/src/nodes/Heading/slug.js b/src/plugins/slug.js similarity index 100% rename from src/nodes/Heading/slug.js rename to src/plugins/slug.js diff --git a/src/tests/plugins/extractHeadings.spec.js b/src/tests/plugins/extractHeadings.spec.js new file mode 100644 index 00000000000..a5b61aab48c --- /dev/null +++ b/src/tests/plugins/extractHeadings.spec.js @@ -0,0 +1,55 @@ +/** + * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import extractHeadings from '../../plugins/extractHeadings.js' +import Heading from '../../nodes/Heading.js' +import { createCustomEditor } from '../helpers.js' + +describe('extractHeadings', () => { + + it('returns an empty array for an empty doc', () => { + const doc = prepareDoc('') + const headings = extractHeadings(doc) + expect(headings).toEqual([]) + }) + + it('returns headings', () => { + const content = '

    Level 1 heading

    ' + const doc = prepareDoc(content) + const headings = extractHeadings(doc) + expect(headings).toEqual([{ + id: 'h-level-1-heading', + level: 1, + offset: 0, + text: 'Level 1 heading', + }]) + }) + + it('ignores an empty heading', () => { + const content = '

    ' + const doc = prepareDoc(content) + const headings = extractHeadings(doc) + expect(headings).toEqual([]) + }) + + it('creates unique ids with a counter', () => { + const content = ` +

    Level 1 heading

    +

    Level 1 heading

    + ` + const doc = prepareDoc(content) + const headings = extractHeadings(doc) + expect(headings[1].id).toEqual('h-level-1-heading--1') + }) + +}) + +function prepareDoc(content) { + const editor = createCustomEditor({ + content, + extensions: [ Heading ] + }) + return editor.state.doc +} From 98520cfd489b6ab1d379c32eec1236fc03ac7f3f Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 13 Jun 2024 14:29:22 +0200 Subject: [PATCH 3/9] test(cy): test headings without node-view-content Signed-off-by: Max --- cypress/e2e/SmartPicker.spec.js | 4 ++-- cypress/e2e/versions.spec.js | 30 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/cypress/e2e/SmartPicker.spec.js b/cypress/e2e/SmartPicker.spec.js index 116f2a63cde..73a8735acb4 100644 --- a/cypress/e2e/SmartPicker.spec.js +++ b/cypress/e2e/SmartPicker.spec.js @@ -37,8 +37,8 @@ describe('Smart picker', () => { .type('Heading{enter}Hello World{enter}') cy.getContent() - .find('h1 [data-node-view-content]') - .should('have.text', 'Hello World') + .find('h1') + .should('contain.text', 'Hello World') }) it('Insert a link with the smart picker', () => { diff --git a/cypress/e2e/versions.spec.js b/cypress/e2e/versions.spec.js index 5da12bc6358..5241761d0f2 100644 --- a/cypress/e2e/versions.spec.js +++ b/cypress/e2e/versions.spec.js @@ -30,18 +30,18 @@ describe('Versions', () => { cy.get('[data-files-versions-versions-list] li a').eq(1).click() cy.get('.viewer__content #read-only-editor') - .find('h1 [data-node-view-content]') - .should('have.text', 'V2') + .find('h1') + .should('contain.text', 'V2') cy.get('[data-files-versions-versions-list] li a').eq(2).click() cy.get('.viewer__content #read-only-editor') - .find('h1 [data-node-view-content]') - .should('have.text', 'V1') + .find('h1') + .should('contain.text', 'V1') cy.get('[data-files-versions-versions-list] li a').eq(0).click() cy.getContent() - .find('h1 [data-node-view-content]') - .should('have.text', 'V3') + .find('h1') + .should('contain.text', 'V3') }) }) @@ -64,18 +64,18 @@ describe('Versions', () => { cy.get('[data-files-versions-versions-list] li a').eq(1).click() cy.get('.viewer__content #read-only-editor') - .find('h1 [data-node-view-content]') - .should('have.text', 'V2') + .find('h1') + .should('contain.text', 'V2') cy.get('[data-files-versions-versions-list] li a').eq(2).click() cy.get('.viewer__content #read-only-editor') - .find('h1 [data-node-view-content]') - .should('have.text', 'V1') + .find('h1') + .should('contain.text', 'V1') cy.get('[data-files-versions-versions-list] li a').eq(0).click() cy.getContent() - .find('h1 [data-node-view-content]') - .should('have.text', 'V3') + .find('h1') + .should('contain.text', 'V3') cy.getContent() .type('Hello') @@ -109,11 +109,11 @@ describe('Versions', () => { .click() cy.get('.viewer__content #read-only-editor') - .find('h1 [data-node-view-content]') - .should('have.text', 'V1') + .find('h1') + .should('contain.text', '#V1') cy.get('.viewer__content .viewer__file--active .ProseMirror') - .find('h1 [data-node-view-content]') + .find('h1') .should('contain.text', 'V3') }) }) From 460f5a8d30a3f6fe9cb44d078d73cf265ef9fb09 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 17 Jun 2024 12:15:59 +0200 Subject: [PATCH 4/9] fix(editor): only setEditable when needed This will trigger an update event on the editor which in turn will trigger all update handlers. Avoid this when not necessary. Signed-off-by: Max --- src/components/Editor.vue | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index db9857b25d1..3a5ac7490f6 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -571,7 +571,10 @@ export default { this.document = document this.syncError = null - this.$editor.setEditable(!this.readOnly) + const editable = !this.readOnly + if (this.$editor.isEditable !== editable) { + this.$editor.setEditable(editable) + } }, onSync({ steps, document }) { From e5930c264aefb9d1e898fe7d04bb051a79210018 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 18 Jun 2024 09:49:24 +0200 Subject: [PATCH 5/9] fix(heading): put anchor in front of cursor and first letter This allows typing even at the beginning of the heading as the cursor now is between the anchor and the first letter. Signed-off-by: Max --- src/plugins/headingAnchor.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/plugins/headingAnchor.js b/src/plugins/headingAnchor.js index c6886f7bd57..62fa4fdb791 100644 --- a/src/plugins/headingAnchor.js +++ b/src/plugins/headingAnchor.js @@ -89,7 +89,11 @@ function anchorDecorations(doc, headings) { * @return {Decoration} */ function decorationForHeading(heading) { - return Decoration.widget(heading.offset + 1, anchorForHeading(heading)) + return Decoration.widget( + heading.offset + 1, + anchorForHeading(heading), + { side: -1 }, + ) } /** From 13d3b8f2c39d7dbbf0bc6a33560670e5e049ff0b Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 18 Jun 2024 09:51:09 +0200 Subject: [PATCH 6/9] test(cy): do not wait for push after initialization The toc does not trigger a transaction anymore and therefore there is no push to wait for either. Also use cypress aliases and avoid deep nesting of cypress calls when possible. Signed-off-by: Max --- cypress/e2e/initial.spec.js | 201 +++++++++++++++++------------------- 1 file changed, 94 insertions(+), 107 deletions(-) diff --git a/cypress/e2e/initial.spec.js b/cypress/e2e/initial.spec.js index aac7fd98c69..c8bb0038688 100644 --- a/cypress/e2e/initial.spec.js +++ b/cypress/e2e/initial.spec.js @@ -39,123 +39,110 @@ describe('Test state loading of documents', function() { it('Initial content can not be undone', function() { cy.shareFile('/test.md', { edit: true }) - .then((token) => { - cy.visit(`/s/${token}`) - }) - .then(() => { - cy.getEditor().should('be.visible') - cy.getContent() - .should('contain', 'Hello world') - .find('h2').should('contain', 'Hello world') - - cy.getMenu().should('be.visible') - cy.getActionEntry('undo').should('be.disabled') - - cy.getContent() - .type('New content') - cy.getActionEntry('undo').should('not.be.disabled') - }) + .then(token => cy.visit(`/s/${token}`)) + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + + cy.getMenu().should('be.visible') + cy.getActionEntry('undo').should('be.disabled') + + cy.getContent() + .type('New content') + cy.getActionEntry('undo').should('not.be.disabled') }) it('Consecutive sessions work properly', function() { - let readToken = null - let writeToken = null cy.interceptCreate() cy.shareFile('/test2.md') - .then((token) => { - readToken = token - cy.logout() - cy.visit(`/s/${readToken}`) - cy.wait('@create') - }) - .then(() => { - // Open read only for the first time - cy.getEditor().should('be.visible') - cy.getContent() - .should('contain', 'Hello world') - .find('h2').should('contain', 'Hello world') - cy.closeInterceptedSession(readToken) - - // Open read only for the second time - cy.reload() - cy.getEditor().should('be.visible') - cy.getContent() - .should('contain', 'Hello world') - .find('h2').should('contain', 'Hello world') - cy.closeInterceptedSession(readToken) - - cy.login(user) - cy.shareFile('/test2.md', { edit: true }) - .then((token) => { - writeToken = token - // Open write link and edit something - cy.visit(`/s/${writeToken}`) - cy.getEditor().should('be.visible') - cy.getContent() - .should('contain', 'Hello world') - .find('h2').should('contain', 'Hello world') - cy.getContent() - .type('Something new {end}') - cy.intercept({ method: 'POST', url: '**/session/*/push' }).as('push') - cy.intercept({ method: 'POST', url: '**/session/*/sync' }).as('sync') - cy.wait('@push') - cy.wait('@sync') - cy.closeInterceptedSession(writeToken) - - // Reopen read only link and check if changes are there - cy.visit(`/s/${readToken}`) - cy.getEditor().should('be.visible') - cy.getContent() - .find('h2').should('contain', 'Something new Hello world') - }) - }) + .as('readToken') + cy.logout() + cy.get('@readToken') + .then(token => cy.visit(`/s/${token}`)) + cy.wait('@create') + // Open read only for the first time + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + cy.get('@readToken') + .then(cy.closeInterceptedSession) + + // Open read only for the second time + cy.reload() + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + cy.get('@readToken') + .then(cy.closeInterceptedSession) + + cy.login(user) + cy.shareFile('/test2.md', { edit: true }) + .as('writeToken') + // Open write link and edit something + cy.get('@writeToken') + .then(token => cy.visit(`/s/${token}`)) + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + cy.getContent() + .type('Something new {end}') + cy.intercept({ method: 'POST', url: '**/session/*/sync' }).as('sync') + cy.wait('@sync') + cy.get('@writeToken') + .then(cy.closeInterceptedSession) + + // Reopen read only link and check if changes are there + cy.get('@readToken') + .then(token => cy.visit(`/s/${token}`)) + cy.getEditor().should('be.visible') + cy.getContent() + .find('h2').should('contain', 'Something new Hello world') }) it('Load after state has been saved', function() { - let readToken = null - let writeToken = null cy.interceptCreate() cy.shareFile('/test3.md', { edit: true }) - .then((token) => { - writeToken = token - cy.logout() - cy.visit(`/s/${writeToken}`) - }) - .then(() => { - // Open a file, write and save - cy.getEditor().should('be.visible') - cy.getContent() - .should('contain', 'Hello world') - .find('h2').should('contain', 'Hello world') - cy.getContent() - .type('Something new {end}') - cy.intercept({ method: 'POST', url: '**/session/*/save' }).as('save') - cy.get('.save-status button').click() - cy.wait('@save', { timeout: 10000 }) - cy.closeInterceptedSession(writeToken) - - // Open writable file again and assert the content - cy.reload() - cy.getEditor().should('be.visible') - cy.getContent() - .should('contain', 'Hello world') - .find('h2').should('contain', 'Something new Hello world') - - cy.login(user) - cy.shareFile('/test3.md') - .then((token) => { - readToken = token - cy.logout() - cy.visit(`/s/${readToken}`) - }) - .then(() => { - // Open read only file again and assert the content - cy.getEditor().should('be.visible') - cy.getContent() - .should('contain', 'Hello world') - .find('h2').should('contain', 'Something new Hello world') - }) - }) + .as('writeToken') + cy.logout() + cy.get('@writeToken') + .then(token => cy.visit(`/s/${token}`)) + + // Open a file, write and save + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + cy.getContent() + .type('Something new {end}') + cy.intercept({ method: 'POST', url: '**/session/*/save' }).as('save') + cy.get('.save-status button').click() + cy.wait('@save', { timeout: 10000 }) + cy.get('@writeToken') + .then(cy.closeInterceptedSession) + + // Open writable file again and assert the content + cy.reload() + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Something new Hello world') + + cy.login(user) + cy.shareFile('/test3.md') + .as('readToken') + cy.logout() + cy.get('@readToken') + .then(token => cy.visit(`/s/${token}`)) + + // Open read only file again and assert the content + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Something new Hello world') }) }) From ce7d9c6f722ef23af6ca2ddaf7b5c20601819c86 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 18 Jun 2024 10:57:22 +0200 Subject: [PATCH 7/9] fix(headings): adjust decoration if heading level changes Signed-off-by: Max --- src/plugins/headingAnchor.js | 41 +++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/src/plugins/headingAnchor.js b/src/plugins/headingAnchor.js index 62fa4fdb791..3080173e93f 100644 --- a/src/plugins/headingAnchor.js +++ b/src/plugins/headingAnchor.js @@ -33,17 +33,10 @@ export default function headingAnchor() { return value } const headings = extractHeadings(newState.doc) - // if the headings are the same, keep the decorations - if (sameHeadings(headings, value.headings)) { - return { - headings, - decorations: value.decorations.map(tr.mapping, tr.doc), - } - } - return { - headings, - decorations: anchorDecorations(newState.doc, headings), - } + const decorations = headingsChanged(headings, value.headings) + ? anchorDecorations(newState.doc, headings) + : value.decorations.map(tr.mapping, tr.doc) + return { headings, decorations } }, }, @@ -56,20 +49,30 @@ export default function headingAnchor() { } /** - * Check if the headings provided have the same ids. - * - * This is enough to ensure no updates are needed - * as the id includes a slugified version of the text - * and the level. + * Check if the headings provided are equivalent. * * @param {Array} current - array of headings * @param {Array} other - headings to compare against * * @return {boolean} */ -function sameHeadings(current, other) { - if (current.length !== other.length) return false - return current.every((heading, i) => heading.id === other[i].id) +function headingsChanged(current, prev) { + return (current.length !== prev.length) + || current.some(isDifferentFrom(prev)) +} + +/** + * Check if headings are different - i.e. have different id or level + * + * @param {Array} other - headings to compare against + * + * Returns a function to be used in a call to Array#some. + * The returned function takes a heading and an index (as provided by iterators) + * and compares the id and level of the heading to the one in `other` with the same index. + */ +const isDifferentFrom = (other) => (heading, i) => { + return heading.id !== other[i].id + || heading.level !== other[i].level } /** From dc2ecd28782bdaf634c697b754a842fec5719206 Mon Sep 17 00:00:00 2001 From: Jonas Date: Thu, 25 Jul 2024 13:01:55 +0200 Subject: [PATCH 8/9] fix(headingAnchor): Rebuild decorationSet in case of replacements When the whole document gets replaced by the same content, `DecorationSet.map()` returns an empty decorationSet. So only use it for updates where no decorations get removed. Signed-off-by: Jonas --- cypress/e2e/conflict.spec.js | 4 ++++ src/plugins/headingAnchor.js | 30 ++++++++++++++++++++++++++---- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/cypress/e2e/conflict.spec.js b/cypress/e2e/conflict.spec.js index cf1e7484a05..dace4d00cf2 100644 --- a/cypress/e2e/conflict.spec.js +++ b/cypress/e2e/conflict.spec.js @@ -125,6 +125,10 @@ variants.forEach(function({ fixture, mime }) { }) }) +/** + * @param {string} fileName - filename + * @param {string} mime - mimetype + */ function createConflict(fileName, mime) { cy.visit('/apps/files') cy.openFile(fileName) diff --git a/src/plugins/headingAnchor.js b/src/plugins/headingAnchor.js index 3080173e93f..000b4c4ad20 100644 --- a/src/plugins/headingAnchor.js +++ b/src/plugins/headingAnchor.js @@ -22,6 +22,7 @@ export default function headingAnchor() { state: { init(_, { doc }) { + console.debug('headingAnchor init') const headings = extractHeadings(doc) return { headings, @@ -33,9 +34,7 @@ export default function headingAnchor() { return value } const headings = extractHeadings(newState.doc) - const decorations = headingsChanged(headings, value.headings) - ? anchorDecorations(newState.doc, headings) - : value.decorations.map(tr.mapping, tr.doc) + const decorations = mapDecorations(value, tr, headings) || anchorDecorations(newState.doc, headings) return { headings, decorations } }, }, @@ -48,11 +47,34 @@ export default function headingAnchor() { }) } +/** + * Map the previous decorations to current document state. + * + * Return false if headings changed or decorations would get removed. + * The latter prevents lost decorations in case of replacements. + * + * @param {object} value - previous plugin state + * @param {object} tr - current transaction + * @param {Array} headings - array of headings + * + * @return {false|DecorationSet} + */ +function mapDecorations(value, tr, headings) { + if (headingsChanged(headings, value.headings)) { + return false + } + let removedDecorations = false + const decorations = value.decorations.map(tr.mapping, tr.doc, { onRemove: () => { removedDecorations = true } }) + return removedDecorations + ? false + : decorations +} + /** * Check if the headings provided are equivalent. * * @param {Array} current - array of headings - * @param {Array} other - headings to compare against + * @param {Array} prev - headings to compare against * * @return {boolean} */ From 581c89502299c4a66efd0c446bb678cd4099a548 Mon Sep 17 00:00:00 2001 From: Jonas Date: Thu, 25 Jul 2024 16:18:03 +0200 Subject: [PATCH 9/9] fix(css): Use single quotes Signed-off-by: Jonas --- src/css/prosemirror.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/css/prosemirror.scss b/src/css/prosemirror.scss index 9d0d3433b92..6e6c1e3176c 100644 --- a/src/css/prosemirror.scss +++ b/src/css/prosemirror.scss @@ -93,7 +93,7 @@ div.ProseMirror { margin-top: 0; } - .heading-anchor[contenteditable="false"] { + .heading-anchor[contenteditable='false'] { // Shrink clickable area of anchor permalinks to not overlay the heading width: 1em; opacity: 0;