From 6024d8c3e162b2ecec7443e9580e3feef085d2cf Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Wed, 21 Oct 2020 10:47:05 +0200 Subject: [PATCH 1/6] Unify title usage within an area --- .../src/buttons/OpenInFullscreenButton.tsx | 18 +++++++++++++++--- .../src/repos/sources/containers/Content.tsx | 1 + 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/scm-ui/ui-components/src/buttons/OpenInFullscreenButton.tsx b/scm-ui/ui-components/src/buttons/OpenInFullscreenButton.tsx index 70d4ee5ead..707e86055e 100644 --- a/scm-ui/ui-components/src/buttons/OpenInFullscreenButton.tsx +++ b/scm-ui/ui-components/src/buttons/OpenInFullscreenButton.tsx @@ -26,10 +26,12 @@ import { FC, ReactNode, useState } from "react"; import { useTranslation } from "react-i18next"; import styled from "styled-components"; import FullscreenModal from "../modals/FullscreenModal"; +import Tooltip from "../Tooltip"; type Props = { modalTitle: string; modalBody: ReactNode; + useTitleTooltip?: boolean; // not recommended }; const Button = styled.a` @@ -39,13 +41,14 @@ const Button = styled.a` } `; -const OpenInFullscreenButton: FC = ({ modalTitle, modalBody }) => { +const OpenInFullscreenButton: FC = ({ modalTitle, modalBody, useTitleTooltip = false }) => { const [t] = useTranslation("repos"); const [showModal, setShowModal] = useState(false); - return ( + const tooltip = t("diff.fullscreen.open"); + const content = ( <> - {showModal && ( @@ -58,6 +61,15 @@ const OpenInFullscreenButton: FC = ({ modalTitle, modalBody }) => { )} ); + + if (useTitleTooltip) { + return <>{content}; + } + return ( + + {content} + + ); }; export default OpenInFullscreenButton; diff --git a/scm-ui/ui-webapp/src/repos/sources/containers/Content.tsx b/scm-ui/ui-webapp/src/repos/sources/containers/Content.tsx index 7f827f855a..2f3ccfb3c0 100644 --- a/scm-ui/ui-webapp/src/repos/sources/containers/Content.tsx +++ b/scm-ui/ui-webapp/src/repos/sources/containers/Content.tsx @@ -138,6 +138,7 @@ class Content extends React.Component { {content}} + useTitleTooltip={true} /> Date: Wed, 21 Oct 2020 10:48:45 +0200 Subject: [PATCH 2/6] Fix vertical placement of comments and code in pr diff view --- scm-ui/ui-components/src/repos/DiffFile.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scm-ui/ui-components/src/repos/DiffFile.tsx b/scm-ui/ui-components/src/repos/DiffFile.tsx index 77d44ec595..c977e228a4 100644 --- a/scm-ui/ui-components/src/repos/DiffFile.tsx +++ b/scm-ui/ui-components/src/repos/DiffFile.tsx @@ -93,6 +93,11 @@ const ChangeTypeTag = styled(Tag)` const MarginlessModalContent = styled.div` margin: -1.25rem; + + & .panel-block { + flex-direction: column; + align-items: stretch; + } `; class DiffFile extends React.Component { From 6a93d861fa8cc5906a564a88ae79c21c85985e14 Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Wed, 21 Oct 2020 10:50:14 +0200 Subject: [PATCH 3/6] Fix empty content in modal view of collapsed panel --- scm-ui/ui-components/src/repos/DiffFile.tsx | 31 +++++++++++---------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/scm-ui/ui-components/src/repos/DiffFile.tsx b/scm-ui/ui-components/src/repos/DiffFile.tsx index c977e228a4..e98d6fc39f 100644 --- a/scm-ui/ui-components/src/repos/DiffFile.tsx +++ b/scm-ui/ui-components/src/repos/DiffFile.tsx @@ -415,30 +415,31 @@ class DiffFile extends React.Component { const { file, collapsed, sideBySide, diffExpander, expansionError } = this.state; const viewType = sideBySide ? "split" : "unified"; - let body = null; + const fileAnnotations = fileAnnotationFactory ? fileAnnotationFactory(file) : null; + const innerContent = ( +
+ {fileAnnotations} + + {(hunks: HunkType[]) => + hunks?.map((hunk, n) => { + return this.renderHunk(file, diffExpander.getHunk(n), n); + }) + } + +
+ ); let icon = "angle-right"; + let body = null; if (!collapsed) { - const fileAnnotations = fileAnnotationFactory ? fileAnnotationFactory(file) : null; icon = "angle-down"; - body = ( -
- {fileAnnotations} - - {(hunks: HunkType[]) => - hunks?.map((hunk, n) => { - return this.renderHunk(file, diffExpander.getHunk(n), n); - }) - } - -
- ); + body = innerContent; } const collapseIcon = this.hasContent(file) ? : null; const fileControls = fileControlFactory ? fileControlFactory(file, this.setCollapse) : null; const openInFullscreen = file?.hunks?.length ? ( {body}} + modalBody={{innerContent}} /> ) : null; const sideBySideToggle = file?.hunks?.length && ( From 8995cb4c81f9cc010460b7103ceb810d349fefe6 Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Wed, 21 Oct 2020 11:00:09 +0200 Subject: [PATCH 4/6] Close Modal by clicking into the background --- scm-ui/ui-components/src/modals/Modal.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scm-ui/ui-components/src/modals/Modal.tsx b/scm-ui/ui-components/src/modals/Modal.tsx index d7552ad898..21825e5c75 100644 --- a/scm-ui/ui-components/src/modals/Modal.tsx +++ b/scm-ui/ui-components/src/modals/Modal.tsx @@ -53,7 +53,7 @@ export const Modal: FC = ({ title, closeFunction, body, footer, active, c const modalElement = (
-
+

{title}

From 5ef3e2ed5219c78e3da2730e5f0433b0db74bf59 Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Wed, 21 Oct 2020 11:04:09 +0200 Subject: [PATCH 5/6] Update storyshots --- .../src/__snapshots__/storyshots.test.ts.snap | 1360 ++++++++++------- 1 file changed, 816 insertions(+), 544 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 9bbf3d36c6..55f0977dc9 100644 --- a/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap +++ b/scm-ui/ui-components/src/__snapshots__/storyshots.test.ts.snap @@ -3954,15 +3954,19 @@ exports[`Storyshots Diff Binaries 1`] = `
@@ -4235,15 +4239,19 @@ exports[`Storyshots Diff Collapsed 1`] = `
@@ -4969,15 +5001,19 @@ exports[`Storyshots Diff CollapsingWithFunction 1`] = ` @@ -5823,15 +5859,19 @@ exports[`Storyshots Diff CollapsingWithFunction 1`] = ` @@ -6273,15 +6313,19 @@ exports[`Storyshots Diff CollapsingWithFunction 1`] = ` @@ -6723,15 +6767,19 @@ exports[`Storyshots Diff CollapsingWithFunction 1`] = ` @@ -6794,15 +6842,19 @@ exports[`Storyshots Diff CollapsingWithFunction 1`] = ` @@ -6874,15 +6926,19 @@ exports[`Storyshots Diff Default 1`] = ` @@ -7451,15 +7507,19 @@ exports[`Storyshots Diff Default 1`] = ` @@ -8305,15 +8365,19 @@ exports[`Storyshots Diff Default 1`] = ` @@ -8755,15 +8819,19 @@ exports[`Storyshots Diff Default 1`] = ` @@ -9205,15 +9273,19 @@ exports[`Storyshots Diff Default 1`] = ` @@ -10262,15 +10334,19 @@ exports[`Storyshots Diff Default 1`] = ` @@ -10796,15 +10872,19 @@ exports[`Storyshots Diff Expandable 1`] = ` @@ -11410,15 +11490,19 @@ exports[`Storyshots Diff Expandable 1`] = ` @@ -12361,15 +12445,19 @@ exports[`Storyshots Diff Expandable 1`] = ` @@ -12883,15 +12971,19 @@ exports[`Storyshots Diff Expandable 1`] = ` @@ -13405,15 +13497,19 @@ exports[`Storyshots Diff Expandable 1`] = ` @@ -14617,15 +14713,19 @@ exports[`Storyshots Diff Expandable 1`] = ` @@ -15188,15 +15288,19 @@ exports[`Storyshots Diff File Annotation 1`] = ` @@ -15769,15 +15873,19 @@ exports[`Storyshots Diff File Annotation 1`] = ` @@ -16627,15 +16735,19 @@ exports[`Storyshots Diff File Annotation 1`] = ` @@ -17081,15 +17193,19 @@ exports[`Storyshots Diff File Annotation 1`] = ` @@ -17535,15 +17651,19 @@ exports[`Storyshots Diff File Annotation 1`] = ` @@ -18596,15 +18716,19 @@ exports[`Storyshots Diff File Annotation 1`] = ` @@ -19134,15 +19258,19 @@ exports[`Storyshots Diff File Controls 1`] = ` @@ -24005,15 +24157,19 @@ exports[`Storyshots Diff Line Annotation 1`] = ` @@ -24594,15 +24750,19 @@ exports[`Storyshots Diff Line Annotation 1`] = ` @@ -25460,15 +25620,19 @@ exports[`Storyshots Diff Line Annotation 1`] = ` @@ -25910,15 +26074,19 @@ exports[`Storyshots Diff Line Annotation 1`] = ` @@ -26360,15 +26528,19 @@ exports[`Storyshots Diff Line Annotation 1`] = ` @@ -27417,15 +27589,19 @@ exports[`Storyshots Diff Line Annotation 1`] = ` @@ -27963,15 +28139,19 @@ exports[`Storyshots Diff OnClick 1`] = ` @@ -28580,15 +28760,19 @@ exports[`Storyshots Diff OnClick 1`] = ` @@ -29496,15 +29680,19 @@ exports[`Storyshots Diff OnClick 1`] = ` @@ -29976,15 +30164,19 @@ exports[`Storyshots Diff OnClick 1`] = ` @@ -30456,15 +30648,19 @@ exports[`Storyshots Diff OnClick 1`] = ` @@ -31589,15 +31785,19 @@ exports[`Storyshots Diff OnClick 1`] = ` @@ -32159,15 +32359,19 @@ exports[`Storyshots Diff Side-By-Side 1`] = ` @@ -32829,15 +33033,19 @@ exports[`Storyshots Diff Side-By-Side 1`] = ` @@ -33773,15 +33981,19 @@ exports[`Storyshots Diff Side-By-Side 1`] = ` @@ -34275,15 +34487,19 @@ exports[`Storyshots Diff Side-By-Side 1`] = ` @@ -34777,15 +34993,19 @@ exports[`Storyshots Diff Side-By-Side 1`] = ` @@ -36003,15 +36223,19 @@ exports[`Storyshots Diff Side-By-Side 1`] = ` @@ -36610,15 +36834,19 @@ exports[`Storyshots Diff SyntaxHighlighting 1`] = ` @@ -37187,15 +37415,19 @@ exports[`Storyshots Diff SyntaxHighlighting 1`] = ` @@ -38041,15 +38273,19 @@ exports[`Storyshots Diff SyntaxHighlighting 1`] = ` @@ -38491,15 +38727,19 @@ exports[`Storyshots Diff SyntaxHighlighting 1`] = ` @@ -38941,15 +39181,19 @@ exports[`Storyshots Diff SyntaxHighlighting 1`] = ` @@ -39998,15 +40242,19 @@ exports[`Storyshots Diff SyntaxHighlighting 1`] = ` @@ -40532,15 +40780,19 @@ exports[`Storyshots Diff WithLinkToFile 1`] = ` @@ -41146,15 +41398,19 @@ exports[`Storyshots Diff WithLinkToFile 1`] = ` @@ -42097,15 +42353,19 @@ exports[`Storyshots Diff WithLinkToFile 1`] = ` @@ -42619,15 +42879,19 @@ exports[`Storyshots Diff WithLinkToFile 1`] = ` @@ -43141,15 +43405,19 @@ exports[`Storyshots Diff WithLinkToFile 1`] = ` @@ -44353,15 +44621,19 @@ exports[`Storyshots Diff WithLinkToFile 1`] = ` From e141e33aa341ac387abdac7eb0ae56df9e5b58b7 Mon Sep 17 00:00:00 2001 From: Florian Scholdei Date: Wed, 21 Oct 2020 12:23:58 +0200 Subject: [PATCH 6/6] Clarify prop usage by renaming in more self-evident name --- .../src/buttons/OpenInFullscreenButton.tsx | 12 ++++++++---- .../src/repos/sources/containers/Content.tsx | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/scm-ui/ui-components/src/buttons/OpenInFullscreenButton.tsx b/scm-ui/ui-components/src/buttons/OpenInFullscreenButton.tsx index 707e86055e..c1cd1c6da2 100644 --- a/scm-ui/ui-components/src/buttons/OpenInFullscreenButton.tsx +++ b/scm-ui/ui-components/src/buttons/OpenInFullscreenButton.tsx @@ -31,7 +31,7 @@ import Tooltip from "../Tooltip"; type Props = { modalTitle: string; modalBody: ReactNode; - useTitleTooltip?: boolean; // not recommended + tooltipStyle?: "tooltipComponent" | "htmlTitle"; }; const Button = styled.a` @@ -41,14 +41,18 @@ const Button = styled.a` } `; -const OpenInFullscreenButton: FC = ({ modalTitle, modalBody, useTitleTooltip = false }) => { +const OpenInFullscreenButton: FC = ({ modalTitle, modalBody, tooltipStyle = "tooltipComponent" }) => { const [t] = useTranslation("repos"); const [showModal, setShowModal] = useState(false); const tooltip = t("diff.fullscreen.open"); const content = ( <> - {showModal && ( @@ -62,7 +66,7 @@ const OpenInFullscreenButton: FC = ({ modalTitle, modalBody, useTitleTool ); - if (useTitleTooltip) { + if (tooltipStyle === "htmlTitle") { return <>{content}; } return ( diff --git a/scm-ui/ui-webapp/src/repos/sources/containers/Content.tsx b/scm-ui/ui-webapp/src/repos/sources/containers/Content.tsx index 2f3ccfb3c0..03f2d6dc84 100644 --- a/scm-ui/ui-webapp/src/repos/sources/containers/Content.tsx +++ b/scm-ui/ui-webapp/src/repos/sources/containers/Content.tsx @@ -138,7 +138,7 @@ class Content extends React.Component { {content}} - useTitleTooltip={true} + tooltipStyle="htmlTitle" />