mirror of
https://github.com/scm-manager/scm-manager.git
synced 2026-03-01 09:50:48 +01:00
Fix conflicting state behavior in DiffDropDown
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>
This commit is contained in:
2
gradle/changelog/fix-invalid-show-whitspace-state.yaml
Normal file
2
gradle/changelog/fix-invalid-show-whitspace-state.yaml
Normal file
@@ -0,0 +1,2 @@
|
||||
- type: changed
|
||||
description: API to whether hide or show whitespaces inside a diff
|
||||
@@ -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">
|
||||
|
||||
@@ -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}
|
||||
|
||||
Reference in New Issue
Block a user