From 903285ad96c52629bb8eda8ce63721782c4e0b89 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Mon, 26 Jul 2021 10:32:10 +0200 Subject: [PATCH] Improve collapse api on diff to enable external state management (#1736) Create new diff collapse api which allows to handle the collapse state externally. --- .../src/__snapshots__/storyshots.test.ts.snap | 4416 +++++++++++++++++ .../ui-components/src/repos/Diff.stories.tsx | 17 + scm-ui/ui-components/src/repos/DiffFile.tsx | 64 +- scm-ui/ui-components/src/repos/DiffTypes.ts | 6 +- 4 files changed, 4475 insertions(+), 28 deletions(-) diff --git a/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap b/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap index 0ca4662c22..490d313f3a 100644 --- a/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap +++ b/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap @@ -20549,6 +20549,4422 @@ exports[`Storyshots Diff Expandable 1`] = ` `; +exports[`Storyshots Diff External state management 1`] = ` +
+
+
+
+
+
+ + + src/main/java/com/cloudogu/scm/review/events/EventListener.java + + + modify + +
+
+
+
+ + + + + +
+
+ + + + + +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ 1 + + 1 + + package com.cloudogu.scm.review.events; +
+ 2 + + 2 + + +
+ 3 + + + import com.cloudogu.scm.review.comment.service.BasicComment; +
+ 4 + + + import com.cloudogu.scm.review.comment.service.BasicCommentEvent; +
+ 5 + + + import com.cloudogu.scm.review.comment.service.CommentEvent; +
+ 6 + + + import com.cloudogu.scm.review.comment.service.ReplyEvent; +
+ 7 + + 3 + + import com.cloudogu.scm.review.pullrequest.service.BasicPullRequestEvent; +
+ 8 + + 4 + + import com.cloudogu.scm.review.pullrequest.service.PullRequest; +
+ 9 + + + import com.cloudogu.scm.review.pullrequest.service.PullRequestEvent; +
+ 10 + + 5 + + import com.github.legman.Subscribe; +
+ 11 + + + import lombok.Data; +
+ 12 + + 6 + + import org.apache.shiro.SecurityUtils; +
+ 13 + + 7 + + import org.apache.shiro.subject.PrincipalCollection; +
+ 14 + + 8 + + import org.apache.shiro.subject.Subject; +
+ 15 + + 9 + + import sonia.scm.EagerSingleton; +
+ 16 + + + import sonia.scm.HandlerEventType; +
+ 17 + + + import sonia.scm.event.HandlerEvent; +
+ 18 + + 10 + + import sonia.scm.plugin.Extension; +
+ 19 + + 11 + + import sonia.scm.repository.Repository; +
+ 20 + + 12 + + import sonia.scm.security.SessionId; +
+
+ + + + diff.expandLastBottomByLines + + + + + + diff.expandLastBottomComplete + +
+
+
+
+
+
+
+
+ + + src/main/js/ChangeNotification.tsx + + + modify + +
+
+
+
+ + + + + +
+
+ + + + + +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+ + + + diff.expandComplete + +
+
+ 2 + + 2 + + import { Link } from "@scm-manager/ui-types"; +
+ 3 + + 3 + + import { apiClient, Toast, ToastButtons, ToastButton } from "@scm-manager/ui-components"; +
+ 4 + + 4 + + import { PullRequest } from "./types/PullRequest"; +
+ + 5 + + import { useTranslation } from "react-i18next"; +
+ 5 + + 6 + + +
+ 6 + + 7 + + type HandlerProps = { +
+ 7 + + 8 + + url: string; +
+
+ + + + diff.expandComplete + +
+
+
+ + + + diff.expandComplete + +
+
+ 15 + + 16 + + pullRequest: setEvent +
+ 16 + + 17 + + }); +
+ 17 + + 18 + + }, [url]); +
+ + 19 + + const { t } = useTranslation("plugins"); +
+ 18 + + 20 + + if (event) { +
+ 19 + + 21 + + return ( +
+ 20 + + + <Toast type="warning" title="New Changes"> +
+ 21 + + + <p>The underlying Pull-Request has changed. Press reload to see the changes.</p> +
+ 22 + + + <p>Warning: Non saved modification will be lost.</p> +
+ + 22 + + <Toast type="warning" title={t("scm-review-plugin.changeNotification.title")}> +
+ + 23 + + <p>{t("scm-review-plugin.changeNotification.description")}</p> +
+ + 24 + + <p>{t("scm-review-plugin.changeNotification.modificationWarning")}</p> +
+ 23 + + 25 + + <ToastButtons> +
+ 24 + + + <ToastButton icon="redo" onClick={reload}>Reload</ToastButton> +
+ 25 + + + <ToastButton icon="times" onClick={() => setEvent(undefined)}>Ignore</ToastButton> +
+ + 26 + + <ToastButton icon="redo" onClick={reload}> +
+ + 27 + + {t("scm-review-plugin.changeNotification.buttons.reload")} +
+ + 28 + + </ToastButton> +
+ + 29 + + <ToastButton icon="times" onClick={() => setEvent(undefined)}> +
+ + 30 + + {t("scm-review-plugin.changeNotification.buttons.ignore")} +
+ + 31 + + </ToastButton> +
+ 26 + + 32 + + </ToastButtons> +
+ 27 + + 33 + + </Toast> +
+ 28 + + 34 + + ); +
+
+ + + + diff.expandLastBottomByLines + + + + + + diff.expandLastBottomComplete + +
+
+
+
+
+
+
+
+ + + src/main/resources/locales/de/plugins.json + + + modify + +
+
+
+
+ + + + + +
+
+ + + + + +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+ + + + diff.expandByLines + + + + + + diff.expandComplete + +
+
+ 181 + + 181 + + "titleClickable": "Der Kommentar bezieht sich auf eine ältere Version des Source- oder Target-Branches. Klicken Sie hier, um den ursprünglichen Kontext zu sehen." +
+ 182 + + 182 + + } +
+ 183 + + 183 + + } +
+ + 184 + + }, +
+ + 185 + + "changeNotification": { +
+ + 186 + + "title": "Neue Änderungen", +
+ + 187 + + "description": "An diesem Pull Request wurden Änderungen vorgenommen. Laden Sie die Seite neu um diese anzuzeigen.", +
+ + 188 + + "modificationWarning": "Warnung: Nicht gespeicherte Eingaben gehen verloren.", +
+ + 189 + + "buttons": { +
+ + 190 + + "reload": "Neu laden", +
+ + 191 + + "ignore": "Ignorieren" +
+ + 192 + + } +
+ 184 + + 193 + + } +
+ 185 + + 194 + + }, +
+ 186 + + 195 + + "permissions": { +
+
+ + + + diff.expandLastBottomByLines + + + + + + diff.expandLastBottomComplete + +
+
+
+
+
+
+
+
+ + + src/main/resources/locales/en/plugins.json + + + modify + +
+
+
+
+ + + + + +
+
+ + + + + +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+ + + + diff.expandByLines + + + + + + diff.expandComplete + +
+
+ 181 + + 181 + + "titleClickable": "The comment is related to an older of the source or target branch. Click here to see the original context." +
+ 182 + + 182 + + } +
+ 183 + + 183 + + } +
+ + 184 + + }, +
+ + 185 + + "changeNotification": { +
+ + 186 + + "title": "New Changes", +
+ + 187 + + "description": "The underlying Pull-Request has changed. Press reload to see the changes.", +
+ + 188 + + "modificationWarning": "Warning: Non saved modification will be lost.", +
+ + 189 + + "buttons": { +
+ + 190 + + "reload": "Reload", +
+ + 191 + + "ignore": "Ignore" +
+ + 192 + + } +
+ 184 + + 193 + + } +
+ 185 + + 194 + + }, +
+ 186 + + 195 + + "permissions": { +
+
+ + + + diff.expandLastBottomByLines + + + + + + diff.expandLastBottomComplete + +
+
+
+
+
+
+
+
+ + + src/test/java/com/cloudogu/scm/review/events/ClientTest.java + + + modify + +
+
+
+
+ + + + + +
+
+ + + + + +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+
+ + + + diff.expandComplete + +
+
+ 7 + + 7 + + import org.mockito.Mock; +
+ 8 + + 8 + + import org.mockito.junit.jupiter.MockitoExtension; +
+ 9 + + 9 + + import sonia.scm.security.SessionId; +
+ + 10 + + +
+ 10 + + 11 + + import javax.ws.rs.sse.OutboundSseEvent; +
+ 11 + + 12 + + import javax.ws.rs.sse.SseEventSink; +
+ 12 + + + +
+ 13 + + 13 + + import java.time.Clock; +
+ 14 + + 14 + + import java.time.Instant; +
+ 15 + + 15 + + import java.time.LocalDateTime; +
+ 16 + + 16 + + import java.time.ZoneOffset; +
+ 17 + + 17 + + import java.time.temporal.ChronoField; +
+ 18 + + + import java.time.temporal.ChronoUnit; +
+ 19 + + + import java.time.temporal.TemporalField; +
+ 20 + + 18 + + import java.util.concurrent.CompletableFuture; +
+ 21 + + 19 + + import java.util.concurrent.CompletionStage; +
+ 22 + + + import java.util.concurrent.atomic.AtomicLong; +
+ 23 + + 20 + + import java.util.concurrent.atomic.AtomicReference; +
+ 24 + + 21 + + +
+ 25 + + 22 + + import static java.time.temporal.ChronoUnit.MINUTES; +
+
+ + + + diff.expandByLines + + + + + + diff.expandComplete + +
+
+
+ + + + diff.expandByLines + + + + + + diff.expandComplete + +
+
+ 83 + + 80 + + +
+ 84 + + 81 + + @Test +
+ 85 + + 82 + + @SuppressWarnings("unchecked") +
+ 86 + + + void shouldCloseEventSinkOnFailure() throws InterruptedException { +
+ + 83 + + void shouldCloseEventSinkOnFailure() { +
+ 87 + + 84 + + CompletionStage future = CompletableFuture.supplyAsync(() -> { +
+ 88 + + 85 + + throw new RuntimeException("failed to send message"); +
+ 89 + + 86 + + }); +
+
+ + + + diff.expandComplete + +
+
+
+ + + + diff.expandComplete + +
+
+ 91 + + 88 + + +
+ 92 + + 89 + + client.send(message); +
+ 93 + + 90 + + +
+ 94 + + + Thread.sleep(50L); +
+ 95 + + + +
+ 96 + + + verify(eventSink).close(); +
+ + 91 + + verify(eventSink, timeout(50L)).close(); +
+ 97 + + 92 + + } +
+ 98 + + 93 + + +
+ 99 + + 94 + + @Test +
+
+ + + + diff.expandLastBottomByLines + + + + + + diff.expandLastBottomComplete + +
+
+
+
+
+
+
+
+ + + Main.java + + + modify + +
+
+
+
+ + + + + +
+
+ + + + + +
+
+
+
+
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + 1 + + import java.io.PrintStream; +
+ 1 + + 2 + + import java.util.Arrays; +
+ 2 + + 3 + + +
+ 3 + + 4 + + class Main { +
+ + 5 + + private static final PrintStream OUT = System.out; +
+ + 6 + + +
+ 4 + + 7 + + public static void main(String[] args) { +
+ + 8 + + <<<<<<< HEAD +
+ 5 + + 9 + + System.out.println("Expect nothing more to happen."); +
+ 6 + + 10 + + System.out.println("The command line parameters are:"); +
+ 7 + + 11 + + Arrays.stream(args).map(arg -> "- " + arg).forEach(System.out::println); +
+ + 12 + + ======= +
+ + 13 + + OUT.println("Expect nothing more to happen."); +
+ + 14 + + OUT.println("Parameters:"); +
+ + 15 + + Arrays.stream(args).map(arg -> "- " + arg).forEach(OUT::println); +
+ + 16 + + >>>>>>> feature/use_constant +
+ 8 + + 17 + + } +
+ 9 + + 18 + + } +
+
+ + + + diff.expandLastBottomByLines + + + + + + diff.expandLastBottomComplete + +
+
+
+
+
+
+`; + exports[`Storyshots Diff File Annotation 1`] = `
ReactNode) => {story()}; const fileControlFactory: (changeset: Changeset) => FileControlFactory = (changeset) => (file) => { @@ -183,4 +187,17 @@ storiesOf("Diff", module) }; return ; + }) + .add("External state management", () => { + const [externalState, setExternalState] = useState({}); + + const isCollapsed = (file: FileDiff) => { + return externalState[file.newPath] || false; + }; + + const onCollapseStateChange = (file: FileDiff) => { + setExternalState((current) => ({ ...current, [file.newPath]: !current[file.newPath] })); + }; + + return ; }); diff --git a/scm-ui/ui-components/src/repos/DiffFile.tsx b/scm-ui/ui-components/src/repos/DiffFile.tsx index 56f7e20ffd..21b1cde77e 100644 --- a/scm-ui/ui-components/src/repos/DiffFile.tsx +++ b/scm-ui/ui-components/src/repos/DiffFile.tsx @@ -97,7 +97,7 @@ const MarginlessModalContent = styled.div` class DiffFile extends React.Component { static defaultProps: Partial = { defaultCollapse: false, - markConflicts: true + markConflicts: true, }; constructor(props: Props) { @@ -106,14 +106,14 @@ class DiffFile extends React.Component { collapsed: this.defaultCollapse(), sideBySide: props.sideBySide, diffExpander: new DiffExpander(props.file), - file: props.file + file: props.file, }; } componentDidUpdate(prevProps: Readonly) { - if (this.props.defaultCollapse !== prevProps.defaultCollapse) { + if (!this.props.isCollapsed && this.props.defaultCollapse !== prevProps.defaultCollapse) { this.setState({ - collapsed: this.defaultCollapse() + collapsed: this.defaultCollapse(), }); } } @@ -130,27 +130,37 @@ class DiffFile extends React.Component { }; toggleCollapse = () => { + const { onCollapseStateChange } = this.props; const { file } = this.state; if (this.hasContent(file)) { - this.setState(state => ({ - collapsed: !state.collapsed - })); + if (onCollapseStateChange) { + onCollapseStateChange(file); + } else { + this.setState((state) => ({ + collapsed: !state.collapsed, + })); + } } }; toggleSideBySide = (callback: () => void) => { this.setState( - state => ({ - sideBySide: !state.sideBySide + (state) => ({ + sideBySide: !state.sideBySide, }), () => callback() ); }; setCollapse = (collapsed: boolean) => { - this.setState({ - collapsed - }); + const { onCollapseStateChange } = this.props; + if (onCollapseStateChange) { + onCollapseStateChange(this.state.file, collapsed); + } else { + this.setState({ + collapsed, + }); + } }; createHunkHeader = (expandableHunk: ExpandableHunk) => { @@ -242,19 +252,13 @@ class DiffFile extends React.Component { expandHead = (expandableHunk: ExpandableHunk, count: number) => { return () => { - return expandableHunk - .expandHead(count) - .then(this.diffExpanded) - .catch(this.diffExpansionFailed); + return expandableHunk.expandHead(count).then(this.diffExpanded).catch(this.diffExpansionFailed); }; }; expandBottom = (expandableHunk: ExpandableHunk, count: number) => { return () => { - return expandableHunk - .expandBottom(count) - .then(this.diffExpanded) - .catch(this.diffExpansionFailed); + return expandableHunk.expandBottom(count).then(this.diffExpanded).catch(this.diffExpansionFailed); }; }; @@ -272,7 +276,7 @@ class DiffFile extends React.Component { if (annotationFactory) { return annotationFactory({ hunk, - file + file, }); } else { return EMPTY_ANNOTATION_FACTORY; @@ -286,7 +290,7 @@ class DiffFile extends React.Component { changeId: getChangeKey(change), change, hunk, - file + file, }; if (onClick) { onClick(context); @@ -299,7 +303,7 @@ class DiffFile extends React.Component { return { onClick: (event: ChangeEvent) => { this.handleClickEvent(event.change, hunk); - } + }, }; } }; @@ -409,12 +413,21 @@ class DiffFile extends React.Component { ); }; + isCollapsed = () => { + const { file, isCollapsed } = this.props; + if (isCollapsed) { + return isCollapsed(file); + } + return this.state.collapsed; + }; + hasContent = (file: FileDiff) => file && !file.isBinary && file.hunks && file.hunks.length > 0; render() { const { fileControlFactory, fileAnnotationFactory, t } = this.props; - const { file, collapsed, sideBySide, diffExpander, expansionError } = this.state; + const { file, sideBySide, diffExpander, expansionError } = this.state; const viewType = sideBySide ? "split" : "unified"; + const collapsed = this.isCollapsed(); const fileAnnotations = fileAnnotationFactory ? fileAnnotationFactory(file) : null; const innerContent = ( @@ -437,9 +450,10 @@ class DiffFile extends React.Component { } const collapseIcon = this.hasContent(file) ? : null; const fileControls = fileControlFactory ? fileControlFactory(file, this.setCollapse) : null; + const modalTitle = file.type === "delete" ? file.oldPath : file.newPath; const openInFullscreen = file?.hunks?.length ? ( {innerContent}} /> ) : null; diff --git a/scm-ui/ui-components/src/repos/DiffTypes.ts b/scm-ui/ui-components/src/repos/DiffTypes.ts index 53a651af99..0147d1833d 100644 --- a/scm-ui/ui-components/src/repos/DiffTypes.ts +++ b/scm-ui/ui-components/src/repos/DiffTypes.ts @@ -40,9 +40,7 @@ export type AnnotationFactoryContext = BaseContext; export type FileAnnotationFactory = (file: File) => ReactNode[]; // key = change id, value = react component -export type AnnotationFactory = ( - context: AnnotationFactoryContext -) => { +export type AnnotationFactory = (context: AnnotationFactoryContext) => { [key: string]: any; }; @@ -63,5 +61,7 @@ export type DiffObjectProps = { annotationFactory?: AnnotationFactory; markConflicts?: boolean; defaultCollapse?: DefaultCollapsed; + isCollapsed?: (file: File) => boolean; + onCollapseStateChange?: (file: File, newState?: boolean) => void; hunkClass?: (hunk: Hunk) => string; };