From 88cd3c215b29aae01393404e60d92eefa2e2df65 Mon Sep 17 00:00:00 2001 From: Mike Burgess Date: Tue, 9 Aug 2022 21:12:52 +0100 Subject: [PATCH] Dashboard UI hrefs should still be enabled for snapshots when the URL is relative #2311. --- .../src/components/DashboardList/index.tsx | 40 ++++++++++++------- .../src/components/dashboards/Card/index.tsx | 22 ++++++---- .../src/components/dashboards/Table/index.tsx | 23 ++++++++--- ui/dashboard/src/utils/url.test.ts | 27 +++++++++++++ ui/dashboard/src/utils/url.ts | 7 ++++ 5 files changed, 90 insertions(+), 29 deletions(-) create mode 100644 ui/dashboard/src/utils/url.test.ts create mode 100644 ui/dashboard/src/utils/url.ts diff --git a/ui/dashboard/src/components/DashboardList/index.tsx b/ui/dashboard/src/components/DashboardList/index.tsx index 35be247fd..cefe26d23 100644 --- a/ui/dashboard/src/components/DashboardList/index.tsx +++ b/ui/dashboard/src/components/DashboardList/index.tsx @@ -10,6 +10,7 @@ import { } from "../../hooks/useDashboard"; import CallToActions from "../CallToActions"; import LoadingIndicator from "../dashboards/LoadingIndicator"; +import { classNames } from "../../utils/styles"; import { default as lodashGroupBy } from "lodash/groupBy"; import { Fragment, useEffect, useState } from "react"; import { stringToColour } from "../../utils/color"; @@ -27,8 +28,8 @@ type AvailableDashboardWithMod = AvailableDashboard & { interface DashboardTagProps { tagKey: string; tagValue: string; - dispatch: (action: DashboardAction) => void; - searchValue: string; + dispatch?: (action: DashboardAction) => void; + searchValue?: string; } interface SectionProps { @@ -45,19 +46,26 @@ const DashboardTag = ({ searchValue, }: DashboardTagProps) => ( { - const existingSearch = searchValue.trim(); - const searchWithTag = existingSearch - ? existingSearch.indexOf(tagValue) < 0 - ? `${existingSearch} ${tagValue}` - : existingSearch - : tagValue; - dispatch({ - type: DashboardActions.SET_DASHBOARD_SEARCH_VALUE, - value: searchWithTag, - }); - }} + className={classNames( + "rounded-md text-xs", + dispatch ? "cursor-pointer" : null + )} + onClick={ + dispatch + ? () => { + const existingSearch = searchValue ? searchValue.trim() : ""; + const searchWithTag = existingSearch + ? existingSearch.indexOf(tagValue) < 0 + ? `${existingSearch} ${tagValue}` + : existingSearch + : tagValue; + dispatch({ + type: DashboardActions.SET_DASHBOARD_SEARCH_VALUE, + value: searchWithTag, + }); + } + : undefined + } style={{ color: stringToColour(tagValue) }} title={`${tagKey} = ${tagValue}`} > @@ -425,3 +433,5 @@ const DashboardListWrapper = ({ wrapperClassName = "" }) => { }; export default DashboardListWrapper; + +export { DashboardTag }; diff --git a/ui/dashboard/src/components/dashboards/Card/index.tsx b/ui/dashboard/src/components/dashboards/Card/index.tsx index 27ebf04da..98fac2194 100644 --- a/ui/dashboard/src/components/dashboards/Card/index.tsx +++ b/ui/dashboard/src/components/dashboards/Card/index.tsx @@ -18,6 +18,7 @@ import { getWrapperClasses, } from "../../../utils/card"; import { getComponent, registerComponent } from "../index"; +import { isRelativeUrl } from "../../../utils/url"; import { renderInterpolatedTemplates } from "../../../utils/template"; import { ThemeNames } from "../../../hooks/useTheme"; import { useDashboard } from "../../../hooks/useDashboard"; @@ -185,17 +186,14 @@ const Card = (props: CardProps) => { }, [setZoomIconClassName, textClasses]); useEffect(() => { - if ( - dataMode === "snapshot" || - ((state.loading || !state.href) && (renderError || renderedHref)) - ) { + if ((state.loading || !state.href) && (renderError || renderedHref)) { setRenderError(null); setRenderedHref(null); } - }, [dataMode, state.loading, state.href, renderError, renderedHref]); + }, [state.loading, state.href, renderError, renderedHref]); useDeepCompareEffect(() => { - if (dataMode === "snapshot" || state.loading || !state.href) { + if (state.loading || !state.href) { return; } // const { label, loading, value, ...rest } = state; @@ -222,7 +220,15 @@ const Card = (props: CardProps) => { setRenderedHref(null); setRenderError(null); } else if (renderedResults[0].card.result) { - setRenderedHref(renderedResults[0].card.result as string); + // We only want to render the HREF if it's live, or it's snapshot and absolute + const isRelative = isRelativeUrl( + renderedResults[0].card.result as string + ); + setRenderedHref( + dataMode === "snapshot" && isRelative + ? null + : (renderedResults[0].card.result as string) + ); setRenderError(null); } else if (renderedResults[0].card.error) { setRenderError(renderedResults[0].card.error as string); @@ -230,7 +236,7 @@ const Card = (props: CardProps) => { } }; doRender(); - }, [dataMode, state, props.data]); + }, [state, props.data]); const card = (
{ interface CellValueProps { column: TableColumnInfo; + dataMode: DashboardDataMode; rowIndex: number; rowTemplateData: RowRenderResult[]; value: any; @@ -106,6 +108,7 @@ interface CellValueProps { const CellValue = ({ column, + dataMode, rowIndex, rowTemplateData, value, @@ -132,7 +135,13 @@ const CellValue = ({ return; } if (renderedTemplateForColumn.result) { - setHref(renderedTemplateForColumn.result); + // We only want to render the HREF if it's live, or it's snapshot and absolute + const isRelative = isRelativeUrl(renderedTemplateForColumn.result); + setHref( + dataMode === "snapshot" && isRelative + ? null + : renderedTemplateForColumn.result + ); setError(null); } else if (renderedTemplateForColumn.error) { setHref(null); @@ -396,13 +405,13 @@ const TableView = ({ setRowTemplateData(renderedResults); }; - if (dataMode === "snapshot" || columns.length === 0 || rows.length === 0) { + if (columns.length === 0 || rows.length === 0) { setRowTemplateData([]); return; } doRender(); - }, [dataMode, columns, rows]); + }, [columns, rows]); return ( <> @@ -483,6 +492,7 @@ const TableView = ({ > { setRowTemplateData(renderedResults); }; - if (dataMode === "snapshot" || columns.length === 0 || rows.length === 0) { + if (columns.length === 0 || rows.length === 0) { setRowTemplateData([]); return; } doRender(); - }, [dataMode, columns, rows]); + }, [columns, rows]); if (columns.length === 0 || rows.length === 0) { return null; @@ -610,6 +620,7 @@ const LineView = (props: TableProps) => { > { + test("null", () => { + expect(isRelativeUrl(null)).toEqual(true); + }); + + test("undefined", () => { + expect(isRelativeUrl(undefined)).toEqual(true); + }); + + test("empty", () => { + expect(isRelativeUrl("")).toEqual(true); + }); + + test("relative", () => { + expect(isRelativeUrl("/foo/bar")).toEqual(true); + }); + + test("relative with query string", () => { + expect(isRelativeUrl("/foo/bar?bar=foo")).toEqual(true); + }); + + test("absolute", () => { + expect(isRelativeUrl("https://foo.bar")).toEqual(false); + }); +}); diff --git a/ui/dashboard/src/utils/url.ts b/ui/dashboard/src/utils/url.ts new file mode 100644 index 000000000..f96f204b5 --- /dev/null +++ b/ui/dashboard/src/utils/url.ts @@ -0,0 +1,7 @@ +const isRelativeUrl = (url) => { + return ( + new URL(document.baseURI).origin === new URL(url, document.baseURI).origin + ); +}; + +export { isRelativeUrl };