From 74a8d6111ad445a32027bd0ebc99f4b0d2a0ed63 Mon Sep 17 00:00:00 2001 From: ilkin-overleaf <100852799+ilkin-overleaf@users.noreply.github.com> Date: Tue, 9 May 2023 11:56:40 +0300 Subject: [PATCH] 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 --- .../components/change-list/toggle-switch.tsx | 75 ++++++++++--------- .../history/hooks/use-add-or-remove-labels.ts | 19 ++--- .../js/features/history/utils/label.ts | 12 +++ 3 files changed, 58 insertions(+), 48 deletions(-) diff --git a/services/web/frontend/js/features/history/components/change-list/toggle-switch.tsx b/services/web/frontend/js/features/history/components/change-list/toggle-switch.tsx index a627879429..85c6b206f9 100644 --- a/services/web/frontend/js/features/history/components/change-list/toggle-switch.tsx +++ b/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 = { - labelsOnly: boolean - setLabelsOnly: React.Dispatch< - React.SetStateAction - > -} +type ToggleSwitchProps = Pick< + HistoryContextValue, + 'labelsOnly' | 'setLabelsOnly' +> 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 - // 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 - 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 ( - 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: [], - }) + setSelection({ + updateRange: range, + comparing: false, + files: [], + }) + } } } diff --git a/services/web/frontend/js/features/history/hooks/use-add-or-remove-labels.ts b/services/web/frontend/js/features/history/hooks/use-add-or-remove-labels.ts index 97ea8ffc4b..465510f9fa 100644 --- a/services/web/frontend/js/features/history/hooks/use-add-or-remove-labels.ts +++ b/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) { - const versionWithLabels = getVersionWithLabels(newLabels) - // build an Array 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() - } + if (isAnyVersionMatchingSelection(newLabels, selection)) { + resetSelection() } } diff --git a/services/web/frontend/js/features/history/utils/label.ts b/services/web/frontend/js/features/history/utils/label.ts index b55056640c..32948b32af 100644 --- a/services/web/frontend/js/features/history/utils/label.ts +++ b/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) => { return versionWithLabels } + +export const isAnyVersionMatchingSelection = ( + labels: Nullable, + selection: Selection +) => { + // build an Array of available versions + const versions = getVersionWithLabels(labels).map(v => v.version) + const selectedVersion = selection.updateRange?.toV + + return selectedVersion && !versions.includes(selectedVersion) +}