Jelajahi Sumber

[cm6] Use a block widget for padding at the top of the editor (#12705)

GitOrigin-RevId: 000ce9c90ea6b2ca72ab969704354a19fcea7a87
Alf Eaton 3 tahun lalu
induk
melakukan
acf6abb0fb

+ 0 - 4
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<number>
     }
   }
 }
@@ -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')
-})

+ 5 - 2
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.

+ 51 - 6
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,19 +162,57 @@ 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 [top] = state.facet(topPaddingFacet)
     const [bottom] = state.facet(bottomPaddingFacet)
-    const style = `padding-top: ${top}px; padding-bottom: ${bottom}px;`
+    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)
+
+    return Decoration.set([
+      Decoration.widget({
+        widget: new TopPaddingWidget(top),
+        block: true,
+      }).range(0),
+    ])
+  }
+)
+
 export function setVerticalOverflow(padding: VerticalPadding): TransactionSpec {
   return {
     effects: [setOverflowPaddingEffect.of(padding)],

+ 8 - 11
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')