From 3adf366a180fe8649a6fb14c036c9828fd4e7d63 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Till-Andr=C3=A9=20Diegeler?=
 <till-andre.diegeler@cloudogu.com>
Date: Fri, 18 Oct 2024 12:46:09 +0200
Subject: [PATCH] Fix conflicting state behavior in DiffDropDown
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Originally, the state of the DiffDropDown was not coupled to the LoadingDiff and instead a toggle method was forwarded. This was causing problems due to their detachment, and so the state of the DiffDropDown was tied to the LoadingDiff one. The original boolean values have been replaced with strings, so that nuances between 'hide all whitespaces' and 'show all whitespaces' in future feature additions can be recognized without affecting the API a second time.

Co-authored-by: Till-André Diegeler<till-andre.diegeler@cloudogu.com>
Co-authored-by: Thomas Zerr<thomas.zerr@cloudogu.com>
Pushed-by: Thomas Zerr<thomas.zerr@cloudogu.com>
Pushed-by: Till-André Diegeler<till-andre.diegeler@cloudogu.com>
---
 .../fix-invalid-show-whitspace-state.yaml     |  2 +
 .../ui-components/src/repos/DiffDropDown.tsx  | 46 +++++++++++++++++--
 .../ui-components/src/repos/LoadingDiff.tsx   | 24 +++++-----
 3 files changed, 56 insertions(+), 16 deletions(-)
 create mode 100644 gradle/changelog/fix-invalid-show-whitspace-state.yaml

diff --git a/gradle/changelog/fix-invalid-show-whitspace-state.yaml b/gradle/changelog/fix-invalid-show-whitspace-state.yaml
new file mode 100644
index 0000000000..fe57a80f57
--- /dev/null
+++ b/gradle/changelog/fix-invalid-show-whitspace-state.yaml
@@ -0,0 +1,2 @@
+- type: changed
+  description: API to whether hide or show whitespaces inside a diff
diff --git a/scm-ui/ui-components/src/repos/DiffDropDown.tsx b/scm-ui/ui-components/src/repos/DiffDropDown.tsx
index e348142086..d0edf9bd10 100644
--- a/scm-ui/ui-components/src/repos/DiffDropDown.tsx
+++ b/scm-ui/ui-components/src/repos/DiffDropDown.tsx
@@ -17,26 +17,54 @@
 import React, { FC, useEffect, useState } from "react";
 import { useTranslation } from "react-i18next";
 import { Checkbox } from "@scm-manager/ui-core";
+
+export type HideWhiteSpaceMode = "ALL" | "NONE";
+
 type DiffDropDownProps = {
   collapseDiffs: () => void;
-  ignoreWhitespaces: () => void;
+  /**
+   * @deprecated Use isIgnoreWhitespaces/setIgnoreWhitespaces instead.
+   */
+  ignoreWhitespaces?: () => void;
   renderOnMount: boolean;
+  ignoreWhitespacesMode?: HideWhiteSpaceMode;
+  setIgnoreWhitespacesMode?: (hideWhiteSpaceMode: HideWhiteSpaceMode) => void;
 };
