From 08ebecb042a1b9d854f302c625df206d5496bb25 Mon Sep 17 00:00:00 2001
From: Frank Niessink <frank@niessink.com>
Date: Wed, 19 Jun 2024 09:26:48 +0200
Subject: [PATCH] Move the share buttons to the headers.

Remove the share tabs for reports, subjects, and metrics and move the share button to the report, subject, and metric headers.

Closes #8821.
---
 .../frontend/src/metric/MetricDetails.js      | 21 ++----
 .../frontend/src/metric/MetricDetails.test.js | 13 ++--
 .../frontend/src/subject/SubjectTitle.js      | 21 ++----
 .../frontend/src/subject/SubjectTitle.test.js |  8 ---
 components/frontend/src/widgets/Button.js     | 71 ++++++-------------
 .../frontend/src/widgets/Button.test.js       | 44 ++++--------
 docs/src/changelog.md                         |  1 +
 7 files changed, 51 insertions(+), 128 deletions(-)

diff --git a/components/frontend/src/metric/MetricDetails.js b/components/frontend/src/metric/MetricDetails.js
index 9d0a071775..1e1420bf5e 100644
--- a/components/frontend/src/metric/MetricDetails.js
+++ b/components/frontend/src/metric/MetricDetails.js
@@ -9,7 +9,6 @@ import { ChangeLog } from "../changelog/ChangeLog"
 import { DataModel } from "../context/DataModel"
 import { EDIT_REPORT_PERMISSION, ReadOnlyOrEditable } from "../context/Permissions"
 import { Label, Tab } from "../semantic_ui_react_wrappers"
