Merge pull request #12987 from overleaf/ii-history-react-fix-label-selection-when-entry-with-no-label-is-selected

[web] Fix and improve labels and history entries selection

GitOrigin-RevId: 92c0b463a4cb9db7a293699e35c28ce5331de43a

services/web/frontend/js/features/history/components/change-list/toggle-switch.tsx

@@ -2,55 +2,56 @@ import { useTranslation } from 'react-i18next'
 import { useHistoryContext } from '../../context/history-context'
 import { getUpdateForVersion } from '../../utils/history-details'
 import { computeUpdateRange } from '../../utils/range'
+import { isAnyVersionMatchingSelection } from '../../utils/label'
+import { HistoryContextValue } from '../../context/types/history-context-value'
 
-type ToggleSwitchProps = {
+type ToggleSwitchProps = Pick<
-  labelsOnly: boolean
+  HistoryContextValue,
-  setLabelsOnly: React.Dispatch<
+  'labelsOnly' | 'setLabelsOnly'
-    React.SetStateAction<ToggleSwitchProps['labelsOnly']>
+>
-  >
-}
 
 function ToggleSwitch({ labelsOnly, setLabelsOnly }: ToggleSwitchProps) {
   const { t } = useTranslation()
-  const { selection, setSelection, resetSelection, updatesInfo } =
+  const { selection, setSelection, resetSelection, updatesInfo, labels } =
     useHistoryContext()
 
   const handleChange = (isLabelsOnly: boolean) => {
-    let isSelectionReset = false
-
-    // using the switch toggle should reset the selection when in `compare` mode
     if (selection.comparing) {
-      isSelectionReset = true
+      // using the switch toggle should reset the selection when in `compare` mode
       resetSelection()
-    }
+    } else {
+      if (isLabelsOnly) {
+        if (isAnyVersionMatchingSelection(labels, selection)) {
+          resetSelection()
+        }
+      } else {
+        // in labels only mode the `fromV` is equal to `toV` value
+        // switching to all history mode and triggering immediate comparison with
+        // an older version would cause a bug if the computation below is skipped.
+        const update = selection.updateRange?.toV
+          ? getUpdateForVersion(selection.updateRange.toV, updatesInfo.updates)
+          : null
 
-    // in labels only mode the `fromV` is equal to `toV` value
+        const { updateRange } = selection
-    // switching to all history mode and triggering immediate comparison with
-    // an older version would cause a bug if the computation below is skipped
-    if (!isLabelsOnly && !isSelectionReset) {
-      const update = selection.updateRange?.toV
-        ? getUpdateForVersion(selection.updateRange.toV, updatesInfo.updates)
-        : null
 
-      const { updateRange } = selection
+        if (
+          updateRange &&
+          update &&
+          (update.fromV !== updateRange.fromV || update.toV !== updateRange.toV)
+        ) {
+          const range = computeUpdateRange(
+            updateRange,
+            update.fromV,
+            update.toV,
+            update.meta.end_ts
+          )
 
-      if (
+          setSelection({
-        updateRange &&
+            updateRange: range,
-        update &&
+            comparing: false,
-        (update.fromV !== updateRange.fromV || update.toV !== updateRange.toV)
+            files: [],
-      ) {
+          })
-        const range = computeUpdateRange(
+        }
-          updateRange,
-          update.fromV,
-          update.toV,
-          update.meta.end_ts
-        )
-
-        setSelection({
-          updateRange: range,
-          comparing: false,
-          files: [],
-        })
       }
     }
 

services/web/frontend/js/features/history/hooks/use-add-or-remove-labels.ts

@@ -1,5 +1,9 @@
 import { useHistoryContext } from '../context/history-context'
-import { getVersionWithLabels, isLabel, loadLabels } from '../utils/label'
+import {
+  isAnyVersionMatchingSelection,
+  isLabel,
+  loadLabels,
+} from '../utils/label'
 import { Label } from '../services/types/label'
 
 function useAddOrRemoveLabels() {
@@ -37,6 +41,7 @@ function useAddOrRemoveLabels() {
 
       return newLabels
     }
+    return null
   }
 
   const addUpdateLabel = (label: Label) => {
@@ -50,16 +55,8 @@ function useAddOrRemoveLabels() {
     const newLabels = addOrRemoveLabel(label, labelHandler)
 
     // removing all labels from current selection should reset the selection
-    if (newLabels) {
+    if (isAnyVersionMatchingSelection(newLabels, selection)) {
-      const versionWithLabels = getVersionWithLabels(newLabels)
+      resetSelection()
-      // build an Array<number> of available versions
-      const versions = versionWithLabels.map(v => v.version)
-      const selectedVersion = selection.updateRange?.toV
-
-      // check whether the versions array has a version matching the current selection
-      if (selectedVersion && !versions.includes(selectedVersion)) {
-        resetSelection()
-      }
     }
   }
 

services/web/frontend/js/features/history/utils/label.ts

@@ -5,6 +5,7 @@ import {
   PseudoCurrentStateLabel,
 } from '../services/types/label'
 import { Nullable } from '../../../../../types/utils'
+import { Selection } from '../services/types/selection'
 
 export const isPseudoLabel = (
   label: LoadedLabel
@@ -77,3 +78,14 @@ export const getVersionWithLabels = (labels: Nullable<LoadedLabel[]>) => {
 
   return versionWithLabels
 }
+
+export const isAnyVersionMatchingSelection = (
+  labels: Nullable<LoadedLabel[]>,
+  selection: Selection
+) => {
+  // build an Array<number> of available versions
+  const versions = getVersionWithLabels(labels).map(v => v.version)
+  const selectedVersion = selection.updateRange?.toV
+
+  return selectedVersion && !versions.includes(selectedVersion)
+}