-const DiffDropDown: FC<DiffDropDownProps> = ({ collapseDiffs, ignoreWhitespaces, renderOnMount }) => {
+const DiffDropDown: FC<DiffDropDownProps> = ({
+  collapseDiffs,
+  ignoreWhitespaces,
+  renderOnMount,
+  ignoreWhitespacesMode,
+  setIgnoreWhitespacesMode,
+}) => {
   const [t] = useTranslation("repos");
+
   const [isOpen, setOpen] = useState(false);
 
   // This is a hack and it needs to be here until we fix the re rendering problem upon changing the whitespaces in the diffs
   useEffect(() => {
     if (renderOnMount) {
-      ignoreWhitespaces();
-      ignoreWhitespaces();
+      if (setIgnoreWhitespacesMode && ignoreWhitespacesMode) {
+        setIgnoreWhitespacesMode(ignoreWhitespacesMode);
+      } else if (ignoreWhitespaces) {
+        ignoreWhitespaces();
+        ignoreWhitespaces();
+      } else {
+        throw new Error("Neither setIgnoreWhitesspaces nor ignoreWhitespaces set for DiffDropDown!");
+      }
     }
   }, []);
 
   const handleOpen = () => {
     setOpen(!isOpen);
   };
+
+  const handleChange: React.ChangeEventHandler<HTMLInputElement> = (event) => {
+    if (setIgnoreWhitespacesMode) {
+      setIgnoreWhitespacesMode(event.target.checked ? "ALL" : "NONE");
+    }
+  };
+
   return (
     <div className={"dropdown" + (isOpen ? " is-active" : "")}>
       <div className="dropdown-trigger">
@@ -52,7 +80,15 @@ const DiffDropDown: FC<DiffDropDownProps> = ({ collapseDiffs, ignoreWhitespaces,
             <span className="has-text-weight-semibold">{t("changesets.checkBoxHeadingWhitespaces")}</span>
           </div>
           <div className="dropdown-item">
-            <Checkbox onChange={ignoreWhitespaces} label={t("changesets.checkBoxHideWhitespaceChanges")}></Checkbox>
+            {setIgnoreWhitespacesMode && ignoreWhitespacesMode ? (
+              <Checkbox
+                checked={ignoreWhitespacesMode === "ALL"}
+                onChange={handleChange}
+                label={t("changesets.checkBoxHideWhitespaceChanges")}
+              ></Checkbox>
+            ) : (
+              <Checkbox onChange={ignoreWhitespaces} label={t("changesets.checkBoxHideWhitespaceChanges")}></Checkbox>
+            )}
           </div>
           <hr className="dropdown-divider" />
           <div className="dropdown-item">
diff --git a/scm-ui/ui-components/src/repos/LoadingDiff.tsx b/scm-ui/ui-components/src/repos/LoadingDiff.tsx
index 3123852f14..5e8f019e2d 100644
--- a/scm-ui/ui-components/src/repos/LoadingDiff.tsx
+++ b/scm-ui/ui-components/src/repos/LoadingDiff.tsx
@@ -29,6 +29,7 @@ import DiffFileTree from "./diff/DiffFileTree";
 import { DiffContent, FileTreeContent } from "./diff/styledElements";
 import { useHistory, useLocation } from "react-router-dom";
 import { getFileNameFromHash } from "./diffs";
+import { HideWhiteSpaceMode } from "./DiffDropDown";
 
 type Props = DiffObjectProps & {
   url: string;
@@ -55,25 +56,19 @@ const PartialNotification: FC<NotificationProps> = ({ fetchNextPage, isFetchingN
 };
 
 const LoadingDiff: FC<Props> = ({ url, limit, refetchOnWindowFocus, ...props }) => {
-  const [ignoreWhitespace, setIgnoreWhitespace] = useState(false);
+  const [ignoreWhitespace, setIgnoreWhitespace] = useState<HideWhiteSpaceMode>("NONE");
   const [collapsed, setCollapsed] = useState(false);
   const [prevHash, setPrevHash] = useState("");
   const location = useLocation();
   const history = useHistory();
 
-  const evaluateWhiteSpace = () => {
-    return ignoreWhitespace ? "ALL" : "NONE";
-  };
   const { error, isLoading, data, fetchNextPage, isFetchingNextPage } = useDiff(url, {
     limit,
     refetchOnWindowFocus,
-    ignoreWhitespace: evaluateWhiteSpace(),
+    ignoreWhitespace: ignoreWhitespace,
   });
-  const [t] = useTranslation("repos");
 
-  const ignoreWhitespaces = () => {
-    setIgnoreWhitespace(!ignoreWhitespace);
-  };
+  const [t] = useTranslation("repos");
 
   const collapseDiffs = () => {
     setCollapsed(!collapsed);
@@ -108,12 +103,19 @@ const LoadingDiff: FC<Props> = ({ url, limit, refetchOnWindowFocus, ...props })
         <DiffContent>
           <div className="is-flex has-gap-4 mb-4 mt-4 is-justify-content-space-between">
             <DiffStatistics data={data.statistics} />
-            <DiffDropDown collapseDiffs={collapseDiffs} ignoreWhitespaces={ignoreWhitespaces} renderOnMount={true} />
+            <DiffDropDown
+              collapseDiffs={collapseDiffs}
+              renderOnMount={true}
+              ignoreWhitespacesMode={ignoreWhitespace}
+              setIgnoreWhitespacesMode={(hideWhiteSpaceMode: HideWhiteSpaceMode) => {
+                setIgnoreWhitespace(hideWhiteSpaceMode);
+              }}
+            />
           </div>
           <Diff
             defaultCollapse={collapsed}
             diff={data.files}
-            ignoreWhitespace={evaluateWhiteSpace()}
+            ignoreWhitespace={ignoreWhitespace}
             fetchNextPage={fetchNextPage}
             isFetchingNextPage={isFetchingNextPage}
             isDataPartial={data.partial}