Просмотр исходного кода

[cm6] Improve "vertical overflow" handling (#12405)

* Avoid setting style on the editor view whenever overflowPaddingState changes
* Round values in recalculateScreenPositions
* Refactor ignoreGeometryChangesUntil slightly
* Restore the original value

GitOrigin-RevId: 75c57be21dd16a748c0d9b14058386849a80b831
Alf Eaton 3 лет назад
Родитель
Сommit
a8ed6b65e1

+ 14 - 16
services/web/frontend/js/features/source-editor/extensions/changes/change-manager.ts

@@ -63,8 +63,8 @@ export const createChangeManager = (
       )
       )
 
 
       if (coords) {
       if (coords) {
-        const y = coords.top - contentRect.top - editorPaddingTop
-        const height = coords.bottom - coords.top
+        const y = Math.round(coords.top - contentRect.top - editorPaddingTop)
+        const height = Math.round(coords.bottom - coords.top)
 
 
         entry.screenPos = { y, height, editorPaddingTop }
         entry.screenPos = { y, height, editorPaddingTop }
       }
       }
@@ -442,8 +442,7 @@ export const createChangeManager = (
     window.removeEventListener('review-panel:event', reviewPanelEventListener)
     window.removeEventListener('review-panel:event', reviewPanelEventListener)
   }
   }
 
 
-  let lastUpdatedTopPaddingAt = 0
-  const PADDING_GEOMETRY_CHANGE_INTERVAL = 50
+  let ignoreGeometryChangesUntil = 0
 
 
   return {
   return {
     initialize() {
     initialize() {
@@ -452,8 +451,18 @@ export const createChangeManager = (
     },
     },
     handleUpdate(update: ViewUpdate) {
     handleUpdate(update: ViewUpdate) {
       if (updateSetsVerticalOverflow(update)) {
       if (updateSetsVerticalOverflow(update)) {
-        lastUpdatedTopPaddingAt = Date.now()
+        ignoreGeometryChangesUntil = Date.now() + 50 // ignore changes for 50ms
+      } else if (
+        (update.geometryChanged || update.viewportChanged) &&
+        Date.now() < ignoreGeometryChangesUntil
+      ) {
+        // Ignore a change to the editor geometry 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.
+        return
       }
       }
+
       if (
       if (
         update.docChanged ||
         update.docChanged ||
         update.focusChanged ||
         update.focusChanged ||
@@ -461,17 +470,6 @@ export const createChangeManager = (
         update.selectionSet ||
         update.selectionSet ||
         update.geometryChanged
         update.geometryChanged
       ) {
       ) {
-        // Ignore a change to the editor geometry 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.
-        if (
-          update.geometryChanged &&
-          Date.now() - lastUpdatedTopPaddingAt <
-            PADDING_GEOMETRY_CHANGE_INTERVAL
-        ) {
-          return
-        }
         dispatchFocusChangedEvent(update.state)
         dispatchFocusChangedEvent(update.state)
       }
       }
     },
     },

+ 29 - 9
services/web/frontend/js/features/source-editor/extensions/vertical-overflow.ts

@@ -1,5 +1,6 @@
 import {
 import {
   Extension,
   Extension,
+  Facet,
   StateEffect,
   StateEffect,
   StateField,
   StateField,
   TransactionSpec,
   TransactionSpec,
@@ -10,6 +11,8 @@ export function verticalOverflow(): Extension {
   return [
   return [
     overflowPaddingState,
     overflowPaddingState,
     minimumBottomPaddingState,
     minimumBottomPaddingState,
+    bottomPadding,
+    topPadding,
     contentAttributes,
     contentAttributes,
     bottomPaddingPlugin,
     bottomPaddingPlugin,
     topPaddingPlugin,
     topPaddingPlugin,
@@ -132,19 +135,36 @@ const bottomPaddingPlugin = ViewPlugin.define(view => {
   }
   }
 })
 })
 
 
+const topPaddingFacet = Facet.define<number>()
+const topPadding = topPaddingFacet.compute([overflowPaddingState], state => {
+  return state.field(overflowPaddingState).top
+})
+
+const bottomPaddingFacet = Facet.define<number>({
+  combine(values) {
+    return [Math.max(...values)]
+  },
+})
+const bottomPadding = bottomPaddingFacet.computeN(
+  [overflowPaddingState, minimumBottomPaddingState],
+  state => {
+    return [
+      state.field(minimumBottomPaddingState),
+      state.field(overflowPaddingState).bottom,
+    ]
+  }
+)
+
 // Set a style attribute on the contentDOM containing the calculated top and bottom padding.
 // Set a style attribute on the contentDOM containing the calculated top and bottom padding.
 // This value will be concatenated with style values from any other extensions.
 // This value will be concatenated with style values from any other extensions.
+// TODO: use elements instead?
 const contentAttributes = EditorView.contentAttributes.compute(
 const contentAttributes = EditorView.contentAttributes.compute(
-  [overflowPaddingState, minimumBottomPaddingState],
+  [topPaddingFacet, bottomPaddingFacet],
   state => {
   state => {
-    const overflowPadding = state.field(overflowPaddingState)
-    const minimumBottomPadding = state.field(minimumBottomPaddingState)
-
-    const bottomPadding = Math.max(minimumBottomPadding, overflowPadding.bottom)
-
-    return {
-      style: `padding-top: ${overflowPadding.top}px; padding-bottom: ${bottomPadding}px;`,
-    }
+    const [top] = state.facet(topPaddingFacet)
+    const [bottom] = state.facet(bottomPaddingFacet)
+    const style = `padding-top: ${top}px; padding-bottom: ${bottom}px;`
+    return { style }
   }
   }
 )
 )