Merge pull request #16493 from overleaf/ii-rp-mini-page-load

[web] Prevent opening and closing of the mini review panel

GitOrigin-RevId: 25297a24ddfb2654b00efa8d33b825620f859147

services/web/frontend/js/features/ide-react/context/review-panel/hooks/use-review-panel-state.ts

@@ -4,11 +4,10 @@ import { isEqual, cloneDeep } from 'lodash'
 import usePersistedState from '@/shared/hooks/use-persisted-state'
 import useScopeValue from '../../../../../shared/hooks/use-scope-value'
 import useSocketListener from '@/features/ide-react/hooks/use-socket-listener'
-import useAsync from '@/shared/hooks/use-async'
 import useAbortController from '@/shared/hooks/use-abort-controller'
 import useScopeEventEmitter from '@/shared/hooks/use-scope-event-emitter'
 import useLayoutToLeft from '@/features/ide-react/context/review-panel/hooks/useLayoutToLeft'
-import { sendMB } from '../../../../../infrastructure/event-tracking'
+import { sendMB } from '@/infrastructure/event-tracking'
 import {
   dispatchReviewPanelLayout as handleLayoutChange,
   UpdateType,
@@ -197,26 +196,25 @@ function useReviewPanelState(): ReviewPanelStateReactIde {
     Record<ThreadId, boolean>
   >({})
 
-  const {
-    isLoading: loadingThreads,
-    reset,
-    runAsync: runAsyncThreads,
-  } = useAsync<ReviewPanelCommentThreadsApi>()
+  const [loadingThreads, setLoadingThreads] =
+    useScopeValue<boolean>('loadingThreads')
+
   const loadThreadsController = useAbortController()
-  const loadThreadsExecuted = useRef(false)
+  const threadsLoadedOnceRef = useRef(false)
+  const loadingThreadsInProgressRef = useRef(false)
   const ensureThreadsAreLoaded = useCallback(() => {
-    if (loadThreadsExecuted.current) {
+    if (threadsLoadedOnceRef.current) {
       // We get any updates in real time so only need to load them once.
       return
     }
-    loadThreadsExecuted.current = true
+    threadsLoadedOnceRef.current = true
+    loadingThreadsInProgressRef.current = true
 
-    return runAsyncThreads(
-      getJSON(`/project/${projectId}/threads`, {
-        signal: loadThreadsController.signal,
-      })
-    )
+    return getJSON(`/project/${projectId}/threads`, {
+      signal: loadThreadsController.signal,
+    })
       .then(threads => {
+        setLoadingThreads(false)
         const tempResolvedThreadIds: typeof resolvedThreadIds = {}
         const threadsEntries = Object.entries(threads) as [
           [
@@ -248,7 +246,10 @@ function useReviewPanelState(): ReviewPanelStateReactIde {
         }
       })
       .catch(debugConsole.error)
-  }, [loadThreadsController.signal, projectId, runAsyncThreads])
+      .finally(() => {
+        loadingThreadsInProgressRef.current = false
+      })
+  }, [loadThreadsController.signal, projectId, setLoadingThreads])
 
   const rangesTrackers = useRef<Record<DocId, RangesTracker>>({})
   const refreshingRangeUsers = useRef(false)
@@ -335,21 +336,6 @@ function useReviewPanelState(): ReviewPanelStateReactIde {
   const updateEntries = useCallback(
     async (docId: DocId) => {
       const rangesTracker = getChangeTracker(docId)
-      let localResolvedThreadIds = resolvedThreadIds
-
-      if (!isRestrictedTokenMember) {
-        if (rangesTracker.comments.length > 0) {
-          const threadsLoadResult = await ensureThreadsAreLoaded()
-          if (typeof threadsLoadResult === 'object') {
-            localResolvedThreadIds = threadsLoadResult.resolvedThreadIds
-          }
-        } else if (loadingThreads) {
-          // ensure that tracked changes are highlighted even if no comments are loaded
-          reset()
-          dispatchReviewPanelEvent('loaded_threads')
-        }
-      }
-
       const docEntries = cloneDeep(getDocEntries(docId))
       const docResolvedComments = cloneDeep(getDocResolvedComments(docId))
       // Assume we'll delete everything until we see it, then we'll remove it from this object
@@ -432,32 +418,41 @@ function useReviewPanelState(): ReviewPanelStateReactIde {
         }
       }
 
-      for (const comment of rangesTracker.comments) {
-        deleteChanges.delete(comment.id)
+      let localResolvedThreadIds = resolvedThreadIds
 
-        const newEntry: Partial<ReviewPanelCommentEntry> = {
-          type: 'comment',
-          thread_id: comment.op.t,
-          entry_ids: [comment.id],
-          content: comment.op.c,
-          offset: comment.op.p,
+      if (!isRestrictedTokenMember) {
+        if (rangesTracker.comments.length > 0) {
+          const threadsLoadResult = await ensureThreadsAreLoaded()
+          if (threadsLoadResult?.resolvedThreadIds) {
+            localResolvedThreadIds = threadsLoadResult.resolvedThreadIds
+          }
+        } else if (loadingThreads) {
+          // ensure that tracked changes are highlighted even if no comments are loaded
+          setLoadingThreads(false)
+          dispatchReviewPanelEvent('loaded_threads')
         }
+      }
 
-        let newComment: any
-        if (localResolvedThreadIds[comment.op.t]) {
-          docResolvedComments[comment.id] ??= {} as ReviewPanelCommentEntry
-          newComment = docResolvedComments[comment.id]
-          delete docEntries[comment.id]
-        } else {
-          docEntries[comment.id] ??= {} as ReviewPanelEntry
-          newComment = docEntries[comment.id]
-          delete docResolvedComments[comment.id]
-        }
+      if (!loadingThreadsInProgressRef.current) {
+        for (const comment of rangesTracker.comments) {
+          deleteChanges.delete(comment.id)
+
+          let newComment: any
+          if (localResolvedThreadIds[comment.op.t]) {
+            docResolvedComments[comment.id] ??= {} as ReviewPanelCommentEntry
+            newComment = docResolvedComments[comment.id]
+            delete docEntries[comment.id]
+          } else {
+            docEntries[comment.id] ??= {} as ReviewPanelEntry
+            newComment = docEntries[comment.id]
+            delete docResolvedComments[comment.id]
+          }
 
-        for (const [key, value] of Object.entries(newEntry) as Entries<
-          typeof newEntry
-        >) {
-          newComment[key] = value
+          newComment.type = 'comment'
+          newComment.thread_id = comment.op.t
+          newComment.entry_ids = [comment.id]
+          newComment.content = comment.op.c
+          newComment.offset = comment.op.p
         }
       }
 
@@ -489,7 +484,7 @@ function useReviewPanelState(): ReviewPanelStateReactIde {
       users,
       ensureThreadsAreLoaded,
       loadingThreads,
-      reset,
+      setLoadingThreads,
     ]
   )
 

services/web/frontend/js/features/ide-react/scope-adapters/review-panel-context-adapter.ts

@@ -1,6 +1,7 @@
 import { ReactScopeValueStore } from '@/features/ide-react/scope-value-store/react-scope-value-store'
 
 export default function populateReviewPanelScope(store: ReactScopeValueStore) {
+  store.set('loadingThreads', true)
   store.set('users', {})
   store.set('addNewComment', () => {})
 }

services/web/frontend/js/ide/review-panel/controllers/ReviewPanelController.js

@@ -404,7 +404,7 @@ export default App.controller('ReviewPanelController', [
       return q
     }
 
-    function updateEntries(doc_id) {
+    async function updateEntries(doc_id) {
       const rangesTracker = getChangeTracker(doc_id)
       const entries = getDocEntries(doc_id)
       const resolvedComments = getDocResolvedComments(doc_id)
@@ -478,7 +478,7 @@ export default App.controller('ReviewPanelController', [
 
       if (!window.isRestrictedTokenMember) {
         if (rangesTracker.comments.length > 0) {
-          ensureThreadsAreLoaded()
+          await ensureThreadsAreLoaded()
         } else if (ide.$scope.loadingThreads === true) {
           // ensure that tracked changes are highlighted even if no comments are loaded
           ide.$scope.loadingThreads = false
@@ -486,32 +486,34 @@ export default App.controller('ReviewPanelController', [
         }
       }
 
-      for (const comment of Array.from(rangesTracker.comments)) {
-        let new_comment
-        changed = true
-        delete delete_changes[comment.id]
-        if (ide.$scope.reviewPanel.resolvedThreadIds[comment.op.t]) {
-          new_comment =
-            resolvedComments[comment.id] != null
-              ? resolvedComments[comment.id]
-              : (resolvedComments[comment.id] = {})
-          delete entries[comment.id]
-        } else {
-          new_comment =
-            entries[comment.id] != null
-              ? entries[comment.id]
-              : (entries[comment.id] = {})
-          delete resolvedComments[comment.id]
-        }
-        const new_entry = {
-          type: 'comment',
-          thread_id: comment.op.t,
-          entry_ids: [comment.id],
-          content: comment.op.c,
-          offset: comment.op.p,
-        }
-        for (const key in new_entry) {
-          new_comment[key] = new_entry[key]
+      if (!_loadingThreadsInProgress) {
+        for (const comment of Array.from(rangesTracker.comments)) {
+          let new_comment
+          changed = true
+          delete delete_changes[comment.id]
+          if (ide.$scope.reviewPanel.resolvedThreadIds[comment.op.t]) {
+            new_comment =
+              resolvedComments[comment.id] != null
+                ? resolvedComments[comment.id]
+                : (resolvedComments[comment.id] = {})
+            delete entries[comment.id]
+          } else {
+            new_comment =
+              entries[comment.id] != null
+                ? entries[comment.id]
+                : (entries[comment.id] = {})
+            delete resolvedComments[comment.id]
+          }
+          const new_entry = {
+            type: 'comment',
+            thread_id: comment.op.t,
+            entry_ids: [comment.id],
+            content: comment.op.c,
+            offset: comment.op.p,
+          }
+          for (const key in new_entry) {
+            new_comment[key] = new_entry[key]
+          }
         }
       }
 
@@ -530,9 +532,9 @@ export default App.controller('ReviewPanelController', [
       return entries
     }
 
-    $scope.$on('editor:track-changes:changed', function () {
+    $scope.$on('editor:track-changes:changed', async function () {
       const doc_id = $scope.editor.open_doc_id
-      const entries = updateEntries(doc_id)
+      const entries = await updateEntries(doc_id)
 
       $scope.$broadcast('review-panel:recalculate-screen-positions')
       dispatchReviewPanelEvent('recalculate-screen-positions', {
@@ -1191,19 +1193,22 @@ export default App.controller('ReviewPanelController', [
         .catch(() => (_refreshingRangeUsers = false))
     }
 
-    let _threadsLoaded = false
-    function ensureThreadsAreLoaded() {
-      if (_threadsLoaded) {
+    let _threadsLoadedOnce = false
+    let _loadingThreadsInProgress = false
+    async function ensureThreadsAreLoaded() {
+      if (_threadsLoadedOnce) {
         // We get any updates in real time so only need to load them once.
         return
       }
-      _threadsLoaded = true
+      _threadsLoadedOnce = true
+      _loadingThreadsInProgress = true
       ide.$scope.loadingThreads = true
       return $http
         .get(`/project/${$scope.project_id}/threads`)
-        .then(function (response) {
+        .then(async function (response) {
           const threads = response.data
           ide.$scope.loadingThreads = false
+          _loadingThreadsInProgress = false
           for (const thread_id in ide.$scope.reviewPanel.resolvedThreadIds) {
             delete ide.$scope.reviewPanel.resolvedThreadIds[thread_id]
           }
@@ -1222,7 +1227,7 @@ export default App.controller('ReviewPanelController', [
           // Update entries so that the view has up-to-date information about
           // the entries when handling the loaded_threads events, which avoids
           // thrashing
-          updateEntries($scope.editor.open_doc_id)
+          await updateEntries($scope.editor.open_doc_id)
 
           dispatchReviewPanelEvent('loaded_threads')
           return $timeout(() => ide.$scope.$broadcast('review-panel:layout'))