From a4dde3a476d12e01b259938ac682763210a51823 Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Sat, 27 Jan 2024 21:32:41 -0500 Subject: [PATCH 1/2] fix(ui): lazy load markdown title + desc, fix CSS issues etc - `react-markdown` and `remark-gfm` are large deps, so we should code-split / lazy load them - and we should only load them if a user is using this feature - can't quite detect if a string is markdown without a dep, but can at least check if the annotations are used - shaved off ~575KB / ~140KB minified / ~41KB gzipped from the main entrypoint - CSS had several issues - was imported from the wrong component - `overlay` was not sufficiently unique (as we currently use global CSS and not something like CSS Modules) - some unnecessary CSS attributes - most importantly, the positioning of rows broke and did not flex to the last column - it caused columns to not match the heading very confusingly and a lot of empty space on the right as well - that also broke some overflow properties - and it broke the details drawer as well - remove `Link` for the entire row as it was unintuitive, especially with the drawer - one could also not copy+paste things from the row because it caused a click - only link the workflow name now - fix nested link issues - capture the event, prevent bubble up, and process it with JS Signed-off-by: Anton Gilgur --- .../workflows-row/react-markdown-gfm.tsx | 26 ++++++++++ .../workflows-row/workflows-row-name.tsx | 20 -------- .../workflows-row/workflows-row.scss | 14 ------ .../workflows-row/workflows-row.tsx | 49 +++++++++++++------ 4 files changed, 61 insertions(+), 48 deletions(-) create mode 100644 ui/src/app/workflows/components/workflows-row/react-markdown-gfm.tsx delete mode 100644 ui/src/app/workflows/components/workflows-row/workflows-row-name.tsx diff --git a/ui/src/app/workflows/components/workflows-row/react-markdown-gfm.tsx b/ui/src/app/workflows/components/workflows-row/react-markdown-gfm.tsx new file mode 100644 index 000000000000..119f60b6e9f5 --- /dev/null +++ b/ui/src/app/workflows/components/workflows-row/react-markdown-gfm.tsx @@ -0,0 +1,26 @@ +import React from 'react'; +import ReactMarkdown from 'react-markdown'; +import remarkGfm from 'remark-gfm'; + +import {openLinkWithKey} from '../../../shared/components/links'; + +export function ReactMarkdownGfm({markdown}: {markdown: string}) { + return ( + + {markdown} + + ); +} +export default ReactMarkdownGfm; // for lazy loading + +function NestedAnchor(props: React.ComponentProps<'a'>) { + return ( + { + ev.preventDefault(); // don't bubble up + openLinkWithKey(props.href); // eslint-disable-line react/prop-types -- it's not interpreting the prop types correctly + }} + /> + ); +} diff --git a/ui/src/app/workflows/components/workflows-row/workflows-row-name.tsx b/ui/src/app/workflows/components/workflows-row/workflows-row-name.tsx deleted file mode 100644 index 92b240daa5f7..000000000000 --- a/ui/src/app/workflows/components/workflows-row/workflows-row-name.tsx +++ /dev/null @@ -1,20 +0,0 @@ -import * as kubernetes from 'argo-ui/src/models/kubernetes'; -import * as React from 'react'; -import ReactMarkdown from 'react-markdown'; -import remarkGfm from 'remark-gfm'; -import {ANNOTATION_DESCRIPTION, ANNOTATION_TITLE} from '../../../shared/annotations'; - -require('./workflows-row.scss'); - -export const WorkflowsRowName = ({metadata}: {metadata: kubernetes.ObjectMeta}) => { - const title = (metadata.annotations && metadata.annotations[ANNOTATION_TITLE]) || metadata.name; - const description = (metadata.annotations && metadata.annotations[ANNOTATION_DESCRIPTION] && `\n${metadata.annotations[ANNOTATION_DESCRIPTION]}`) || ''; - const markdown = `${title}${description}`; - return ( -
- - {markdown} - -
- ); -}; diff --git a/ui/src/app/workflows/components/workflows-row/workflows-row.scss b/ui/src/app/workflows/components/workflows-row/workflows-row.scss index 9e1f0bdeae04..fca441264981 100644 --- a/ui/src/app/workflows/components/workflows-row/workflows-row.scss +++ b/ui/src/app/workflows/components/workflows-row/workflows-row.scss @@ -3,20 +3,6 @@ .wf-rows-name { line-height: 1.5em; display: inline-block; - position: relative; - pointer-events: none; vertical-align: middle; white-space: pre-line; } -.overlay { - position: absolute; - top: 0; - left: 0; - right: 0; - bottom: 0; -} -.wf-rows-name a { - pointer-events: all; - position: relative; - z-index: 1; -} diff --git a/ui/src/app/workflows/components/workflows-row/workflows-row.tsx b/ui/src/app/workflows/components/workflows-row/workflows-row.tsx index 7b6e04d1e2a1..bbd4be1e4317 100644 --- a/ui/src/app/workflows/components/workflows-row/workflows-row.tsx +++ b/ui/src/app/workflows/components/workflows-row/workflows-row.tsx @@ -1,16 +1,21 @@ import {Ticker} from 'argo-ui/src/index'; import * as React from 'react'; -import {useState} from 'react'; +import {useMemo, useState} from 'react'; import {Link} from 'react-router-dom'; +import type {Plugin} from 'unified'; + import * as models from '../../../../models'; import {isArchivedWorkflow, Workflow} from '../../../../models'; +import {ANNOTATION_DESCRIPTION, ANNOTATION_TITLE} from '../../../shared/annotations'; import {uiUrl} from '../../../shared/base'; +import {Loading} from '../../../shared/components/loading'; import {DurationPanel} from '../../../shared/components/duration-panel'; import {PhaseIcon} from '../../../shared/components/phase-icon'; import {Timestamp} from '../../../shared/components/timestamp'; import {wfDuration} from '../../../shared/duration'; import {WorkflowDrawer} from '../workflow-drawer/workflow-drawer'; -import {WorkflowsRowName} from './workflows-row-name'; + +require('./workflows-row.scss'); interface WorkflowsRowProps { workflow: Workflow; @@ -23,7 +28,11 @@ interface WorkflowsRowProps { export function WorkflowsRow(props: WorkflowsRowProps) { const [hideDrawer, setHideDrawer] = useState(true); const wf = props.workflow; - const workflowLink = uiUrl(`workflows/${wf.metadata.namespace}/${wf.metadata.name}?uid=${wf.metadata.uid}`); + // title + description vars + const title = wf.metadata.annotations?.[ANNOTATION_TITLE] ?? wf.metadata.name; + const description = (wf.metadata.annotations?.[ANNOTATION_DESCRIPTION] && `\n${wf.metadata.annotations[ANNOTATION_DESCRIPTION]}`) || ''; + const hasAnnotation = title !== wf.metadata.name && description !== ''; + const markdown = `${title}${description}`; return (
@@ -42,16 +51,15 @@ export function WorkflowsRow(props: WorkflowsRowProps) { />
-
- - -
- +
+ +
{hasAnnotation ? : markdown}
+
{wf.metadata.namespace}
@@ -96,8 +104,21 @@ export function WorkflowsRow(props: WorkflowsRowProps) { ); })} {hideDrawer ? : } - +
); } + +// lazy load ReactMarkdown (and remark-gfm) as it is a large optional component (which can be split into a separate bundle) +const LazyReactMarkdownGfm = React.lazy(() => { + return import(/* webpackChunkName: "react-markdown-plus-gfm" */ './react-markdown-gfm'); +}); + +function SuspenseReactMarkdownGfm(props: {markdown: string}) { + return ( + }> + + + ); +} From 041db5d5402aa0733ff09a04d2e144d7bf664ac8 Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Sun, 28 Jan 2024 13:46:51 -0500 Subject: [PATCH 2/2] remove no longer used imports - these were from when I was trying to lazy load `remark-gfm` in the same file and for some reason forgot I could just split both imports into one file that I could lazy load / code-split Signed-off-by: Anton Gilgur --- .../app/workflows/components/workflows-row/workflows-row.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/src/app/workflows/components/workflows-row/workflows-row.tsx b/ui/src/app/workflows/components/workflows-row/workflows-row.tsx index bbd4be1e4317..5f8122bf551a 100644 --- a/ui/src/app/workflows/components/workflows-row/workflows-row.tsx +++ b/ui/src/app/workflows/components/workflows-row/workflows-row.tsx @@ -1,8 +1,7 @@ import {Ticker} from 'argo-ui/src/index'; import * as React from 'react'; -import {useMemo, useState} from 'react'; +import {useState} from 'react'; import {Link} from 'react-router-dom'; -import type {Plugin} from 'unified'; import * as models from '../../../../models'; import {isArchivedWorkflow, Workflow} from '../../../../models';