diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx index 528ea9d4ef..64895d7392 100644 --- a/webview/src/experiments/components/App.test.tsx +++ b/webview/src/experiments/components/App.test.tsx @@ -42,6 +42,7 @@ import { import { getRow } from '../../test/queries' import { dragAndDrop } from '../../test/dragDrop' import { DragEnterDirection } from '../../shared/components/dragDrop/util' +import { setExperimentsAsStarred } from '../../test/tableDataFixture' jest.mock('../../shared/api') jest.mock('../../util/styles') @@ -372,6 +373,129 @@ describe('App', () => { }) }) + describe('Sub-rows middle states indicators', () => { + const renderWithAllRowsExpanded = () => { + render() + + fireEvent( + window, + new MessageEvent('message', { + data: { + data: tableDataFixture, + type: MessageToWebviewType.SET_DATA + } + }) + ) + } + + const getMiddleStatesTestRow = () => { + return getRow('4fb124a') + } + + const getCountIndicators = (element: HTMLElement) => { + return within(element).queryAllByRole('marquee') + } + + const getCountIndicatorById = (row: HTMLElement, rowActionId: string) => { + const checkboxRowAction = within(row).getByTestId(rowActionId) + + return within(checkboxRowAction).queryByRole('marquee') + } + + const getCheckboxCountIndicator = (row: HTMLElement) => { + return getCountIndicatorById(row, 'row-action-checkbox') + } + + const clickRowCheckbox = (label: string) => { + const checkbox = within(getRow(label)).getByRole('checkbox') + + fireEvent.click(checkbox) + } + + const selectSomeSubRows = () => { + clickRowCheckbox('d1343a8') + clickRowCheckbox('1ee5f2e') + + return 2 + } + + const starSomeSubRows = () => { + const starredFixture = setExperimentsAsStarred(tableDataFixture, [ + 'd1343a8', + '1ee5f2e' + ]) + + fireEvent( + window, + new MessageEvent('message', { + data: { + data: starredFixture, + type: MessageToWebviewType.SET_DATA + } + }) + ) + + return 2 + } + + const toggleExpansion = (row: HTMLElement) => { + const expandButton = within(row).getByTitle('Contract Row') + + fireEvent.click(expandButton) + } + + it('should be hidden when the parent row is expanded', () => { + renderWithAllRowsExpanded() + const row = getMiddleStatesTestRow() + const indicators = getCountIndicators(row) + expect(indicators).toHaveLength(0) + }) + + describe('Checkbox selection counter', () => { + it('should not be visible if no sub-row was checked', () => { + renderWithAllRowsExpanded() + const row = getMiddleStatesTestRow() + const indicator = getCheckboxCountIndicator(row) + expect(indicator).not.toBeInTheDocument() + }) + + it('should display the correct number of checked sub-rows when the parent is collapsed', () => { + renderWithAllRowsExpanded() + const row = getMiddleStatesTestRow() + const numberOfSubrowsSelected = selectSomeSubRows() + expect(getCheckboxCountIndicator(row)).not.toBeInTheDocument() + toggleExpansion(row) + const collapsed = getMiddleStatesTestRow() + expect(getCheckboxCountIndicator(collapsed)).toHaveTextContent( + `${numberOfSubrowsSelected}` + ) + }) + }) + + describe('Stars counter', () => { + it('should not be visible if no sub-row was starred', () => { + renderWithAllRowsExpanded() + const row = getMiddleStatesTestRow() + const indicator = getCountIndicatorById(row, 'row-action-star') + expect(indicator).not.toBeInTheDocument() + }) + + it('should display the correct number of starred sub-rows when the parent is collapsed', () => { + renderWithAllRowsExpanded() + const row = getMiddleStatesTestRow() + const numberOfSubrowsStarred = starSomeSubRows() + expect( + getCountIndicatorById(row, 'row-action-star') + ).not.toBeInTheDocument() + toggleExpansion(row) + const collapsed = getMiddleStatesTestRow() + expect( + getCountIndicatorById(collapsed, 'row-action-star') + ).toHaveTextContent(`${numberOfSubrowsStarred}`) + }) + }) + }) + describe('Toggle experiment status', () => { it('should send a message to the extension to toggle an experiment when the row is clicked', () => { render() diff --git a/webview/src/experiments/components/Experiments.tsx b/webview/src/experiments/components/Experiments.tsx index 03d40fc509..10e6832ec2 100644 --- a/webview/src/experiments/components/Experiments.tsx +++ b/webview/src/experiments/components/Experiments.tsx @@ -94,7 +94,8 @@ const getColumns = (columns: Column[]): TableColumn[] => Header: ExperimentHeader, accessor: 'id', id: 'id', - width: 150 + minWidth: 250, + width: 250 }, { Cell: ({ value }: { value: string }) => { diff --git a/webview/src/experiments/components/table/Cell.tsx b/webview/src/experiments/components/table/Cell.tsx index 828277cdcd..787443a40e 100644 --- a/webview/src/experiments/components/table/Cell.tsx +++ b/webview/src/experiments/components/table/Cell.tsx @@ -1,11 +1,13 @@ import React from 'react' import cx from 'classnames' import { VSCodeCheckbox } from '@vscode/webview-ui-toolkit/react' +import { Indicator, IndicatorWithJustTheCounter } from './Indicators' import styles from './styles.module.scss' import { CellProp, RowProp } from './interfaces' import ClockIcon from '../../../shared/components/icons/Clock' import { clickAndEnterProps } from '../../../util/props' import { StarFull, StarEmpty } from '../../../shared/components/icons' +import { pluralize } from '../../../util/strings' const RowExpansionButton: React.FC = ({ row }) => row.canExpand ? ( @@ -31,27 +33,115 @@ const RowExpansionButton: React.FC = ({ row }) => ) -export const FirstCell: React.FC< - CellProp & { - bulletColor?: string - isRowSelected: boolean - toggleExperiment: () => void - toggleRowSelection: () => void - toggleStarred: () => void +export type RowActionsProps = { + isRowSelected: boolean + showSubRowStates: boolean + starred?: boolean + subRowStates: { + selections: number + stars: number + plotSelections: number } -> = ({ - cell, - bulletColor, + toggleRowSelection: () => void + toggleStarred: () => void +} + +export type RowActionProps = { + showSubRowStates: boolean + subRowsAffected: number + actionAppliedLabel: string + children: React.ReactElement + hidden?: boolean + testId?: string +} + +export const RowAction: React.FC = ({ + showSubRowStates, + subRowsAffected, + actionAppliedLabel, + children, + hidden, + testId +}) => { + const count = (showSubRowStates && subRowsAffected) || 0 + + return ( + + ) +} + +export const RowActions: React.FC = ({ isRowSelected, - toggleExperiment, + showSubRowStates, + starred, + subRowStates: { selections, stars }, toggleRowSelection, toggleStarred }) => { + return ( + <> + + + + +
+ {starred && } + {!starred && } +
+
+ + ) +} + +export const FirstCell: React.FC< + CellProp & + RowActionsProps & { + bulletColor?: string + toggleExperiment: () => void + } +> = ({ cell, bulletColor, toggleExperiment, ...rowActionsProps }) => { const { row, isPlaceholder } = cell const { - original: { starred, queued } + original: { queued } } = row + const { + subRowStates: { plotSelections } + } = rowActionsProps + return (
-
- -
- {starred && } - {!starred && } -
-
+ + {queued && } {isPlaceholder ? null : ( diff --git a/webview/src/experiments/components/table/Indicators.tsx b/webview/src/experiments/components/table/Indicators.tsx index 79e817b48a..a12b7fa957 100644 --- a/webview/src/experiments/components/table/Indicators.tsx +++ b/webview/src/experiments/components/table/Indicators.tsx @@ -3,6 +3,7 @@ import { SortDefinition } from 'dvc/src/experiments/model/sortBy' import cx from 'classnames' import { MessageFromWebviewType } from 'dvc/src/webview/contract' import { FilteredCounts } from 'dvc/src/experiments/model/filterBy/collect' +import { TippyProps } from '@tippyjs/react' import styles from './styles.module.scss' import { Icon } from '../../../shared/components/Icon' import SvgSortPrecedence from '../../../shared/components/icons/SortPrecedence' @@ -10,34 +11,82 @@ import SvgFilter from '../../../shared/components/icons/Filter' import { sendMessage } from '../../../shared/vscode' import Tooltip from '../../../shared/components/tooltip/Tooltip' import tooltipStyles from '../../../shared/components/tooltip/styles.module.scss' +import { pluralize } from '../../../util/strings' -const Indicator = ({ +export type IndicatorTooltipProps = Pick & { + tooltipContent: ReactNode +} + +export const IndicatorTooltip: React.FC = ({ + children, + tooltipContent +}) => { + const wrapperRef = React.useRef(null) + return ( + + {children} + + ) +} + +export type CounterBadgeProps = { + count?: number +} + +export const CounterBadge: React.FC = ({ count }) => { + return count ? ( + + {count} + + ) : null +} + +export const Indicator = ({ children, count, 'aria-label': ariaLabel, tooltipContent, onClick -}: { - children: ReactNode - count?: number - 'aria-label'?: string - onClick?: MouseEventHandler - tooltipContent: ReactNode -}) => ( - +}: IndicatorTooltipProps & + CounterBadgeProps & { + 'aria-label'?: string + onClick?: MouseEventHandler + }) => ( + - + +) + +export const IndicatorWithJustTheCounter = ({ + count, + 'aria-label': ariaLabel, + tooltipContent +}: CounterBadgeProps & + IndicatorTooltipProps & { + 'aria-label'?: string + }) => ( + + + + + ) const focusFiltersTree = () => @@ -45,9 +94,6 @@ const focusFiltersTree = () => const focusSortsTree = () => sendMessage({ type: MessageFromWebviewType.FOCUS_SORTS_TREE }) -const pluralize = (word: string, number: number | undefined) => - number === 1 ? word : `${word}s` - const formatCountMessage = (item: string, count: number | undefined) => `${count || 'No'} ${pluralize(item, count)} Applied` diff --git a/webview/src/experiments/components/table/Row.tsx b/webview/src/experiments/components/table/Row.tsx index 7991374c5a..666fdd28b8 100644 --- a/webview/src/experiments/components/table/Row.tsx +++ b/webview/src/experiments/components/table/Row.tsx @@ -322,9 +322,12 @@ export const RowContent: React.FC< cells: [firstCell, ...cells], original, flatIndex, + isExpanded, + subRows, + depth, values: { id } } = row - const { displayColor } = original + const { displayColor, starred } = original const isWorkspace = id === 'workspace' const changesIfWorkspace = isWorkspace ? changes : undefined const toggleExperiment = () => { @@ -360,6 +363,21 @@ export const RowContent: React.FC< [row, toggleRowSelected, isWorkspace, batchRowSelection] ) + const subRowStates = React.useMemo(() => { + const stars = subRows?.filter(subRow => subRow.original.starred).length ?? 0 + const plotSelections = + subRows?.filter(subRow => subRow.original.selected).length ?? 0 + + const selections = + subRows?.filter(subRow => selectedRows[subRow.values.id]).length ?? 0 + + return { + plotSelections, + selections, + stars + } + }, [subRows, selectedRows]) + return ( 0} + subRowStates={subRowStates} toggleExperiment={toggleExperiment} toggleRowSelection={toggleRowSelection} toggleStarred={toggleStarred} diff --git a/webview/src/experiments/components/table/styles.module.scss b/webview/src/experiments/components/table/styles.module.scss index a77d3792da..4420fef570 100644 --- a/webview/src/experiments/components/table/styles.module.scss +++ b/webview/src/experiments/components/table/styles.module.scss @@ -54,6 +54,10 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; } } +.hidden { + visibility: hidden; +} + .iconMenu { position: absolute; left: 0; @@ -450,12 +454,13 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; align-items: center; position: relative; left: -$cell-padding; + height: 100%; .starSwitch { display: inline-flex; align-items: center; + justify-content: center; width: 14px; - margin-left: 8px; opacity: 0.4; cursor: pointer; @@ -463,6 +468,23 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; opacity: 1; } } + + .indicatorIcon { + display: inline-flex; + align-items: center; + justify-content: center; + width: 1.3rem; + height: 100%; + padding: 0; + margin: 0; + } + + .indicatorCount { + z-index: 2; + &[title='0'] { + display: none; + } + } } } } @@ -672,3 +694,26 @@ $badge-size: 0.85rem; .cellTooltip { padding: 2px 6px; } + +.indicatorCount { + &[aria-label='0'] { + display: none; + } +} + +.experimentCell { + .innerCell { + .indicatorCount { + display: none; + .experimentGroup & { + display: block; + &[aria-label='0'] { + display: none; + } + } + .experimentGroup.expandedGroup & { + display: none; + } + } + } +} diff --git a/webview/src/shared/components/contextMenu/ContextMenu.tsx b/webview/src/shared/components/contextMenu/ContextMenu.tsx index 45bdee652c..4901af7461 100644 --- a/webview/src/shared/components/contextMenu/ContextMenu.tsx +++ b/webview/src/shared/components/contextMenu/ContextMenu.tsx @@ -61,6 +61,7 @@ export const ContextMenu: React.FC = ({ content={content} onShow={onShow} disabled={!content || disabled} + appendTo={'parent'} > {children} diff --git a/webview/src/shared/components/tooltip/Tooltip.tsx b/webview/src/shared/components/tooltip/Tooltip.tsx index 3ea32473d8..2af60cbe1a 100644 --- a/webview/src/shared/components/tooltip/Tooltip.tsx +++ b/webview/src/shared/components/tooltip/Tooltip.tsx @@ -26,6 +26,7 @@ const TooltipRenderFunction: React.ForwardRefRenderFunction< onClickOutside, hideOnClick, onTrigger, + appendTo, animation = false, className = typeof content === 'string' ? styles.padded : undefined, arrow = false @@ -34,7 +35,7 @@ const TooltipRenderFunction: React.ForwardRefRenderFunction< ) => ( ({ ...experiment, + starred: experiment.starred || experiment.label === '42b8736', subRows: experiment.subRows?.map(checkpoint => ({ ...checkpoint, - running: checkpoint.running || checkpoint.label === '23250b3' + running: checkpoint.running || checkpoint.label === '23250b3', + starred: checkpoint.starred || checkpoint.label === '22e40e1' })) })) })), @@ -68,6 +74,31 @@ const Template: ComponentStory = ({ tableData }) => { export const WithData = Template.bind({}) +export const WithMiddleStates = Template.bind({}) +const tableDataWithSomeSelectedExperiments = setExperimentsAsSelected( + tableData, + ['d1343a8', '91116c1', 'e821416'] +) +WithMiddleStates.args = { + tableData: setExperimentsAsStarred(tableDataWithSomeSelectedExperiments, [ + 'd1343a8', + '9523bde', + 'e821416' + ]) +} +WithMiddleStates.play = async ({ canvasElement }) => { + await within(canvasElement).findByText('1ba7bcd') + let checkboxes = await within(canvasElement).findAllByRole('checkbox') + userEvent.click(checkboxes[10], { bubbles: true }) + checkboxes = await within(canvasElement).findAllByRole('checkbox') + userEvent.click(checkboxes[11], { bubbles: true }) + const collapseButtons = () => + within(canvasElement).getAllByTitle('Contract Row') + userEvent.click(collapseButtons()[1]) + userEvent.click(collapseButtons()[2]) + userEvent.click(collapseButtons()[3]) +} + export const WithNoRunningExperiments = Template.bind({}) WithNoRunningExperiments.args = { tableData: { diff --git a/webview/src/test/tableDataFixture.ts b/webview/src/test/tableDataFixture.ts new file mode 100644 index 0000000000..2b6cc33a7a --- /dev/null +++ b/webview/src/test/tableDataFixture.ts @@ -0,0 +1,69 @@ +import { copyOriginalColors } from 'dvc/src/experiments/model/status/colors' +import { Row, TableData } from 'dvc/src/experiments/webview/contract' + +const matchAndTransform = ( + rows: Row[], + labelOrIds: string[], + transform: (source: Row) => Row +): Row[] => { + return rows.map(parent => { + let newParent = parent + + if (labelOrIds.includes(parent.id) || labelOrIds.includes(parent.label)) { + newParent = transform(parent) + } + + return { + ...newParent, + subRows: matchAndTransform(newParent.subRows || [], labelOrIds, transform) + } + }) +} + +export const transformRows = ( + fixture: TableData, + labelOrIds: string[], + transform: (source: Row) => Row +) => { + const [workspace, main] = fixture.rows + + const starredRows = [ + workspace, + { + ...main, + subRows: matchAndTransform(main.subRows || [], labelOrIds, transform) + } + ] + + return { + ...fixture, + rows: starredRows + } as TableData +} + +export const setRowPropertyAsTrue = + (prop: keyof Row) => (fixture: TableData, labelOrIds: string[]) => { + return transformRows(fixture, labelOrIds, row => ({ ...row, [prop]: true })) + } + +export const setExperimentsAsStarred = setRowPropertyAsTrue('starred') + +export const setExperimentsAsSelected = ( + fixture: TableData, + labelOrIds: string[] +) => { + let colors = copyOriginalColors().reverse() + const nextColor = () => { + if (colors.length === 0) { + colors = copyOriginalColors().reverse() + } + return colors.pop() + } + + return transformRows(fixture, labelOrIds, row => ({ + ...row, + displayColor: nextColor(), + selected: true + })) +} +export const setExperimentsAsRunning = setRowPropertyAsTrue('running') diff --git a/webview/src/util/strings.ts b/webview/src/util/strings.ts index 72e3e57c4a..93525b5757 100644 --- a/webview/src/util/strings.ts +++ b/webview/src/util/strings.ts @@ -1,2 +1,5 @@ export const capitalize = (s: string) => s[0].toUpperCase() + s.slice(1).toLowerCase() + +export const pluralize = (word: string, number: number | undefined) => + number === 1 ? word : `${word}s`