diff --git a/extension/src/cli/reader.ts b/extension/src/cli/reader.ts index b38618d45f..7c2f6ec166 100644 --- a/extension/src/cli/reader.ts +++ b/extension/src/cli/reader.ts @@ -49,14 +49,8 @@ export type StatusesOrAlwaysChanged = StageOrFileStatuses | 'always changed' export type StatusOutput = Record -export type Value = - | string - | number - | boolean - | null - | number[] - | string[] - | boolean[] +type SingleValue = string | number | boolean | null +export type Value = SingleValue | SingleValue[] export interface ValueTreeOrError { data?: ValueTree diff --git a/extension/src/experiments/columns/model.test.ts b/extension/src/experiments/columns/model.test.ts index f85ccdc959..22c6b888be 100644 --- a/extension/src/experiments/columns/model.test.ts +++ b/extension/src/experiments/columns/model.test.ts @@ -10,21 +10,32 @@ import { deeplyNestedOutput, columns as deeplyNestedColumns } from '../../test/fixtures/expShow/deeplyNested' +import { + dataTypesOutput, + columns as dataTypesColumns +} from '../../test/fixtures/expShow/dataTypes' describe('ColumnsModel', () => { const exampleDvcRoot = 'test' - it('should return columns that equal the columns fixture when given the output fixture', async () => { + it('should return the expected columns when given the default output fixture', async () => { const model = new ColumnsModel('', buildMockMemento()) await model.transformAndSet(outputFixture) expect(model.getSelected()).toStrictEqual(columnsFixture) }) - it('should return data that equal the deeply nested output fixture', async () => { + it('should return the expected columns when given the deeply nested output fixture', async () => { const model = new ColumnsModel('', buildMockMemento()) await model.transformAndSet(deeplyNestedOutput) expect(model.getSelected()).toStrictEqual(deeplyNestedColumns) }) + + it('should return the expected columns when given the data types output fixture', async () => { + const model = new ColumnsModel('', buildMockMemento()) + await model.transformAndSet(dataTypesOutput) + expect(model.getSelected()).toStrictEqual(dataTypesColumns) + }) + describe('persistence', () => { const paramsDotYamlPath = buildMetricOrParamPath( ColumnType.PARAMS, diff --git a/extension/src/experiments/model/index.test.ts b/extension/src/experiments/model/index.test.ts index 04edc91417..c593850c36 100644 --- a/extension/src/experiments/model/index.test.ts +++ b/extension/src/experiments/model/index.test.ts @@ -12,6 +12,10 @@ import { buildMockMemento } from '../../test/util' import { buildMetricOrParamPath } from '../columns/paths' import { Experiment, ColumnType } from '../webview/contract' import { definedAndNonEmpty } from '../../util/array' +import { + dataTypesOutput, + rows as dataTypesRows +} from '../../test/fixtures/expShow/dataTypes' jest.mock('vscode') @@ -64,18 +68,24 @@ describe('ExperimentsModel', () => { return { data } } - it('should return rows that equal the rows fixture when given the output fixture', () => { + it('should return the expected rows when given the output fixture', () => { const model = new ExperimentsModel('', buildMockMemento()) model.transformAndSet(outputFixture) expect(model.getRowData()).toStrictEqual(rowsFixture) }) - it('should return data that equal the deeply nested output fixture', () => { + it('should return the expected rows when given the deeply nested output fixture', () => { const model = new ExperimentsModel('', buildMockMemento()) model.transformAndSet(deeplyNestedOutput) expect(model.getRowData()).toStrictEqual(deeplyNestedRows) }) + it('should return the expected rows when given the data types output fixture', () => { + const model = new ExperimentsModel('', buildMockMemento()) + model.transformAndSet(dataTypesOutput) + expect(model.getRowData()).toStrictEqual(dataTypesRows) + }) + it('should continue to apply filters to new data if selection mode is set to use filters', () => { const testPath = buildMetricOrParamPath( ColumnType.PARAMS, diff --git a/extension/src/test/fixtures/expShow/dataTypes.ts b/extension/src/test/fixtures/expShow/dataTypes.ts new file mode 100644 index 0000000000..e58072761b --- /dev/null +++ b/extension/src/test/fixtures/expShow/dataTypes.ts @@ -0,0 +1,193 @@ +import { ExperimentsOutput } from '../../../cli/reader' +import { + Column, + ColumnType, + Row, + TableData +} from '../../../experiments/webview/contract' + +export const dataTypesOutput: ExperimentsOutput = { + workspace: { + baseline: { + data: { + timestamp: null, + params: { + 'params.yaml': { + data: { + bool1: true, + bool2: false, + zero: 0, + negative: -123, + float: 1.9293040037155151, + string: 'string', + emptyString: '', + array: [true, false, 'string', 2] + } + } + }, + queued: false, + running: false, + executor: null + } + } + }, + '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77': { + baseline: { + data: { + timestamp: '2020-11-21T19:58:22', + queued: false, + running: false, + executor: null, + name: 'main' + } + } + } +} + +export const columns: Column[] = [ + { + hasChildren: true, + label: 'params.yaml', + parentPath: ColumnType.PARAMS, + path: 'params:params.yaml', + type: ColumnType.PARAMS + }, + { + hasChildren: false, + label: 'bool1', + maxStringLength: 4, + parentPath: 'params:params.yaml', + path: 'params:params.yaml:bool1', + pathArray: [ColumnType.PARAMS, 'params.yaml', 'bool1'], + type: ColumnType.PARAMS, + types: ['boolean'] + }, + { + hasChildren: false, + label: 'bool2', + maxStringLength: 5, + parentPath: 'params:params.yaml', + path: 'params:params.yaml:bool2', + pathArray: [ColumnType.PARAMS, 'params.yaml', 'bool2'], + type: ColumnType.PARAMS, + types: ['boolean'] + }, + { + hasChildren: false, + label: 'zero', + maxNumber: 0, + maxStringLength: 1, + minNumber: 0, + parentPath: 'params:params.yaml', + path: 'params:params.yaml:zero', + pathArray: [ColumnType.PARAMS, 'params.yaml', 'zero'], + type: ColumnType.PARAMS, + types: ['number'] + }, + { + hasChildren: false, + label: 'negative', + maxNumber: -123, + maxStringLength: 4, + minNumber: -123, + parentPath: 'params:params.yaml', + path: 'params:params.yaml:negative', + pathArray: [ColumnType.PARAMS, 'params.yaml', 'negative'], + type: ColumnType.PARAMS, + types: ['number'] + }, + { + hasChildren: false, + label: 'float', + maxNumber: 1.9293040037155151, + maxStringLength: 18, + minNumber: 1.9293040037155151, + parentPath: 'params:params.yaml', + path: 'params:params.yaml:float', + pathArray: [ColumnType.PARAMS, 'params.yaml', 'float'], + type: ColumnType.PARAMS, + types: ['number'] + }, + { + hasChildren: false, + label: 'string', + maxStringLength: 6, + parentPath: 'params:params.yaml', + path: 'params:params.yaml:string', + pathArray: [ColumnType.PARAMS, 'params.yaml', 'string'], + type: ColumnType.PARAMS, + types: ['string'] + }, + { + hasChildren: false, + label: 'emptyString', + maxStringLength: 0, + parentPath: 'params:params.yaml', + path: 'params:params.yaml:emptyString', + pathArray: [ColumnType.PARAMS, 'params.yaml', 'emptyString'], + type: ColumnType.PARAMS, + types: ['string'] + }, + { + hasChildren: false, + label: 'array', + maxStringLength: 19, + parentPath: 'params:params.yaml', + path: 'params:params.yaml:array', + pathArray: [ColumnType.PARAMS, 'params.yaml', 'array'], + type: ColumnType.PARAMS, + types: ['array'] + } +] +export const rows: Row[] = [ + { + displayColor: '#945dd6', + executor: null, + id: 'workspace', + label: 'workspace', + params: { + 'params.yaml': { + array: [true, false, 'string', 2], + emptyString: '', + bool2: false, + float: 1.9293040037155151, + negative: -123, + string: 'string', + bool1: true, + zero: 0 + } + }, + queued: false, + running: false, + selected: true, + timestamp: null + }, + { + displayColor: '#13adc7', + executor: null, + id: 'main', + label: 'main', + name: 'main', + queued: false, + running: false, + selected: true, + sha: '53c3851f46955fa3e2b8f6e1c52999acc8c9ea77', + timestamp: '2020-11-21T19:58:22' + } +] + +export const dataTypesTableData: TableData = { + changes: [], + columnOrder: [], + columnWidths: {}, + filteredCounts: { experiments: 0, checkpoints: 0 }, + filters: [], + hasCheckpoints: false, + hasRunningExperiment: false, + sorts: [], + columns, + hasColumns: true, + rows +} + +export default dataTypesTableData diff --git a/webview/src/experiments/components/App.test.tsx b/webview/src/experiments/components/App.test.tsx index 7eede5b21e..528ea9d4ef 100644 --- a/webview/src/experiments/components/App.test.tsx +++ b/webview/src/experiments/components/App.test.tsx @@ -25,6 +25,7 @@ import { TableData } from 'dvc/src/experiments/webview/contract' import { buildMetricOrParamPath } from 'dvc/src/experiments/columns/paths' +import { dataTypesTableData } from 'dvc/src/test/fixtures/expShow/dataTypes' import { App } from './App' import { useIsFullyContained } from './overflowHoverTooltip/useIsFullyContained' import styles from './table/styles.module.scss' @@ -613,9 +614,6 @@ describe('App', () => { }) it('should show and hide a tooltip on mouseEnter and mouseLeave of a cell', () => { - jest - .spyOn(window, 'requestAnimationFrame') - .mockImplementation(cb => window.setTimeout(cb, 1)) render() fireEvent( window, @@ -641,13 +639,11 @@ describe('App', () => { fireEvent.mouseLeave(testParamCell, { bubbles: true }) - jest.advanceTimersByTime(1) + jest.advanceTimersByTime(CELL_TOOLTIP_DELAY) expect(screen.queryByRole('tooltip')).not.toBeInTheDocument() - - jest.mocked(window.requestAnimationFrame).mockRestore() }) - it('should show a tooltip with the full number on number cells', () => { + it('should persist a cell tooltip when it is moused into', () => { render() fireEvent( window, @@ -659,13 +655,67 @@ describe('App', () => { }) ) - const testMetricCell = screen.getByText('1.9293') - fireEvent.mouseEnter(testMetricCell, { bubbles: true }) + const testParamCell = screen.getByText(testParamStringValue) + fireEvent.mouseEnter(testParamCell, { bubbles: true }) jest.advanceTimersByTime(CELL_TOOLTIP_DELAY) const tooltip = screen.getByRole('tooltip') expect(tooltip).toBeInTheDocument() - expect(tooltip).toHaveTextContent(String(testMetricNumberValue)) + + fireEvent.mouseLeave(testParamCell, { bubbles: true }) + fireEvent.mouseEnter(tooltip, { bubbles: true }) + + jest.advanceTimersByTime(CELL_TOOLTIP_DELAY) + expect(tooltip).toBeInTheDocument() + }) + + it('should show the expected tooltip for all data types', () => { + const expectTooltipValue: (args: { + cellLabel: string + expectedTooltipResult: string + }) => void = ({ cellLabel, expectedTooltipResult }) => { + const testParamCell = screen.getByText(cellLabel) + expect(testParamCell).toBeInTheDocument() + + fireEvent.mouseEnter(testParamCell, { bubbles: true }) + + jest.advanceTimersByTime(CELL_TOOLTIP_DELAY) + const tooltip = screen.queryByRole('tooltip') + expect(tooltip).toHaveTextContent(expectedTooltipResult) + + fireEvent.mouseLeave(testParamCell, { bubbles: true }) + + jest.advanceTimersByTime(CELL_TOOLTIP_DELAY) + expect(screen.queryByRole('tooltip')).not.toBeInTheDocument() + } + + render() + fireEvent( + window, + new MessageEvent('message', { + data: { + data: dataTypesTableData, + type: MessageToWebviewType.SET_DATA + } + }) + ) + + expectTooltipValue({ + cellLabel: '1.9293', + expectedTooltipResult: '1.9293040037155151' + }) + expectTooltipValue({ + cellLabel: 'true', + expectedTooltipResult: 'true' + }) + expectTooltipValue({ + cellLabel: 'false', + expectedTooltipResult: 'false' + }) + expectTooltipValue({ + cellLabel: '[true, false, string, 2]', + expectedTooltipResult: '[true, false, string, 2]' + }) }) }) diff --git a/webview/src/experiments/components/table/styles.module.scss b/webview/src/experiments/components/table/styles.module.scss index a33846a00a..cb2a33ee93 100644 --- a/webview/src/experiments/components/table/styles.module.scss +++ b/webview/src/experiments/components/table/styles.module.scss @@ -663,3 +663,7 @@ $badge-size: 0.85rem; vertical-align: middle; font-size: 0.5rem; } + +.cellTooltip { + padding: 2px 6px; +} diff --git a/webview/src/experiments/util/buildDynamicColumns.tsx b/webview/src/experiments/util/buildDynamicColumns.tsx index 01882b23c5..4207665c9e 100644 --- a/webview/src/experiments/util/buildDynamicColumns.tsx +++ b/webview/src/experiments/util/buildDynamicColumns.tsx @@ -1,4 +1,4 @@ -import React from 'react' +import React, { SyntheticEvent } from 'react' import get from 'lodash/get' import { Column as TableColumn, @@ -13,7 +13,6 @@ import Tooltip, { CELL_TOOLTIP_DELAY } from '../../shared/components/tooltip/Tooltip' import styles from '../components/table/styles.module.scss' -import tooltipStyles from '../../shared/components/tooltip/styles.module.scss' import { CopyButton } from '../../shared/components/copyButton/CopyButton' import { OverflowHoverTooltip } from '../components/overflowHoverTooltip/OverflowHoverTooltip' const UndefinedCell = ( @@ -23,21 +22,18 @@ const UndefinedCell = ( ) const CellTooltip: React.FC<{ - cell: Cell -}> = ({ cell }) => { - const { value } = cell + stringValue: string +}> = ({ stringValue }) => { + const stopPropagation = (e: SyntheticEvent) => e.stopPropagation() return (
{ - e.stopPropagation() - }} - onKeyDown={e => { - e.stopPropagation() - }} + onClick={stopPropagation} + onKeyDown={stopPropagation} > - {value} + {stringValue}
) } @@ -57,8 +53,7 @@ const Cell: React.FC> = cell => { return ( } + content={} placement="bottom" arrow={true} delay={[CELL_TOOLTIP_DELAY, 0]} diff --git a/webview/src/stories/Table.stories.tsx b/webview/src/stories/Table.stories.tsx index 5318a5a88d..d28710773d 100644 --- a/webview/src/stories/Table.stories.tsx +++ b/webview/src/stories/Table.stories.tsx @@ -5,10 +5,13 @@ import columnsFixture from 'dvc/src/test/fixtures/expShow/columns' import { TableData } from 'dvc/src/experiments/webview/contract' import workspaceChangesFixture from 'dvc/src/test/fixtures/expShow/workspaceChanges' import deeplyNestedTableData from 'dvc/src/test/fixtures/expShow/deeplyNested' +import { dataTypesTableData } from 'dvc/src/test/fixtures/expShow/dataTypes' +import { within, userEvent } from '@storybook/testing-library' import Experiments from '../experiments/components/Experiments' import './test-vscode-styles.scss' import '../shared/style.scss' +import { CELL_TOOLTIP_DELAY } from '../shared/components/tooltip/Tooltip' const tableData: TableData = { changes: workspaceChangesFixture, @@ -79,6 +82,16 @@ WithNoRunningExperiments.args = { } } +export const WithAllDataTypes = Template.bind({}) +WithAllDataTypes.args = { tableData: dataTypesTableData } +WithAllDataTypes.play = async ({ canvasElement }) => { + const falseCell = await within(canvasElement).findByText('false') + userEvent.hover(falseCell, { bubbles: true }) +} +WithAllDataTypes.parameters = { + chromatic: { delay: CELL_TOOLTIP_DELAY } +} + export const WithDeeplyNestedHeaders = Template.bind({}) WithDeeplyNestedHeaders.args = { tableData: deeplyNestedTableData }