From 45d893c98fef29079663f8c16219e62c387e7634 Mon Sep 17 00:00:00 2001 From: Matt Seddon <mattseddon@hotmail.com> Date: Thu, 15 Jun 2023 11:43:01 +1000 Subject: [PATCH] Switch experiment table radio buttons to plot icons --- .../src/experiments/components/App.test.tsx | 8 +- .../components/table/body/CellRowActions.tsx | 90 +++++++++++++++---- .../experiments/components/table/body/Row.tsx | 26 ++---- .../components/table/styles.module.scss | 69 ++++---------- .../plots/components/ribbon/RibbonBlock.tsx | 5 +- 5 files changed, 105 insertions(+), 93 deletions(-) diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx index 1e561c3fdb..24eb5a5b68 100644 --- a/webview/src/experiments/components/App.test.tsx +++ b/webview/src/experiments/components/App.test.tsx @@ -668,10 +668,10 @@ describe('App', () => { expect(screen.queryByRole('tooltip')).not.toBeInTheDocument() - const radioButton = within(getRow(EXPERIMENT_WORKSPACE_ID)).getByTestId( + const plotIcon = within(getRow(EXPERIMENT_WORKSPACE_ID)).getByTestId( 'row-action-plot' ) - fireEvent.mouseEnter(radioButton) + fireEvent.mouseEnter(plotIcon) advanceTimersByTime(NORMAL_TOOLTIP_DELAY[0]) const tooltip = screen.queryByRole('tooltip') @@ -692,8 +692,8 @@ describe('App', () => { expect(screen.queryByRole('tooltip')).not.toBeInTheDocument() - const radioButton = within(getRow('main')).getByTestId('row-action-star') - fireEvent.mouseEnter(radioButton) + const starIcon = within(getRow('main')).getByTestId('row-action-star') + fireEvent.mouseEnter(starIcon) advanceTimersByTime(NORMAL_TOOLTIP_DELAY[0]) const tooltip = screen.queryByRole('tooltip') diff --git a/webview/src/experiments/components/table/body/CellRowActions.tsx b/webview/src/experiments/components/table/body/CellRowActions.tsx index 6fa1385a81..8c42ec199f 100644 --- a/webview/src/experiments/components/table/body/CellRowActions.tsx +++ b/webview/src/experiments/components/table/body/CellRowActions.tsx @@ -1,26 +1,37 @@ import React, { MouseEventHandler, ReactElement } from 'react' -import { VSCodeCheckbox } from '@vscode/webview-ui-toolkit/react' +import { + VSCodeCheckbox, + VSCodeProgressRing +} from '@vscode/webview-ui-toolkit/react' +import cx from 'classnames' import { ExperimentStatus, - isQueued + isQueued, + isRunning } from 'dvc/src/experiments/webview/contract' import { CellHintTooltip } from './CellHintTooltip' import { Indicator } from '../Indicators' import { addStarredFilter, openPlotsWebview } from '../../../util/messages' import styles from '../styles.module.scss' import { clickAndEnterProps } from '../../../../util/props' -import { Clock, StarFull, StarEmpty } from '../../../../shared/components/icons' +import { + Clock, + StarFull, + StarEmpty, + GraphScatter +} from '../../../../shared/components/icons' export type CellRowActionsProps = { - bulletColor?: string isRowSelected: boolean + plotColor?: string showSubRowStates: boolean starred?: boolean status?: ExperimentStatus subRowStates: { + plotSelections: number + running: number selections: number stars: number - plotSelections: number } toggleExperiment: () => void toggleRowSelection: () => void @@ -32,7 +43,7 @@ type CellRowActionProps = { subRowsAffected: number children: ReactElement testId: string - tooltipContent: string | ReactElement + tooltipContent: string | ReactElement | null onClick?: MouseEventHandler } @@ -46,20 +57,40 @@ const CellRowAction: React.FC<CellRowActionProps> = ({ }) => { const count = (showSubRowStates && subRowsAffected) || 0 - return ( - <CellHintTooltip tooltipContent={tooltipContent}> - <div className={styles.rowActions} data-testid={testId}> - <Indicator onClick={onClick} count={count}> - {children} - </Indicator> - </div> - </CellHintTooltip> + const action = ( + <div className={styles.rowActions} data-testid={testId}> + <Indicator onClick={onClick} count={count}> + {children} + </Indicator> + </div> + ) + + return tooltipContent ? ( + <CellHintTooltip tooltipContent={tooltipContent}>{action}</CellHintTooltip> + ) : ( + action ) } const getTooltipContent = (determiner: boolean, action: string): string => 'Click to ' + (determiner ? `un${action}` : action) +const getRunningTooltipContent = ( + running: boolean, + showSubRowStates: boolean, + subRowsRunning: number +): string | null => { + if (running) { + return 'Experiment is currently running' + } + + if (showSubRowStates && subRowsRunning) { + return `There are currently ${subRowsRunning} running under this commit.` + } + + return null +} + type ClickableTooltipContentProps = { clickableText: string helperText: string @@ -81,16 +112,18 @@ const ClickableTooltipContent: React.FC<ClickableTooltipContentProps> = ({ ) export const CellRowActions: React.FC<CellRowActionsProps> = ({ - bulletColor, + plotColor, status, toggleExperiment, isRowSelected, showSubRowStates, starred, - subRowStates: { selections, stars, plotSelections }, + subRowStates: { plotSelections, running: subRowsRunning, selections, stars }, toggleRowSelection, toggleStarred }) => { + const running = isRunning(status) + return ( <> <CellRowAction @@ -104,6 +137,24 @@ export const CellRowActions: React.FC<CellRowActionsProps> = ({ checked={isRowSelected} /> </CellRowAction> + <CellRowAction + showSubRowStates={showSubRowStates} + subRowsAffected={subRowsRunning} + testId="row-action-running-indicator" + tooltipContent={getRunningTooltipContent( + running, + showSubRowStates, + subRowsRunning + )} + > + {running ? ( + <VSCodeProgressRing + className={cx(styles.running, 'chromatic-ignore')} + /> + ) : ( + <div /> + )} + </CellRowAction> <CellRowAction showSubRowStates={showSubRowStates} subRowsAffected={stars} @@ -141,13 +192,16 @@ export const CellRowActions: React.FC<CellRowActionsProps> = ({ tooltipContent={ <ClickableTooltipContent clickableText="Open the plots view" - helperText={getTooltipContent(!!bulletColor, 'plot')} + helperText={getTooltipContent(!!plotColor, 'plot')} onClick={openPlotsWebview} /> } onClick={toggleExperiment} > - <span className={styles.bullet} style={{ color: bulletColor }} /> + <GraphScatter + style={{ fill: plotColor }} + className={styles.plotIcon} + /> </CellRowAction> )} </> diff --git a/webview/src/experiments/components/table/body/Row.tsx b/webview/src/experiments/components/table/body/Row.tsx index cc267d79e8..b644f8bfe4 100644 --- a/webview/src/experiments/components/table/body/Row.tsx +++ b/webview/src/experiments/components/table/body/Row.tsx @@ -2,7 +2,7 @@ import cx from 'classnames' import React, { useCallback, useContext, useMemo } from 'react' import { useSelector } from 'react-redux' import { EXPERIMENT_WORKSPACE_ID } from 'dvc/src/cli/dvc/contract' -import { isQueued, isRunning } from 'dvc/src/experiments/webview/contract' +import { isRunning } from 'dvc/src/experiments/webview/contract' import { FirstCell, CellWrapper } from './Cell' import { RowContextMenu } from './RowContextMenu' import styles from '../styles.module.scss' @@ -24,9 +24,8 @@ export const RowContent: React.FC< const changes = useSelector( (state: ExperimentsState) => state.tableData.changes ) - const { getVisibleCells, original, index, getIsExpanded, subRows } = row - const { branch, displayColor, error, starred, id, status, selected } = - original + const { getVisibleCells, original, getIsExpanded, subRows } = row + const { branch, displayColor, error, starred, id } = original const [firstCell, ...cells] = getVisibleCells() const isWorkspace = id === EXPERIMENT_WORKSPACE_ID const changesIfWorkspace = isWorkspace ? changes : undefined @@ -53,6 +52,9 @@ export const RowContent: React.FC< const plotSelections = subRows?.filter(subRow => subRow.original.selected).length ?? 0 + const running = + subRows?.filter(subRow => isRunning(subRow.original.status)).length ?? 0 + const selections = subRows?.filter( subRow => @@ -63,16 +65,12 @@ export const RowContent: React.FC< return { plotSelections, + running, selections, stars } }, [subRows, selectedRows]) - const running = isRunning(status) - const queued = isQueued(status) - const unselected = selected === false - const isOdd = index % 2 !== 0 && !isRowSelected - return ( <ContextMenu content={<RowContextMenu row={row} />}> <tr @@ -82,14 +80,6 @@ export const RowContent: React.FC< styles.bodyRow, styles.row, { - [styles.runningExperiment]: running, - [styles.queuedExperiment]: queued, - [styles.unselectedExperiment]: !running && !queued && unselected, - [styles.normalExperiment]: !running && !queued && !unselected, - [styles.oddRow]: isOdd, - [styles.evenRow]: !isOdd, - [styles.workspaceRow]: isWorkspace, - [styles.normalRow]: !isWorkspace, [styles.rowSelected]: isRowSelected } )} @@ -100,7 +90,7 @@ export const RowContent: React.FC< <FirstCell cell={firstCell} changesIfWorkspace={!!changesIfWorkspace?.length} - bulletColor={displayColor} + plotColor={displayColor} starred={starred} isRowSelected={isRowSelected} showSubRowStates={!getIsExpanded() && !isWorkspace} diff --git a/webview/src/experiments/components/table/styles.module.scss b/webview/src/experiments/components/table/styles.module.scss index c1db910c28..fe1826cc98 100644 --- a/webview/src/experiments/components/table/styles.module.scss +++ b/webview/src/experiments/components/table/styles.module.scss @@ -10,7 +10,7 @@ $row-border: 1px solid $border-color; $edge-padding: 0.8rem; $cell-padding: 0.5rem; $workspace-row-edge-margin: $edge-padding - $cell-padding; -$bullet-size: calc(var(--design-unit) * 4px); +$bullet-size: calc(var(--design-unit) * 8px); $badge-size: 0.85rem; // Extendable Silent Rules @@ -451,6 +451,14 @@ $badge-size: 0.85rem; visibility: hidden; } + .running { + visibility: visible; + } + + .plotIcon { + visibility: visible; + } + .timestampInnerCell { height: 42px; } @@ -576,7 +584,7 @@ $badge-size: 0.85rem; height: 100%; &:first-child { - margin-right: 20px; + margin-right: 4px; } .nestedRow & { @@ -626,57 +634,14 @@ $badge-size: 0.85rem; font-size: 0.65rem; } -.bullet { - visibility: visible; - align-items: center; - background: $checkbox-background; - border-radius: $bullet-size; - border: calc(var(--border-width) * 1px) solid $checkbox-border; - color: $icon-color; - cursor: pointer; - display: flex; - height: $bullet-size; - justify-content: center; - outline: none; - padding: 0; - position: relative; - width: $bullet-size; - - &::before { - display: inline-block; - position: relative; - content: ''; - border-radius: 99px; - width: 6px; - height: 6px; - z-index: 2; - - .normalExperiment & { - line-height: 0; - background: currentcolor; - border-radius: 100%; - } - - .unselectedExperiment & { - width: 4px; - height: 4px; - vertical-align: middle; - border: 1px solid $icon-color; - background-color: $checkbox-background; - } +.plotIcon { + margin-top: 4px; +} - .runningExperiment & { - width: 4px; - height: 4px; - vertical-align: middle; - border: 1.5px solid $checkbox-background; - border-radius: 100%; - border-right: 1.5px solid currentcolor; - border-top: 1.5px solid currentcolor; - animation: spin 1s cubic-bezier(0.53, 0.21, 0.29, 0.67) infinite; - background-color: $checkbox-background; - } - } +.running { + position: relative; + height: 16px; + width: 16px; } .queued { diff --git a/webview/src/plots/components/ribbon/RibbonBlock.tsx b/webview/src/plots/components/ribbon/RibbonBlock.tsx index d84e2ab134..ba3abbc3d9 100644 --- a/webview/src/plots/components/ribbon/RibbonBlock.tsx +++ b/webview/src/plots/components/ribbon/RibbonBlock.tsx @@ -1,5 +1,6 @@ import { Revision } from 'dvc/src/plots/webview/contract' import React from 'react' +import cx from 'classnames' import { VSCodeProgressRing } from '@vscode/webview-ui-toolkit/react' import styles from './styles.module.scss' import { RibbonBlockTooltip } from './RibbonBlockTooltip' @@ -19,7 +20,9 @@ const RevisionIcon: React.FC<{ fetched: boolean; errors?: string[] }> = ({ }) => ( <div className={styles.iconPlaceholder}> {fetched && errors && '!'} - {!fetched && <VSCodeProgressRing className={styles.fetching} />} + {!fetched && ( + <VSCodeProgressRing className={cx(styles.fetching, 'chromatic-ignore')} /> + )} </div> )