Przeglądaj źródła

Merge pull request #12933 from overleaf/ii-history-react-select-latest-version-when-all-labels-are-removed

[web] Fix history entries and labels selection issues

GitOrigin-RevId: 73b14ba15ab4f0d9ff5946b1159ae7d5912582dd
Miguel Serrano 3 lat temu
rodzic
commit
92ade70601

+ 1 - 6
services/web/frontend/js/features/history/components/change-list/change-list.tsx

@@ -1,15 +1,10 @@
-import usePersistedState from '../../../../shared/hooks/use-persisted-state'
 import ToggleSwitch from './toggle-switch'
 import AllHistoryList from './all-history-list'
 import LabelsList from './labels-list'
 import { useHistoryContext } from '../../context/history-context'
 
 function ChangeList() {
-  const { projectId, error } = useHistoryContext()
-  const [labelsOnly, setLabelsOnly] = usePersistedState(
-    `history.userPrefs.showOnlyLabels.${projectId}`,
-    false
-  )
+  const { error, labelsOnly, setLabelsOnly } = useHistoryContext()
 
   return (
     <aside className="change-list">

+ 17 - 29
services/web/frontend/js/features/history/components/change-list/dropdown/menu-item/compare.tsx

@@ -2,6 +2,7 @@ import { useTranslation } from 'react-i18next'
 import { MenuItem, MenuItemProps } from 'react-bootstrap'
 import Icon from '../../../../../../shared/components/icon'
 import { useHistoryContext } from '../../../../context/history-context'
+import { computeUpdateRange } from '../../../../utils/range'
 import { UpdateRange } from '../../../../services/types/update'
 
 type CompareProps = {
@@ -19,37 +20,24 @@ function Compare({
   const { t } = useTranslation()
   const { selection, setSelection } = useHistoryContext()
 
-  function compare() {
-    const { updateRange } = selection
-    if (!updateRange) {
-      return
-    }
-    const fromVersion = Math.min(fromV, updateRange.fromV)
-    const toVersion = Math.max(toV, updateRange.toV)
-    const fromVTimestamp = Math.min(
-      updateMetaEndTimestamp,
-      updateRange.fromVTimestamp
-    )
-    const toVTimestamp = Math.max(
-      updateMetaEndTimestamp,
-      updateRange.toVTimestamp
-    )
-
-    setSelection({
-      updateRange: {
-        fromV: fromVersion,
-        toV: toVersion,
-        fromVTimestamp,
-        toVTimestamp,
-      },
-      comparing: true,
-      files: [],
-    })
-  }
-
   const handleCompareVersion = (e: React.MouseEvent<MenuItemProps>) => {
     e.stopPropagation()
-    compare()
+
+    const { updateRange } = selection
+    if (updateRange) {
+      const range = computeUpdateRange(
+        updateRange,
+        fromV,
+        toV,
+        updateMetaEndTimestamp
+      )
+
+      setSelection({
+        updateRange: range,
+        comparing: true,
+        files: [],
+      })
+    }
   }
 
   return (

+ 6 - 12
services/web/frontend/js/features/history/components/change-list/labels-list.tsx

@@ -1,3 +1,4 @@
+import { useMemo } from 'react'
 import { useTranslation } from 'react-i18next'
 import HistoryVersionDetails from './history-version-details'
 import TagTooltip from './tag-tooltip'
@@ -6,25 +7,18 @@ import LabelDropdown from './dropdown/label-dropdown'
 import { useHistoryContext } from '../../context/history-context'
 import { useUserContext } from '../../../../shared/context/user-context'
 import { isVersionSelected } from '../../utils/history-details'
-import { isPseudoLabel } from '../../utils/label'
+import { getVersionWithLabels, isPseudoLabel } from '../../utils/label'
 import { formatTime, isoToUnix } from '../../../utils/format-date'
-import { groupBy, orderBy } from 'lodash'
-import { LoadedLabel } from '../../services/types/label'
 
 function LabelsList() {
   const { t } = useTranslation()
   const { labels, projectId, selection } = useHistoryContext()
   const { id: currentUserId } = useUserContext()
 
-  let versionWithLabels: { version: number; labels: LoadedLabel[] }[] = []
-  if (labels) {
-    const groupedLabelsHash = groupBy(labels, 'version')
-    versionWithLabels = Object.keys(groupedLabelsHash).map(key => ({
-      version: parseInt(key, 10),
-      labels: groupedLabelsHash[key],
-    }))
-    versionWithLabels = orderBy(versionWithLabels, ['version'], ['desc'])
-  }
+  const versionWithLabels = useMemo(
+    () => getVersionWithLabels(labels),
+    [labels]
+  )
 
   return (
     <>

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

@@ -1,5 +1,7 @@
 import { useTranslation } from 'react-i18next'
 import { useHistoryContext } from '../../context/history-context'
+import { getUpdateForVersion } from '../../utils/history-details'
+import { computeUpdateRange } from '../../utils/range'
 
 type ToggleSwitchProps = {
   labelsOnly: boolean
@@ -10,13 +12,48 @@ type ToggleSwitchProps = {
 
 function ToggleSwitch({ labelsOnly, setLabelsOnly }: ToggleSwitchProps) {
   const { t } = useTranslation()
-  const { resetSelection, selection } = useHistoryContext()
+  const { selection, setSelection, resetSelection, updatesInfo } =
+    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
       resetSelection()
     }
 
+    // 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
+    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
+        )
+
+        setSelection({
+          updateRange: range,
+          comparing: false,
+          files: [],
+        })
+      }
+    }
+
     setLabelsOnly(isLabelsOnly)
   }
 

+ 9 - 0
services/web/frontend/js/features/history/context/history-context.tsx

@@ -16,6 +16,7 @@ import { renamePathnameKey } from '../utils/file-tree'
 import { isFileRenamed } from '../utils/file-diff'
 import { loadLabels } from '../utils/label'
 import { autoSelectFile } from '../utils/auto-select-file'
+import usePersistedState from '../../../shared/hooks/use-persisted-state'
 import ColorManager from '../../../ide/colors/ColorManager'
 import moment from 'moment'
 import { cloneDeep } from 'lodash'
@@ -86,6 +87,10 @@ function useHistory() {
     nextBeforeTimestamp: undefined,
   })
   const [labels, setLabels] = useState<HistoryContextValue['labels']>(null)
+  const [labelsOnly, setLabelsOnly] = usePersistedState(
+    `history.userPrefs.showOnlyLabels.${projectId}`,
+    false
+  )
   const [loadingState, setLoadingState] =
     useState<HistoryContextValue['loadingState']>('loadingInitial')
   const [error, setError] = useState<HistoryContextValue['error']>(null)
@@ -269,6 +274,8 @@ function useHistory() {
       setUpdatesInfo,
       labels,
       setLabels,
+      labelsOnly,
+      setLabelsOnly,
       userHasFullFeature,
       currentUserIsOwner,
       projectId,
@@ -286,6 +293,8 @@ function useHistory() {
       setUpdatesInfo,
       labels,
       setLabels,
+      labelsOnly,
+      setLabelsOnly,
       userHasFullFeature,
       currentUserIsOwner,
       projectId,

+ 2 - 0
services/web/frontend/js/features/history/context/types/history-context-value.ts

@@ -31,6 +31,8 @@ export type HistoryContextValue = {
   setError: React.Dispatch<React.SetStateAction<HistoryContextValue['error']>>
   labels: Nullable<LoadedLabel[]>
   setLabels: React.Dispatch<React.SetStateAction<HistoryContextValue['labels']>>
+  labelsOnly: boolean
+  setLabelsOnly: React.Dispatch<React.SetStateAction<boolean>>
   projectId: string
   selection: Selection
   setSelection: React.Dispatch<

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

@@ -1,9 +1,16 @@
 import { useHistoryContext } from '../context/history-context'
-import { isLabel, loadLabels } from '../utils/label'
+import { getVersionWithLabels, isLabel, loadLabels } from '../utils/label'
 import { Label } from '../services/types/label'
 
 function useAddOrRemoveLabels() {
-  const { updatesInfo, setUpdatesInfo, labels, setLabels } = useHistoryContext()
+  const {
+    updatesInfo,
+    setUpdatesInfo,
+    labels,
+    setLabels,
+    selection,
+    resetSelection,
+  } = useHistoryContext()
 
   const addOrRemoveLabel = (
     label: Label,
@@ -25,7 +32,10 @@ function useAddOrRemoveLabels() {
     if (labels) {
       const nonPseudoLabels = labels.filter(isLabel)
       const processedNonPseudoLabels = labelsHandler(nonPseudoLabels)
-      setLabels(loadLabels(processedNonPseudoLabels, tempUpdates[0].toV))
+      const newLabels = loadLabels(processedNonPseudoLabels, tempUpdates[0].toV)
+      setLabels(newLabels)
+
+      return newLabels
     }
   }
 
@@ -37,7 +47,20 @@ function useAddOrRemoveLabels() {
   const removeUpdateLabel = (label: Label) => {
     const labelHandler = (labels: Label[]) =>
       labels.filter(({ id }) => id !== label.id)
-    addOrRemoveLabel(label, labelHandler)
+    const newLabels = addOrRemoveLabel(label, labelHandler)
+
+    // removing all labels from current selection should reset the selection
+    if (newLabels) {
+      const versionWithLabels = getVersionWithLabels(newLabels)
+      // 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()
+      }
+    }
   }
 
   return { addUpdateLabel, removeUpdateLabel }

+ 15 - 2
services/web/frontend/js/features/history/utils/label.ts

@@ -1,4 +1,4 @@
-import { orderBy } from 'lodash'
+import { orderBy, groupBy } from 'lodash'
 import {
   LoadedLabel,
   Label,
@@ -63,4 +63,17 @@ export const loadLabels = (
   return labelsWithPseudoLabelIfNeeded
 }
 
-export const updateLabels = () => {}
+export const getVersionWithLabels = (labels: Nullable<LoadedLabel[]>) => {
+  let versionWithLabels: { version: number; labels: LoadedLabel[] }[] = []
+
+  if (labels) {
+    const groupedLabelsHash = groupBy(labels, 'version')
+    versionWithLabels = Object.keys(groupedLabelsHash).map(key => ({
+      version: parseInt(key, 10),
+      labels: groupedLabelsHash[key],
+    }))
+    versionWithLabels = orderBy(versionWithLabels, ['version'], ['desc'])
+  }
+
+  return versionWithLabels
+}

+ 26 - 0
services/web/frontend/js/features/history/utils/range.ts

@@ -0,0 +1,26 @@
+import { UpdateRange } from '../services/types/update'
+
+export const computeUpdateRange = (
+  updateRange: UpdateRange,
+  fromV: number,
+  toV: number,
+  updateMetaEndTimestamp: number
+) => {
+  const fromVersion = Math.min(fromV, updateRange.fromV)
+  const toVersion = Math.max(toV, updateRange.toV)
+  const fromVTimestamp = Math.min(
+    updateMetaEndTimestamp,
+    updateRange.fromVTimestamp
+  )
+  const toVTimestamp = Math.max(
+    updateMetaEndTimestamp,
+    updateRange.toVTimestamp
+  )
+
+  return {
+    fromV: fromVersion,
+    toV: toVersion,
+    fromVTimestamp,
+    toVTimestamp,
+  }
+}