-import { Share } from "../share/Share"
 import {
     datePropType,
     reportPropType,
@@ -20,7 +19,7 @@ import {
 import { SourceEntities } from "../source/SourceEntities"
 import { Sources } from "../source/Sources"
 import { getMetricScale, getSourceName } from "../utils"
-import { DeleteButton, ReorderButtonGroup } from "../widgets/Button"
+import { DeleteButton, PermLinkButton, ReorderButtonGroup } from "../widgets/Button"
 import { FocusableTab } from "../widgets/FocusableTab"
 import { showMessage } from "../widgets/toast"
 import { MetricConfigurationParameters } from "./MetricConfigurationParameters"
@@ -28,7 +27,7 @@ import { MetricDebtParameters } from "./MetricDebtParameters"
 import { MetricTypeHeader } from "./MetricTypeHeader"
 import { TrendGraph } from "./TrendGraph"
 
-function Buttons({ isFirstMetric, isLastMetric, metric_uuid, reload, stopFilteringAndSorting }) {
+function Buttons({ isFirstMetric, isLastMetric, metric_uuid, reload, stopFilteringAndSorting, url }) {
     return (
         <ReadOnlyOrEditable
             requiredPermissions={[EDIT_REPORT_PERMISSION]}
@@ -44,6 +43,7 @@ function Buttons({ isFirstMetric, isLastMetric, metric_uuid, reload, stopFilteri
                             set_metric_attribute(metric_uuid, "position", direction, reload)
                         }}
                     />
+                    <PermLinkButton itemType="metric" url={url} />
                     <DeleteButton itemType="metric" onClick={() => delete_metric(metric_uuid, reload)} />
                 </div>
             }
@@ -56,6 +56,7 @@ Buttons.propTypes = {
     metric_uuid: string,
     reload: func,
     stopFilteringAndSorting: func,
+    url: string,
 }
 
 function fetchMeasurements(reportDate, metric_uuid, setMeasurements) {
@@ -176,19 +177,6 @@ export function MetricDetails({
                 </Tab.Pane>
             ),
         },
-        {
-            menuItem: (
-                <Menu.Item key="share">
-                    <Icon name="share square" />
-                    <FocusableTab>{"Share"}</FocusableTab>
-                </Menu.Item>
-            ),
-            render: () => (
-                <Tab.Pane>
-                    <Share title="Metric permanent link" url={metricUrl} />
-                </Tab.Pane>
-            ),
-        },
     )
     if (measurements.length > 0) {
         if (getMetricScale(metric, dataModel) !== "version_number") {
@@ -251,6 +239,7 @@ export function MetricDetails({
                 isLastMetric={isLastMetric}
                 reload={reload}
                 stopFilteringAndSorting={stopFilteringAndSorting}
+                url={metricUrl}
             />
         </>
     )
diff --git a/components/frontend/src/metric/MetricDetails.test.js b/components/frontend/src/metric/MetricDetails.test.js
index b3a556e796..e9e4ef295c 100644
--- a/components/frontend/src/metric/MetricDetails.test.js
+++ b/components/frontend/src/metric/MetricDetails.test.js
@@ -141,18 +141,15 @@ it("does not show the trend graph tab if the metric scale is version number", as
     expect(screen.queryAllByText(/Trend graph/).length).toBe(0)
 })
 
-it("switches tabs to the share tab", async () => {
-    await renderMetricDetails()
-    expect(screen.getAllByText(/Metric name/).length).toBe(1)
-    fireEvent.click(screen.getByText(/Share/))
-    expect(screen.getAllByText(/Metric permanent link/).length).toBe(1)
-})
-
 it("removes the existing hashtag from the URL to share", async () => {
     history.push("#hash_that_should_be_removed")
+    Object.assign(window, { isSecureContext: true })
+    Object.assign(navigator, {
+        clipboard: { writeText: jest.fn().mockImplementation(() => Promise.resolve()) },
+    })
     await renderMetricDetails()
     fireEvent.click(screen.getByText(/Share/))
-    expect(screen.getByTestId("permlink").value).toBe("http://localhost/#metric_uuid")
+    expect(navigator.clipboard.writeText).toHaveBeenCalledWith("http://localhost/#metric_uuid")
 })
 
 it("displays whether sources have errors", async () => {
diff --git a/components/frontend/src/subject/SubjectTitle.js b/components/frontend/src/subject/SubjectTitle.js
index c3b99686bf..54281f8684 100644
--- a/components/frontend/src/subject/SubjectTitle.js
+++ b/components/frontend/src/subject/SubjectTitle.js
@@ -8,10 +8,9 @@ import { ChangeLog } from "../changelog/ChangeLog"
 import { DataModel } from "../context/DataModel"
 import { EDIT_REPORT_PERMISSION, ReadOnlyOrEditable } from "../context/Permissions"
 import { Header, Tab } from "../semantic_ui_react_wrappers"
-import { Share } from "../share/Share"
 import { reportPropType, settingsPropType } from "../sharedPropTypes"
 import { getSubjectType, slugify } from "../utils"
-import { DeleteButton, ReorderButtonGroup } from "../widgets/Button"
+import { DeleteButton, PermLinkButton, ReorderButtonGroup } from "../widgets/Button"
 import { FocusableTab } from "../widgets/FocusableTab"
 import { HeaderWithDetails } from "../widgets/HeaderWithDetails"
 import { HyperLink } from "../widgets/HyperLink"
@@ -37,7 +36,7 @@ SubjectHeader.propTypes = {
     subjectType: object,
 }
 
-function ButtonRow({ subject_uuid, firstSubject, lastSubject, reload }) {
+function ButtonRow({ subject_uuid, firstSubject, lastSubject, reload, url }) {
     return (
         <ReadOnlyOrEditable
             requiredPermissions={[EDIT_REPORT_PERMISSION]}
@@ -51,6 +50,7 @@ function ButtonRow({ subject_uuid, firstSubject, lastSubject, reload }) {
                             set_subject_attribute(subject_uuid, "position", direction, reload)
                         }}
                     />
+                    <PermLinkButton itemType="subject" url={url} />
                     <DeleteButton itemType="subject" onClick={() => delete_subject(subject_uuid, reload)} />
                 </>
             }
@@ -62,6 +62,7 @@ ButtonRow.propTypes = {
     firstSubject: bool,
     lastSubject: bool,
     reload: func,
+    url: string,
 }
 
 export function SubjectTitle({
@@ -112,19 +113,6 @@ export function SubjectTitle({
                 </Tab.Pane>
             ),
         },
-        {
-            menuItem: (
-                <Menu.Item key="share">
-                    <Icon name="share square" />
-                    <FocusableTab>{"Share"}</FocusableTab>
-                </Menu.Item>
-            ),
-            render: () => (
-                <Tab.Pane>
-                    <Share title="Subject permanent link" url={subjectUrl} />
-                </Tab.Pane>
-            ),
-        },
     ]
 
     return (
@@ -149,6 +137,7 @@ export function SubjectTitle({
                     firstSubject={firstSubject}
                     lastSubject={lastSubject}
                     reload={reload}
+                    url={subjectUrl}
                 />
             </div>
         </HeaderWithDetails>
diff --git a/components/frontend/src/subject/SubjectTitle.test.js b/components/frontend/src/subject/SubjectTitle.test.js
index 41ed1b2b73..0110b9db48 100644
--- a/components/frontend/src/subject/SubjectTitle.test.js
+++ b/components/frontend/src/subject/SubjectTitle.test.js
@@ -105,14 +105,6 @@ it("loads the changelog", async () => {
     expect(fetch_server_api.fetch_server_api).toHaveBeenCalledWith("get", "changelog/subject/subject_uuid/5")
 })
 
-it("shows the share tab", async () => {
-    await renderSubjectTitle()
-    await act(async () => {
-        fireEvent.click(screen.getByText(/Share/))
-    })
-    expect(screen.getAllByText(/Subject permanent link/).length).toBe(1)
-})
-
 it("moves the subject", async () => {
     fetch_server_api.fetch_server_api = jest.fn().mockResolvedValue({ ok: true })
     await renderSubjectTitle()
diff --git a/components/frontend/src/widgets/Button.js b/components/frontend/src/widgets/Button.js
index 7d0da853d4..65334eb450 100644
--- a/components/frontend/src/widgets/Button.js
+++ b/components/frontend/src/widgets/Button.js
@@ -2,7 +2,7 @@ import { array, arrayOf, bool, func, string } from "prop-types"
 import { useState } from "react"
 import { Icon, Input } from "semantic-ui-react"
 
-import { Button, Checkbox, Dropdown, Label, Popup } from "../semantic_ui_react_wrappers"
+import { Button, Checkbox, Dropdown, Popup } from "../semantic_ui_react_wrappers"
 import { popupContentPropType } from "../sharedPropTypes"
 import { showMessage } from "../widgets/toast"
 import { ItemBreadcrumb } from "./ItemBreadcrumb"
@@ -296,7 +296,7 @@ ReorderButton.propTypes = {
 
 export function ReorderButtonGroup(props) {
     return (
-        <Button.Group style={{ marginTop: "0px" }}>
+        <Button.Group style={{ marginTop: "0px", marginRight: "5px" }}>
             <ReorderButton {...props} direction="first" />
             <ReorderButton {...props} direction="previous" />
             <ReorderButton {...props} direction="next" />
@@ -364,59 +364,32 @@ export function MoveButton(props) {
     return <ActionAndItemPickerButton {...props} action="Move" icon="shuffle" />
 }
 
-export function PermLinkButton({ url }) {
-    if (navigator.clipboard) {
+export function PermLinkButton({ itemType, url }) {
+    if (window.isSecureContext) {
         // Frontend runs in a secure context (https) so we can use the Clipboard API
         return (
-            <Button
-                as="div"
-                labelPosition="right"
-                onClick={() =>
-                    navigator.clipboard
-                        .writeText(url)
-                        .then(() => showMessage("success", "Copied URL to clipboard"))
-                        .catch((error) => showMessage("error", "Could not copy URL to clipboard", `${error}`))
+            <Popup
+                content={`Copy a permanent link to this ${itemType} to the clipboard`}
+                trigger={
+                    <Button
+                        basic
+                        content="Share"
+                        icon="share square"
+                        onClick={() =>
+                            navigator.clipboard
+                                .writeText(url)
+                                .then(() => showMessage("success", "Copied URL to clipboard"))
+                                .catch((error) => showMessage("error", "Could not copy URL to clipboard", `${error}`))
+                        }
+                        primary
+                    />
                 }
-            >
-                <Button basic content="Copy" icon="copy" primary />
-                <Label as="a" color="blue">
-                    {url}
-                </Label>
-            </Button>
-        )
-    } else {
-        // Frontend does not run in a secure context (https) so we cannot use the Clipboard API, and have
-        // to use the deprecated Document.execCommand. As document.exeCommand expects selected text, we also
-        // cannot use the Label component but have to use a (read only) input element so we can select the URL
-        // before copying it to the clipboard.
-        return (
-            <Input action actionPosition="left" color="blue" defaultValue={url} fluid readOnly>
-                <Button
-                    basic
-                    color="blue"
-                    content="Copy"
-                    icon="copy"
-                    onClick={() => {
-                        let urlText = document.querySelector("#permlink")
-                        urlText.select()
-                        document.execCommand("copy")
-                        showMessage("success", "Copied URL to clipboard")
-                    }}
-                    style={{ fontWeight: "bold" }}
-                />
-                <input
-                    data-testid="permlink"
-                    id="permlink"
-                    style={{
-                        border: "1px solid rgb(143, 208, 255)",
-                        color: "rgb(143, 208, 255)",
-                        fontWeight: "bold",
-                    }}
-                />
-            </Input>
+            />
         )
     }
+    return null
 }
 PermLinkButton.propTypes = {
+    itemType: string,
     url: string,
 }
diff --git a/components/frontend/src/widgets/Button.test.js b/components/frontend/src/widgets/Button.test.js
index 78859116ed..885a871f08 100644
--- a/components/frontend/src/widgets/Button.test.js
+++ b/components/frontend/src/widgets/Button.test.js
@@ -314,56 +314,38 @@ Array("first", "last", "previous", "next").forEach((direction) => {
     })
 })
 
-test("PermLinkButton copies url to clipboard if not in a secure context", () => {
-    Object.assign(document, { execCommand: jest.fn() })
-    render(<PermLinkButton url="https://example.org" />)
-    fireEvent.click(screen.getByText(/Copy/))
-    expect(document.execCommand).toHaveBeenCalledWith("copy")
+test("PermLinkButton is not shown in an insecure context", () => {
+    Object.assign(window, { isSecureContext: false })
+    render(<PermLinkButton itemType="metric" url="https://example.org" />)
+    expect(screen.queryAllByText(/Share/).length).toBe(0)
 })
 
-test("PermLinkButton shows success message if not in a secure context", async () => {
+test("PermLinkButton copies URL to clipboard", async () => {
     toast.showMessage = jest.fn()
-    Object.assign(document, { execCommand: jest.fn() })
-    render(<PermLinkButton url="https://example.org" />)
-    await act(async () => {
-        fireEvent.click(screen.getByText(/Copy/))
-    })
-    expect(toast.showMessage).toHaveBeenCalledWith("success", "Copied URL to clipboard")
-})
-
-test("PermLinkButton copies URL to clipboard if in a secure context", async () => {
+    Object.assign(window, { isSecureContext: true })
     Object.assign(navigator, {
         clipboard: { writeText: jest.fn().mockImplementation(() => Promise.resolve()) },
     })
-    render(<PermLinkButton url="https://example.org" />)
+    render(<PermLinkButton itemType="metric" url="https://example.org" />)
+    screen.debug()
     await act(async () => {
-        fireEvent.click(screen.getByText(/example.org/))
+        fireEvent.click(screen.getByText(/Share/))
     })
     expect(navigator.clipboard.writeText).toHaveBeenCalledWith("https://example.org")
-})
-
-test("PermLinkButton shows success message if in a secure context", async () => {
-    toast.showMessage = jest.fn()
-    Object.assign(navigator, {
-        clipboard: { writeText: jest.fn().mockImplementation(() => Promise.resolve()) },
-    })
-    render(<PermLinkButton url="https://example.org" />)
-    await act(async () => {
-        fireEvent.click(screen.getByText(/example.org/))
-    })
     expect(toast.showMessage).toHaveBeenCalledWith("success", "Copied URL to clipboard")
 })
 
-test("PermLinkButton shows error message if in a secure context", async () => {
+test("PermLinkButton shows error message if copying fails", async () => {
     toast.showMessage = jest.fn()
+    Object.assign(window, { isSecureContext: true })
     Object.assign(navigator, {
         clipboard: {
             writeText: jest.fn().mockImplementation(() => Promise.reject(new Error("fail"))),
         },
     })
-    render(<PermLinkButton url="https://example.org" />)
+    render(<PermLinkButton itemType="metric" url="https://example.org" />)
     await act(async () => {
-        fireEvent.click(screen.getByText(/example.org/))
+        fireEvent.click(screen.getByText(/Share/))
     })
     expect(toast.showMessage).toHaveBeenCalledWith("error", "Could not copy URL to clipboard", "Error: fail")
 })
diff --git a/docs/src/changelog.md b/docs/src/changelog.md
index ae5b0ceb03..fe80d568e5 100644
--- a/docs/src/changelog.md
+++ b/docs/src/changelog.md
@@ -40,6 +40,7 @@ If your currently installed *Quality-time* version is not v5.13.0, please first
 
   In addition:
   - A new parameter 'clean code attributes category' is added.
+- Remove the share tabs for reports, subjects, and metrics and move the share button to the report, subject, and metric headers. Closes [#8821](https://github.com/ICTU/quality-time/issues/8821).
 - Set the MongoDB feature compatibility version to v7. Closes [#8896](https://github.com/ICTU/quality-time/issues/8896).
 
 ## v5.13.0 - 2024-05-23