From a8ed6b65e107bbb5df65f6eb8bcc211434a6a2dc Mon Sep 17 00:00:00 2001
From: Alf Eaton <alf.eaton@overleaf.com>
Date: Fri, 14 Apr 2023 09:54:36 +0100
Subject: [PATCH] [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
---
 .../extensions/changes/change-manager.ts      | 30 +++++++-------
 .../extensions/vertical-overflow.ts           | 40 ++++++++++++++-----
 2 files changed, 44 insertions(+), 26 deletions(-)

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 32f638e70f..2b1cd0db34 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
@@ -63,8 +63,8 @@ export const createChangeManager = (
       )
 
       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 }
       }
@@ -442,8 +442,7 @@ export const createChangeManager = (
     window.removeEventListener('review-panel:event', reviewPanelEventListener)
   }
 
-  let lastUpdatedTopPaddingAt = 0
-  const PADDING_GEOMETRY_CHANGE_INTERVAL = 50
+  let ignoreGeometryChangesUntil = 0
 
   return {
     initialize() {
@@ -452,8 +451,18 @@ export const createChangeManager = (
     },
     handleUpdate(update: ViewUpdate) {
       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 (
         update.docChanged ||
         update.focusChanged ||
@@ -461,17 +470,6 @@ export const createChangeManager = (
         update.selectionSet ||
         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)
       }
     },
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 62233eaa17..8ab63fd358 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
@@ -1,5 +1,6 @@
 import {
   Extension,
+  Facet,
   StateEffect,
   StateField,
   TransactionSpec,
@@ -10,6 +11,8 @@ export function verticalOverflow(): Extension {
   return [
     overflowPaddingState,
     minimumBottomPaddingState,
+    bottomPadding,
+    topPadding,
     contentAttributes,
     bottomPaddingPlugin,
     topPaddingPlugin,
@@ -132,19 +135,36 @@ const bottomPaddingPlugin = ViewPlugin.define(view => {
   }
 })
 
-// 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.
-const contentAttributes = EditorView.contentAttributes.compute(
+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 => {
-    const overflowPadding = state.field(overflowPaddingState)
-    const minimumBottomPadding = state.field(minimumBottomPaddingState)
+    return [
+      state.field(minimumBottomPaddingState),
+      state.field(overflowPaddingState).bottom,
+    ]
+  }
+)
 
-    const bottomPadding = Math.max(minimumBottomPadding, overflowPadding.bottom)
-
-    return {
-      style: `padding-top: ${overflowPadding.top}px; padding-bottom: ${bottomPadding}px;`,
-    }
+// 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.
+// TODO: use elements instead?
+const contentAttributes = EditorView.contentAttributes.compute(
+  [topPaddingFacet, bottomPaddingFacet],
+  state => {
+    const [top] = state.facet(topPaddingFacet)
+    const [bottom] = state.facet(bottomPaddingFacet)
+    const style = `padding-top: ${top}px; padding-bottom: ${bottom}px;`
+    return { style }
   }
 )