Skip to content

Commit

Permalink
fix(ui): lazy load markdown title + desc, fix CSS issues etc
Browse files Browse the repository at this point in the history
- `react-markdown` and `remark-gfm` are large deps, so we should 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

- 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 <agilgur5@gmail.com>
  • Loading branch information
agilgur5 committed Jan 28, 2024
1 parent baef485 commit ab6bf33
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 48 deletions.
Original file line number Diff line number Diff line change
@@ -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 (
<ReactMarkdown components={{p: React.Fragment, a: NestedAnchor}} remarkPlugins={[remarkGfm]}>
{markdown}
</ReactMarkdown>
);
}
export default ReactMarkdownGfm; // for lazy loading

function NestedAnchor(props: React.ComponentProps<'a'>) {
return (
<a
{...props}
onClick={ev => {
ev.preventDefault(); // don't bubble up
openLinkWithKey(props.href); // eslint-disable-line react/prop-types -- it's not interpreting the prop types correctly
}}
/>
);
}

This file was deleted.

14 changes: 0 additions & 14 deletions ui/src/app/workflows/components/workflows-row/workflows-row.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
49 changes: 35 additions & 14 deletions ui/src/app/workflows/components/workflows-row/workflows-row.tsx
Original file line number Diff line number Diff line change
@@ -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';

Check failure on line 3 in ui/src/app/workflows/components/workflows-row/workflows-row.tsx

View workflow job for this annotation

GitHub Actions / UI

'useMemo' is defined but never used
import {Link} from 'react-router-dom';
import type {Plugin} from 'unified';

Check failure on line 5 in ui/src/app/workflows/components/workflows-row/workflows-row.tsx

View workflow job for this annotation

GitHub Actions / UI

'Plugin' is defined but never used

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;
Expand All @@ -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 (
<div className='workflows-list__row-container'>
Expand All @@ -42,16 +51,15 @@ export function WorkflowsRow(props: WorkflowsRowProps) {
/>
<PhaseIcon value={wf.status.phase} />
</div>
<div className='columns small-2'>
<a className='overlay' href={workflowLink}></a>
<WorkflowsRowName metadata={wf.metadata} />
</div>
<Link
to={{
pathname: uiUrl(`workflows/${wf.metadata.namespace}/${wf.metadata.name}`),
search: `?uid=${wf.metadata.uid}`
}}
className='small-8 row'>
<div className='small-11 row'>
<Link
to={{
pathname: uiUrl(`workflows/${wf.metadata.namespace}/${wf.metadata.name}`),
search: `?uid=${wf.metadata.uid}`
}}
className='columns small-2'>
<div className='wf-rows-name'>{hasAnnotation ? <SuspenseReactMarkdownGfm markdown={markdown} /> : markdown}</div>
</Link>
<div className='columns small-1'>{wf.metadata.namespace}</div>
<div className='columns small-1'>
<Timestamp date={wf.status.startedAt} />
Expand Down Expand Up @@ -96,8 +104,21 @@ export function WorkflowsRow(props: WorkflowsRowProps) {
);
})}
{hideDrawer ? <span /> : <WorkflowDrawer name={wf.metadata.name} namespace={wf.metadata.namespace} onChange={props.onChange} />}
</Link>
</div>
</div>
</div>
);
}

// 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 (
<React.Suspense fallback={<Loading />}>
<LazyReactMarkdownGfm markdown={props.markdown} />
</React.Suspense>
);
}

0 comments on commit ab6bf33

Please sign in to comment.