From b108ed54550179e3e1f2648af68503d478df1aa3 Mon Sep 17 00:00:00 2001 From: Frank Niessink Date: Wed, 3 Jul 2024 19:35:09 +0200 Subject: [PATCH] Refactor tabs. --- .../frontend/src/metric/MetricDetails.js | 156 ++++++------------ components/frontend/src/report/ReportTitle.js | 86 ++-------- .../src/report/ReportsOverviewTitle.js | 48 +----- .../src/semantic_ui_react_wrappers/Icon.js | 15 +- components/frontend/src/source/Source.js | 59 ++----- .../frontend/src/subject/SubjectTitle.js | 44 ++--- .../frontend/src/widgets/FocusableTab.js | 14 -- .../frontend/src/widgets/FocusableTab.test.js | 18 -- .../widgets/{FocusableTab.css => TabPane.css} | 0 components/frontend/src/widgets/TabPane.js | 44 +++++ .../frontend/src/widgets/TabPane.test.js | 24 +++ 11 files changed, 182 insertions(+), 326 deletions(-) delete mode 100644 components/frontend/src/widgets/FocusableTab.js delete mode 100644 components/frontend/src/widgets/FocusableTab.test.js rename components/frontend/src/widgets/{FocusableTab.css => TabPane.css} (100%) create mode 100644 components/frontend/src/widgets/TabPane.js create mode 100644 components/frontend/src/widgets/TabPane.test.js diff --git a/components/frontend/src/metric/MetricDetails.js b/components/frontend/src/metric/MetricDetails.js index 95f060999c..c57ffd873e 100644 --- a/components/frontend/src/metric/MetricDetails.js +++ b/components/frontend/src/metric/MetricDetails.js @@ -1,6 +1,5 @@ import { bool, func, string } from "prop-types" import { useContext, useEffect, useState } from "react" -import { Icon, Menu } from "semantic-ui-react" import { get_metric_measurements } from "../api/measurement" import { delete_metric, set_metric_attribute } from "../api/metric" @@ -8,7 +7,7 @@ import { activeTabIndex, tabChangeHandler } from "../app_ui_settings" 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 { Tab } from "../semantic_ui_react_wrappers" import { datePropType, metricPropType, @@ -21,7 +20,7 @@ import { SourceEntities } from "../source/SourceEntities" import { Sources } from "../source/Sources" import { getMetricScale, getSourceName, isMeasurementRequested } from "../utils" import { ActionButton, DeleteButton, PermLinkButton, ReorderButtonGroup } from "../widgets/Button" -import { FocusableTab } from "../widgets/FocusableTab" +import { changelogTabPane, configurationTabPane, tabPane } from "../widgets/TabPane" import { showMessage } from "../widgets/toast" import { MetricConfigurationParameters } from "./MetricConfigurationParameters" import { MetricDebtParameters } from "./MetricDebtParameters" @@ -124,102 +123,51 @@ export function MetricDetails({ const subject = report.subjects[subject_uuid] const metric = subject.metrics[metric_uuid] const last_measurement = measurements[measurements.length - 1] - let any_error = last_measurement?.sources.some((source) => source.connection_error || source.parse_error) - any_error = - any_error || + let anyError = last_measurement?.sources.some((source) => source.connection_error || source.parse_error) + anyError = + anyError || Object.values(metric.sources ?? {}).some( (source) => !dataModel.metrics[metric.type].sources.includes(source.type), ) - const sources_menu_item = any_error ? : "Sources" const metricUrl = `${window.location.href.split("#")[0]}#${metric_uuid}` let panes = [] panes.push( - { - menuItem: ( - - - {"Configuration"} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {"Technical debt"} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {sources_menu_item} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {"Changelog"} - - ), - render: () => ( - - - - ), - }, + configurationTabPane( + , + ), + tabPane( + "Technical debt", + , + { iconName: "money" }, + ), + tabPane( + "Sources", + , + { iconName: "server", error: Boolean(anyError) }, + ), + changelogTabPane(), ) if (measurements.length > 0) { if (getMetricScale(metric, dataModel) !== "version_number") { - panes.push({ - menuItem: ( - - - {"Trend graph"} - - ), - render: () => ( - - - - ), - }) + panes.push( + tabPane("Trend graph", , { + iconName: "linegraph", + }), + ) } last_measurement.sources.forEach((source) => { const report_source = metric.sources[source.source_uuid] @@ -231,24 +179,18 @@ export function MetricDetails({ return } // no entities to show, continue const source_name = getSourceName(report_source, dataModel) - panes.push({ - menuItem: ( - - {source_name} - - ), - render: () => ( - - - + panes.push( + tabPane( + source_name, + , ), - }) + ) }) } diff --git a/components/frontend/src/report/ReportTitle.js b/components/frontend/src/report/ReportTitle.js index 66ce3d7335..6dbb87f409 100644 --- a/components/frontend/src/report/ReportTitle.js +++ b/components/frontend/src/report/ReportTitle.js @@ -1,5 +1,5 @@ import { func, string } from "prop-types" -import { Grid, Icon, Menu } from "semantic-ui-react" +import { Grid } from "semantic-ui-react" import { delete_report, set_report_attribute } from "../api/report" import { activeTabIndex, tabChangeHandler } from "../app_ui_settings" @@ -14,9 +14,9 @@ import { Label, Segment, Tab } from "../semantic_ui_react_wrappers" import { reportPropType, settingsPropType } from "../sharedPropTypes" import { getDesiredResponseTime } from "../utils" import { DeleteButton, PermLinkButton } from "../widgets/Button" -import { FocusableTab } from "../widgets/FocusableTab" import { HeaderWithDetails } from "../widgets/HeaderWithDetails" import { LabelWithHelp } from "../widgets/LabelWithHelp" +import { changelogTabPane, configurationTabPane, tabPane } from "../widgets/TabPane" import { setDocumentTitle } from "./document_title" import { IssueTracker } from "./IssueTracker" @@ -313,75 +313,19 @@ export function ReportTitle({ report, openReportsOverview, reload, settings }) { const tabIndex = activeTabIndex(settings.expandedItems, report_uuid) const reportUrl = `${window.location}` const panes = [ - { - menuItem: ( - - - {"Configuration"} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {"Desired reaction times"} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {"Notifications"} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {"Issue tracker"} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {"Changelog"} - - ), - render: () => ( - - - - ), - }, + configurationTabPane(), + tabPane("Desired reaction times", , { iconName: "time" }), + tabPane( + "Notifications", + , + { iconName: "feed" }, + ), + tabPane("Issue tracker", , { iconName: "tasks" }), + changelogTabPane(), ] setDocumentTitle(report.title) diff --git a/components/frontend/src/report/ReportsOverviewTitle.js b/components/frontend/src/report/ReportsOverviewTitle.js index cb583768ca..b5f383e1ee 100644 --- a/components/frontend/src/report/ReportsOverviewTitle.js +++ b/components/frontend/src/report/ReportsOverviewTitle.js @@ -1,5 +1,5 @@ import { func, shape } from "prop-types" -import { Grid, Icon, Menu } from "semantic-ui-react" +import { Grid } from "semantic-ui-react" import { set_reports_attribute } from "../api/report" import { activeTabIndex, tabChangeHandler } from "../app_ui_settings" @@ -11,8 +11,8 @@ import { StringInput } from "../fields/StringInput" import { Tab } from "../semantic_ui_react_wrappers" import { permissionsPropType, reportsOverviewPropType, settingsPropType } from "../sharedPropTypes" import { dropdownOptions } from "../utils" -import { FocusableTab } from "../widgets/FocusableTab" import { HeaderWithDetails } from "../widgets/HeaderWithDetails" +import { changelogTabPane, configurationTabPane, tabPane } from "../widgets/TabPane" import { setDocumentTitle } from "./document_title" function ReportsOverviewConfiguration({ reports_overview, reload }) { @@ -106,45 +106,11 @@ export function ReportsOverviewTitle({ reports_overview, reload, settings }) { const uuid = "reports_overview" const tabIndex = activeTabIndex(settings.expandedItems, uuid) const panes = [ - { - menuItem: ( - - - {"Configuration"} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {"Permissions"} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {"Changelog"} - - ), - render: () => ( - - - - ), - }, + configurationTabPane(), + tabPane("Permissions", , { + iconName: "lock", + }), + changelogTabPane(), ] setDocumentTitle(reports_overview.title) diff --git a/components/frontend/src/semantic_ui_react_wrappers/Icon.js b/components/frontend/src/semantic_ui_react_wrappers/Icon.js index 82d744d89d..1222f0f230 100644 --- a/components/frontend/src/semantic_ui_react_wrappers/Icon.js +++ b/components/frontend/src/semantic_ui_react_wrappers/Icon.js @@ -1,8 +1,21 @@ +import { string } from "prop-types" import { useContext } from "react" import { Icon as SemanticUIIcon } from "semantic-ui-react" import { DarkMode } from "../context/DarkMode" export function Icon(props) { - return + let { className, name, ...otherProps } = props + /* Using name="linegraph" results in "Invalid prop `name` of value `linegraph` supplied to `Icon`." + Using name="line graph" does not show the icon. Using className works around this. */ + if (name === "linegraph") { + className = className ?? "" + className += ` ${name}` + name = "" + } + return +} +Icon.propTypes = { + className: string, + name: string, } diff --git a/components/frontend/src/source/Source.js b/components/frontend/src/source/Source.js index 99232b1bd1..51e7b88370 100644 --- a/components/frontend/src/source/Source.js +++ b/components/frontend/src/source/Source.js @@ -1,6 +1,6 @@ import { bool, func, object, oneOfType, string } from "prop-types" import { useContext } from "react" -import { Grid, Menu } from "semantic-ui-react" +import { Grid } from "semantic-ui-react" import { delete_source, set_source_attribute } from "../api/source" import { ChangeLog } from "../changelog/ChangeLog" @@ -8,7 +8,7 @@ import { DataModel } from "../context/DataModel" import { EDIT_REPORT_PERMISSION, ReadOnlyOrEditable } from "../context/Permissions" import { ErrorMessage } from "../errorMessage" import { StringInput } from "../fields/StringInput" -import { Icon, Label, Tab } from "../semantic_ui_react_wrappers" +import { Tab } from "../semantic_ui_react_wrappers" import { measurementSourcePropType, metricPropType, @@ -18,8 +18,8 @@ import { } from "../sharedPropTypes" import { getMetricName, getSourceName } from "../utils" import { DeleteButton, ReorderButtonGroup } from "../widgets/Button" -import { FocusableTab } from "../widgets/FocusableTab" import { HyperLink } from "../widgets/HyperLink" +import { changelogTabPane, configurationTabPane } from "../widgets/TabPane" import { SourceParameters } from "./SourceParameters" import { SourceType } from "./SourceType" import { SourceTypeHeader } from "./SourceTypeHeader" @@ -171,45 +171,22 @@ export function Source({ ) const configError = dataModel.metrics[metric.type].sources.includes(source.type) ? "" : configErrorMessage - const configurationTabLabel = - configError || connectionError || parseError ? : "Configuration" const panes = [ - { - menuItem: ( - - - {configurationTabLabel} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {"Changelog"} - - ), - render: () => ( - - - - ), - }, + configurationTabPane( + , + { error: Boolean(configError || connectionError || parseError) }, + ), + changelogTabPane(), ] return ( <> diff --git a/components/frontend/src/subject/SubjectTitle.js b/components/frontend/src/subject/SubjectTitle.js index 54281f8684..8f91619829 100644 --- a/components/frontend/src/subject/SubjectTitle.js +++ b/components/frontend/src/subject/SubjectTitle.js @@ -1,6 +1,6 @@ import { bool, func, object, string } from "prop-types" import { useContext } from "react" -import { Icon, Menu } from "semantic-ui-react" +import { Icon } from "semantic-ui-react" import { delete_subject, set_subject_attribute } from "../api/subject" import { activeTabIndex, tabChangeHandler } from "../app_ui_settings" @@ -11,9 +11,9 @@ import { Header, Tab } from "../semantic_ui_react_wrappers" import { reportPropType, settingsPropType } from "../sharedPropTypes" import { getSubjectType, slugify } from "../utils" import { DeleteButton, PermLinkButton, ReorderButtonGroup } from "../widgets/Button" -import { FocusableTab } from "../widgets/FocusableTab" import { HeaderWithDetails } from "../widgets/HeaderWithDetails" import { HyperLink } from "../widgets/HyperLink" +import { changelogTabPane, configurationTabPane } from "../widgets/TabPane" import { SubjectParameters } from "./SubjectParameters" function SubjectHeader({ subjectType }) { @@ -82,37 +82,15 @@ export function SubjectTitle({ const subjectTitle = (atReportsOverview ? report.title + " ❯ " : "") + subjectName const subjectUrl = `${window.location}#${subject_uuid}` const panes = [ - { - menuItem: ( - - - {"Configuration"} - - ), - render: () => ( - - - - ), - }, - { - menuItem: ( - - - {"Changelog"} - - ), - render: () => ( - - - - ), - }, + configurationTabPane( + , + ), + changelogTabPane(), ] return ( diff --git a/components/frontend/src/widgets/FocusableTab.js b/components/frontend/src/widgets/FocusableTab.js deleted file mode 100644 index 1e156b86a4..0000000000 --- a/components/frontend/src/widgets/FocusableTab.js +++ /dev/null @@ -1,14 +0,0 @@ -import "./FocusableTab.css" - -import { useContext } from "react" - -import { DarkMode } from "../context/DarkMode" -import { childrenPropType } from "../sharedPropTypes" - -export function FocusableTab(props) { - const className = useContext(DarkMode) ? "tabbutton inverted" : "tabbutton" - return -} -FocusableTab.propTypes = { - children: childrenPropType, -} diff --git a/components/frontend/src/widgets/FocusableTab.test.js b/components/frontend/src/widgets/FocusableTab.test.js deleted file mode 100644 index c0bfbedfe2..0000000000 --- a/components/frontend/src/widgets/FocusableTab.test.js +++ /dev/null @@ -1,18 +0,0 @@ -import { render, screen } from "@testing-library/react" - -import { DarkMode } from "../context/DarkMode" -import { FocusableTab } from "./FocusableTab" - -it("shows the tab", () => { - render(Tab) - expect(screen.queryAllByText("Tab").length).toBe(1) -}) - -it("is inverted in dark mode", () => { - const { container } = render( - - Tab - , - ) - expect(container.firstChild.className).toEqual(expect.stringContaining("inverted")) -}) diff --git a/components/frontend/src/widgets/FocusableTab.css b/components/frontend/src/widgets/TabPane.css similarity index 100% rename from components/frontend/src/widgets/FocusableTab.css rename to components/frontend/src/widgets/TabPane.css diff --git a/components/frontend/src/widgets/TabPane.js b/components/frontend/src/widgets/TabPane.js new file mode 100644 index 0000000000..82a2e18804 --- /dev/null +++ b/components/frontend/src/widgets/TabPane.js @@ -0,0 +1,44 @@ +import "./TabPane.css" + +import { bool, element, oneOfType, string } from "prop-types" +import { useContext } from "react" +import { Menu } from "semantic-ui-react" + +import { DarkMode } from "../context/DarkMode" +import { Icon, Label, Tab } from "../semantic_ui_react_wrappers" + +function FocusableTab({ error, iconName, label }) { + const className = useContext(DarkMode) ? "tabbutton inverted" : "tabbutton" + const tabLabel = error ? : label + return ( + <> + {iconName && } + + + ) +} +FocusableTab.propTypes = { + error: bool, + iconName: string, + label: oneOfType([element, string]), +} + +export function tabPane(label, pane, options) { + // Return a tab and pane, to be used as follows: + return { + menuItem: ( + + + + ), + render: () => {pane}, + } +} + +export function configurationTabPane(pane, options) { + return tabPane("Configuration", pane, { ...options, iconName: "settings" }) +} + +export function changelogTabPane(pane, options) { + return tabPane("Changelog", pane, { ...options, iconName: "history" }) +} diff --git a/components/frontend/src/widgets/TabPane.test.js b/components/frontend/src/widgets/TabPane.test.js new file mode 100644 index 0000000000..62c9d70b25 --- /dev/null +++ b/components/frontend/src/widgets/TabPane.test.js @@ -0,0 +1,24 @@ +import { render, screen } from "@testing-library/react" + +import { DarkMode } from "../context/DarkMode" +import { Tab } from "../semantic_ui_react_wrappers" +import { tabPane } from "./TabPane" + +it("shows the tab", () => { + render() + expect(screen.queryAllByText("Tab").length).toBe(1) +}) + +it("is inverted in dark mode", () => { + const { container } = render( + + + , + ) + expect(container.firstChild.firstChild.className).toEqual(expect.stringContaining("inverted")) +}) + +it("shows the tab red when there is an error", () => { + render(Pane

, { error: true })]} />) + expect(screen.getByText("Tab").className).toEqual(expect.stringContaining("red")) +})