diff --git a/services/web/cypress/support/shared/commands/index.ts b/services/web/cypress/support/shared/commands/index.ts index a75aa1afbb..676906bf75 100644 --- a/services/web/cypress/support/shared/commands/index.ts +++ b/services/web/cypress/support/shared/commands/index.ts @@ -20,7 +20,6 @@ declare global { interceptSpelling: typeof interceptSpelling waitForCompile: typeof waitForCompile interceptDeferredCompile: typeof interceptDeferredCompile - index: () => Chainable } } } @@ -31,6 +30,3 @@ Cypress.Commands.add('interceptEvents', interceptEvents) Cypress.Commands.add('interceptSpelling', interceptSpelling) Cypress.Commands.add('waitForCompile', waitForCompile) Cypress.Commands.add('interceptDeferredCompile', interceptDeferredCompile) -Cypress.Commands.add('index', { prevSubject: true }, subject => { - return cy.wrap(subject).invoke('index') -}) diff --git a/services/web/frontend/js/features/source-editor/extensions/changes/change-manager.ts b/services/web/frontend/js/features/source-editor/extensions/changes/change-manager.ts index 2b1cd0db34..5fa7863312 100644 --- a/services/web/frontend/js/features/source-editor/extensions/changes/change-manager.ts +++ b/services/web/frontend/js/features/source-editor/extensions/changes/change-manager.ts @@ -342,7 +342,10 @@ export const createChangeManager = ( view.contentDOM.clientHeight - padding.top - padding.bottom const paddingNeeded = height - contentHeight - if (overflowTop !== padding.top || paddingNeeded !== padding.bottom) { + if ( + overflowTop !== editorVerticalTopPadding(view) || + paddingNeeded !== padding.bottom + ) { view.dispatch( setVerticalOverflow({ top: overflowTop, @@ -456,7 +459,7 @@ export const createChangeManager = ( (update.geometryChanged || update.viewportChanged) && Date.now() < ignoreGeometryChangesUntil ) { - // Ignore a change to the editor geometry that occurs immediately after + // Ignore a change to the editor geometry or viewport that occurs immediately after // an update to the vertical padding because otherwise it triggers // another update to the padding and so on ad infinitum. This is not an // ideal way to handle this but I couldn't see another way. diff --git a/services/web/frontend/js/features/source-editor/extensions/vertical-overflow.ts b/services/web/frontend/js/features/source-editor/extensions/vertical-overflow.ts index 8ab63fd358..e17d82edaf 100644 --- a/services/web/frontend/js/features/source-editor/extensions/vertical-overflow.ts +++ b/services/web/frontend/js/features/source-editor/extensions/vertical-overflow.ts @@ -5,7 +5,13 @@ import { StateField, TransactionSpec, } from '@codemirror/state' -import { EditorView, ViewPlugin, ViewUpdate } from '@codemirror/view' +import { + Decoration, + EditorView, + ViewPlugin, + ViewUpdate, + WidgetType, +} from '@codemirror/view' export function verticalOverflow(): Extension { return [ @@ -14,6 +20,7 @@ export function verticalOverflow(): Extension { bottomPadding, topPadding, contentAttributes, + topPaddingDecoration, bottomPaddingPlugin, topPaddingPlugin, ] @@ -155,16 +162,54 @@ const bottomPadding = bottomPaddingFacet.computeN( } ) -// Set a style attribute on the contentDOM containing the calculated top and bottom padding. +// Set a style attribute on the contentDOM containing the calculated bottom padding. // This value will be concatenated with style values from any other extensions. -// TODO: use elements instead? const contentAttributes = EditorView.contentAttributes.compute( - [topPaddingFacet, bottomPaddingFacet], + [bottomPaddingFacet], + state => { + const [bottom] = state.facet(bottomPaddingFacet) + const style = `padding-bottom: ${bottom}px;` + return { style } + } +) + +class TopPaddingWidget extends WidgetType { + constructor(private readonly height: number) { + super() + this.height = height + } + + toDOM(view: EditorView): HTMLElement { + const element = document.createElement('div') + element.style.height = this.height + 'px' + return element + } + + get estimatedHeight() { + return this.height + } + + eq(widget: TopPaddingWidget) { + return this.height === widget.height + } + + updateDOM(element: HTMLElement): boolean { + element.style.height = this.height + 'px' + return true + } +} + +const topPaddingDecoration = EditorView.decorations.compute( + [topPaddingFacet], state => { const [top] = state.facet(topPaddingFacet) - const [bottom] = state.facet(bottomPaddingFacet) - const style = `padding-top: ${top}px; padding-bottom: ${bottom}px;` - return { style } + + return Decoration.set([ + Decoration.widget({ + widget: new TopPaddingWidget(top), + block: true, + }).range(0), + ]) } ) diff --git a/services/web/test/frontend/features/source-editor/components/codemirror-editor-shortcuts.spec.tsx b/services/web/test/frontend/features/source-editor/components/codemirror-editor-shortcuts.spec.tsx index 698016e88f..486a21ce04 100644 --- a/services/web/test/frontend/features/source-editor/components/codemirror-editor-shortcuts.spec.tsx +++ b/services/web/test/frontend/features/source-editor/components/codemirror-editor-shortcuts.spec.tsx @@ -158,16 +158,13 @@ describe('emacs keybindings', { scrollBehavior: false }, function () { cy.interceptEvents() cy.interceptSpelling() - // Make a short doc that will fit entirely into the dom tree, so that - // index() corresponds to line number - 1 const shortDoc = ` \\documentclass{article} \\begin{document} contentLine1 contentLine2 contentLine3 -\\end{document} -` +\\end{document}` const scope = mockScope(shortDoc) scope.settings.mode = 'emacs' @@ -188,7 +185,7 @@ contentLine3 }) it('emulates search behaviour', function () { - activeEditorLine().index().should('equal', 1) + activeEditorLine().should('have.text', '\\documentclass{article}') // Search should be closed cy.findByRole('search').should('have.length', 0) @@ -217,7 +214,7 @@ contentLine3 cy.findByRole('search').should('have.length', 0) // Cursor should be back to where the search originated from - activeEditorLine().index().should('equal', 1) + activeEditorLine().should('have.text', '\\documentclass{article}') // Invoke C-r cy.get('@line').type('{ctrl}r') @@ -243,11 +240,11 @@ contentLine3 }) it('should jump between start and end with M-S-, and M-S-.', function () { - activeEditorLine().index().should('equal', 1) + activeEditorLine().should('have.text', '\\documentclass{article}') activeEditorLine().type('{alt}{shift},') - activeEditorLine().index().should('equal', 0) + activeEditorLine().should('have.text', '') activeEditorLine().type('{alt}{shift}.') - activeEditorLine().index().should('equal', 7) + activeEditorLine().should('have.text', '\\end{document}') }) it('can enter characters', function () { @@ -307,11 +304,11 @@ contentLine3 it('can move around in normal mode', function () { // Move cursor up cy.get('@line').type('k') - activeEditorLine().index().should('equal', 0) + activeEditorLine().should('have.text', '') // Move cursor down cy.get('@line').type('j') - activeEditorLine().index().should('equal', 2) + activeEditorLine().should('have.text', '\\begin{document}') // Move the cursor left, insert 1, move it right, insert a 2 cy.get('@line')