From 6d3e1d12d8084cd4ac498413595b8aaaee2e6afe Mon Sep 17 00:00:00 2001 From: Wolmir Nemitz Date: Wed, 1 Jun 2022 14:20:33 -0300 Subject: [PATCH 1/9] Add arrow sort indicators and filter indicators to columns --- extension/src/experiments/index.ts | 1 + extension/src/experiments/webview/contract.ts | 2 + .../src/test/fixtures/expShow/deeplyNested.ts | 1 + .../src/test/fixtures/expShow/tableData.ts | 1 + .../experiments/components/Experiments.tsx | 5 +- .../components/table/MergeHeaderGroups.tsx | 4 + .../components/table/SortPicker.tsx | 50 ------- .../components/table/Table.test.tsx | 91 +++++------- .../experiments/components/table/Table.tsx | 17 ++- .../components/table/TableHead.tsx | 4 + .../components/table/TableHeader.tsx | 133 ++++++++++++------ .../components/table/styles.module.scss | 36 +++-- .../components/contextMenu/ContextMenu.tsx | 1 + .../shared/components/iconMenu/IconMenu.tsx | 18 +-- .../components/iconMenu/IconMenuItem.tsx | 1 + 15 files changed, 199 insertions(+), 166 deletions(-) delete mode 100644 webview/src/experiments/components/table/SortPicker.tsx diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts index 34ddf474a8..7c29be638c 100644 --- a/extension/src/experiments/index.ts +++ b/extension/src/experiments/index.ts @@ -440,6 +440,7 @@ export class Experiments extends BaseRepository { columnOrder: this.columns.getColumnOrder(), columnWidths: this.columns.getColumnWidths(), columns: this.columns.getSelected(), + filters: this.experiments.getFilters(), hasCheckpoints: this.hasCheckpoints(), hasColumns: this.columns.hasColumns(), hasRunningExperiment: this.experiments.hasRunningExperiment(), diff --git a/extension/src/experiments/webview/contract.ts b/extension/src/experiments/webview/contract.ts index 47804cc362..2dd9cbac61 100644 --- a/extension/src/experiments/webview/contract.ts +++ b/extension/src/experiments/webview/contract.ts @@ -1,4 +1,5 @@ import { BaseExperimentFields, ValueTree } from '../../cli/reader' +import { FilterDefinition } from '../model/filterBy' import { SortDefinition } from '../model/sortBy' export interface MetricOrParamColumns { @@ -60,6 +61,7 @@ export type TableData = { hasRunningExperiment: boolean rows: Row[] sorts: SortDefinition[] + filters: FilterDefinition[] } export type InitiallyUndefinedTableData = TableData | undefined diff --git a/extension/src/test/fixtures/expShow/deeplyNested.ts b/extension/src/test/fixtures/expShow/deeplyNested.ts index 4fe2131f31..59c9200399 100644 --- a/extension/src/test/fixtures/expShow/deeplyNested.ts +++ b/extension/src/test/fixtures/expShow/deeplyNested.ts @@ -281,6 +281,7 @@ const deeplyNestedTableData: TableData = { changes: [], columnOrder: [], columnWidths: {}, + filters: [], hasCheckpoints: false, hasRunningExperiment: false, sorts: [ diff --git a/extension/src/test/fixtures/expShow/tableData.ts b/extension/src/test/fixtures/expShow/tableData.ts index b3f2f6be02..7007fddec4 100644 --- a/extension/src/test/fixtures/expShow/tableData.ts +++ b/extension/src/test/fixtures/expShow/tableData.ts @@ -5,6 +5,7 @@ import columnsFixture from './columns' const tableDataFixture: TableData = { rows: rowsFixture, columns: columnsFixture, + filters: [], hasCheckpoints: true, hasRunningExperiment: true, hasColumns: true, diff --git a/webview/src/experiments/components/Experiments.tsx b/webview/src/experiments/components/Experiments.tsx index 285f09dae9..db0c2d81e5 100644 --- a/webview/src/experiments/components/Experiments.tsx +++ b/webview/src/experiments/components/Experiments.tsx @@ -28,8 +28,8 @@ import { GetStarted } from '../../shared/components/getStarted/GetStarted' import { DragDropProvider } from '../../shared/components/dragDrop/DragDropContext' import { EmptyState } from '../../shared/components/emptyState/EmptyState' -const DEFAULT_COLUMN_WIDTH = 75 -const MINIMUM_COLUMN_WIDTH = 50 +const DEFAULT_COLUMN_WIDTH = 90 +const MINIMUM_COLUMN_WIDTH = 90 const timeFormatter = new Intl.DateTimeFormat([], { hour: '2-digit', @@ -138,6 +138,7 @@ export const ExperimentsTable: React.FC<{ columnOrder: [], columnWidths: {}, columns: [], + filters: [], hasCheckpoints: false, hasColumns: false, hasRunningExperiment: false, diff --git a/webview/src/experiments/components/table/MergeHeaderGroups.tsx b/webview/src/experiments/components/table/MergeHeaderGroups.tsx index fcbffd3869..c6f2a5bd2a 100644 --- a/webview/src/experiments/components/table/MergeHeaderGroups.tsx +++ b/webview/src/experiments/components/table/MergeHeaderGroups.tsx @@ -3,6 +3,7 @@ import cx from 'classnames' import { SortDefinition } from 'dvc/src/experiments/model/sortBy' import { Experiment, Column } from 'dvc/src/experiments/webview/contract' import { HeaderGroup } from 'react-table' +import { FilterDefinition } from 'dvc/src/experiments/model/filterBy' import { TableHeader } from './TableHeader' import styles from './styles.module.scss' import { @@ -15,6 +16,7 @@ export const MergedHeaderGroups: React.FC<{ headerGroup: HeaderGroup columns: HeaderGroup[] sorts: SortDefinition[] + filters: FilterDefinition[] orderedColumns: Column[] onDragUpdate: OnDragOver onDragStart: OnDragStart @@ -22,6 +24,7 @@ export const MergedHeaderGroups: React.FC<{ }> = ({ headerGroup, sorts, + filters, columns, orderedColumns, onDragUpdate, @@ -41,6 +44,7 @@ export const MergedHeaderGroups: React.FC<{ column={column} columns={columns} sorts={sorts} + filters={filters} onDragOver={onDragUpdate} onDragStart={onDragStart} onDrop={onDragEnd} diff --git a/webview/src/experiments/components/table/SortPicker.tsx b/webview/src/experiments/components/table/SortPicker.tsx deleted file mode 100644 index 8ab9057c17..0000000000 --- a/webview/src/experiments/components/table/SortPicker.tsx +++ /dev/null @@ -1,50 +0,0 @@ -import React from 'react' -import { SelectMenu } from '../../../shared/components/selectMenu/SelectMenu' -import { SelectMenuOptionProps } from '../../../shared/components/selectMenu/SelectMenuOption' - -export enum SortOrder { - ASCENDING = 'ascending', - DESCENDING = 'descending', - NONE = 'none' -} - -export enum SortOrderLabel { - ASCENDING = 'Sort Ascending', - DESCENDING = 'Sort Descending', - NONE = 'Remove Sort' -} - -export const SortPicker: React.FC<{ - sortOrder: SortOrder - setSelectedOrder: (order: SortOrder) => void -}> = ({ sortOrder, setSelectedOrder }) => { - const options: SelectMenuOptionProps[] = [ - { - id: SortOrder.ASCENDING, - isSelected: sortOrder === SortOrder.ASCENDING, - label: SortOrderLabel.ASCENDING - }, - { - id: SortOrder.DESCENDING, - isSelected: sortOrder === SortOrder.DESCENDING, - label: SortOrderLabel.DESCENDING - }, - { - id: SortOrder.NONE, - isSelected: !sortOrder || sortOrder === SortOrder.NONE, - label: SortOrderLabel.NONE - } - ] - return ( - { - const order = (id as SortOrder) || SortOrder.NONE - - if (order !== sortOrder) { - setSelectedOrder(order) - } - }} - /> - ) -} diff --git a/webview/src/experiments/components/table/Table.test.tsx b/webview/src/experiments/components/table/Table.test.tsx index 14f63b0a37..22cd03ef03 100644 --- a/webview/src/experiments/components/table/Table.test.tsx +++ b/webview/src/experiments/components/table/Table.test.tsx @@ -3,19 +3,12 @@ */ /* eslint jest/expect-expect: ["error", { "assertFunctionNames": ["expect", "expectHeaders"] }] */ import '@testing-library/jest-dom/extend-expect' -import { - cleanup, - fireEvent, - render, - screen, - within -} from '@testing-library/react' +import { cleanup, fireEvent, render, screen } from '@testing-library/react' import { Experiment, TableData } from 'dvc/src/experiments/webview/contract' import { MessageFromWebviewType } from 'dvc/src/webview/contract' import React from 'react' import { TableInstance } from 'react-table' import tableDataFixture from 'dvc/src/test/fixtures/expShow/tableData' -import { SortOrderLabel } from './SortPicker' import { Table } from './Table' import styles from './styles.module.scss' import { ExperimentsTable } from '../Experiments' @@ -141,27 +134,25 @@ describe('Table', () => { }) describe('Sorting through the UI', () => { - const mockColumnName = 'C' const mockColumnPath = 'params:C' - const findSortableColumn = async (headerText = mockColumnName) => - await screen.findByText(headerText) + const findSortableColumn = async () => + await screen.findByTestId(`header-${mockColumnPath}`) - const clickOnSortOption = async (optionLabel: SortOrderLabel) => { - const column = await findSortableColumn() - fireEvent.contextMenu(column, { - bubbles: true - }) - const columnMenu = await screen.findAllByRole('menu') + describe('Sortable column', () => { + it('should not not have a sorting indicator if it is not sorted yet', () => { + renderExperimentsTable() + const sortIcons = screen.queryAllByTestId('sorting-indicator') - const sortOption = await within(columnMenu[0]).findByText(optionLabel) - fireEvent.click(sortOption) - } + expect(sortIcons.length).toBe(0) + }) - describe('Given no sorting is present yet', () => { - it('should add an ascending sort to the column path when the user clicks on the Ascending option', async () => { + it('should add an ascending sort to the column, if it is not sorted yet', async () => { renderExperimentsTable() - await clickOnSortOption(SortOrderLabel.ASCENDING) + const column = await findSortableColumn() + + fireEvent.click(column) + expect(mockedPostMessage).toBeCalledWith({ payload: { descending: false, @@ -171,27 +162,27 @@ describe('Table', () => { }) }) - it('should add a descending sort to the column path when the user clicks on the Descending option', async () => { + it('should work with the keyboard as well', async () => { renderExperimentsTable() - await clickOnSortOption(SortOrderLabel.DESCENDING) + const column = await findSortableColumn() + + fireEvent.keyDown(column, { + bubbles: true, + code: 'Enter', + key: 'Enter', + keyCode: 13 + }) + expect(mockedPostMessage).toBeCalledWith({ payload: { - descending: true, + descending: false, path: mockColumnPath }, type: MessageFromWebviewType.SORT_COLUMN }) }) - it('should not do anything when the user clicks on the None option', async () => { - renderExperimentsTable() - await clickOnSortOption(SortOrderLabel.NONE) - expect(mockedPostMessage).not.toHaveBeenCalled() - }) - }) - - describe('Given an initial ascending column sort', () => { - it('should add a descending sort to the column path when the user clicks on the Descending option', async () => { + it('should add a descending sort to the column, if it has an ascending sort', async () => { renderExperimentsTable({ ...sortingTableDataFixture, sorts: [ @@ -201,7 +192,12 @@ describe('Table', () => { } ] }) - await clickOnSortOption(SortOrderLabel.DESCENDING) + + const column = await findSortableColumn() + expect(column).toHaveClass('sortingHeaderCellAsc') + + fireEvent.click(column) + expect(mockedPostMessage).toBeCalledWith({ payload: { descending: true, @@ -211,31 +207,22 @@ describe('Table', () => { }) }) - it('should not do anything when the user clicks on the Ascending option', async () => { + it('should remove the column sort if it has a descending sort', async () => { renderExperimentsTable({ ...sortingTableDataFixture, sorts: [ { - descending: false, + descending: true, path: mockColumnPath } ] }) - await clickOnSortOption(SortOrderLabel.ASCENDING) - expect(mockedPostMessage).not.toHaveBeenCalled() - }) - it('should remove the column sort when the user clicks on the None option', async () => { - renderExperimentsTable({ - ...sortingTableDataFixture, - sorts: [ - { - descending: false, - path: mockColumnPath - } - ] - }) - await clickOnSortOption(SortOrderLabel.NONE) + const column = await findSortableColumn() + expect(column).toHaveClass('sortingHeaderCellDesc') + + fireEvent.click(column) + expect(mockedPostMessage).toBeCalledWith({ payload: mockColumnPath, type: MessageFromWebviewType.REMOVE_COLUMN_SORT diff --git a/webview/src/experiments/components/table/Table.tsx b/webview/src/experiments/components/table/Table.tsx index eb294032bb..d5cb13d4e5 100644 --- a/webview/src/experiments/components/table/Table.tsx +++ b/webview/src/experiments/components/table/Table.tsx @@ -101,13 +101,24 @@ export const Table: React.FC = ({ tableData }) => { const { getTableProps, rows } = instance - const { sorts, columns, changes, hasCheckpoints, hasRunningExperiment } = - tableData + const { + filters, + sorts, + columns, + changes, + hasCheckpoints, + hasRunningExperiment + } = tableData return (
- + {rows.map(row => ( columns: Column[] sorts: SortDefinition[] + filters: FilterDefinition[] } export const TableHead: React.FC = ({ + filters, instance: { headerGroups, setColumnOrder, @@ -90,6 +93,7 @@ export const TableHead: React.FC = ({ headerGroup={headerGroup} columns={allHeaders} sorts={sorts} + filters={filters} onDragStart={onDragStart} onDragUpdate={onDragUpdate} onDragEnd={onDragEnd} diff --git a/webview/src/experiments/components/table/TableHeader.tsx b/webview/src/experiments/components/table/TableHeader.tsx index 0fec2f73ad..506f2d85f1 100644 --- a/webview/src/experiments/components/table/TableHeader.tsx +++ b/webview/src/experiments/components/table/TableHeader.tsx @@ -4,13 +4,13 @@ import { Column, ColumnType } from 'dvc/src/experiments/webview/contract' -import React from 'react' +import React, { useEffect } from 'react' import { HeaderGroup } from 'react-table' import cx from 'classnames' import { MessageFromWebviewType } from 'dvc/src/webview/contract' import { VSCodeDivider } from '@vscode/webview-ui-toolkit/react' +import { FilterDefinition } from 'dvc/src/experiments/model/filterBy' import styles from './styles.module.scss' -import { SortOrder, SortPicker } from './SortPicker' import { countUpperLevels, isFirstLevelHeader } from '../../util/columns' import { sendMessage } from '../../../shared/vscode' import { ContextMenu } from '../../../shared/components/contextMenu/ContextMenu' @@ -22,6 +22,15 @@ import { } from '../../../shared/components/dragDrop/DragDropWorkbench' import { MessagesMenu } from '../../../shared/components/messagesMenu/MessagesMenu' import { MessagesMenuOptionProps } from '../../../shared/components/messagesMenu/MessagesMenuOption' +import { IconMenu } from '../../../shared/components/iconMenu/IconMenu' +import { IconMenuItemProps } from '../../../shared/components/iconMenu/IconMenuItem' +import { AllIcons } from '../../../shared/components/Icon' + +export enum SortOrder { + ASCENDING = 'ascending', + DESCENDING = 'descending', + NONE = 'none' +} export const ColumnDragHandle: React.FC<{ disabled: boolean @@ -63,10 +72,12 @@ export const ColumnDragHandle: React.FC<{ const TableHeaderCell: React.FC<{ column: HeaderGroup columns: HeaderGroup[] + hasFilter: boolean orderedColumns: Column[] sortOrder: SortOrder - menuDisabled: boolean - menuContent: React.ReactNode + sortEnabled: boolean + menuDisabled?: boolean + menuContent?: React.ReactNode onDragOver: OnDragOver onDragStart: OnDragStart onDrop: OnDrop @@ -74,7 +85,9 @@ const TableHeaderCell: React.FC<{ column, columns, orderedColumns, + hasFilter, sortOrder, + sortEnabled, menuContent, menuDisabled, onDragOver, @@ -106,6 +119,7 @@ const TableHeaderCell: React.FC<{ [styles.depHeaderCell]: column.group === ColumnType.DEPS, [styles.firstLevelHeader]: isFirstLevelHeader(column.id), [styles.leafHeader]: column.headers === undefined, + [styles.menuEnabled]: sortEnabled, ...sortingClasses() } ) @@ -114,16 +128,46 @@ const TableHeaderCell: React.FC<{ const isDraggable = !column.placeholderOf && !['id', 'timestamp'].includes(column.id) + const menuItems: IconMenuItemProps[] = [ + { + hidden: !sortEnabled || sortOrder === SortOrder.NONE, + icon: + (sortOrder === SortOrder.DESCENDING && AllIcons.DOWN_ARROW) || + AllIcons.UP_ARROW, + onClick: () => {}, + tooltip: 'Sorted column' + }, + { + hidden: !hasFilter, + icon: AllIcons.LINES, + onClick: () => {}, + tooltip: 'Filtered column' + } + ] + + const [menuSupressed, setMenuSupressed] = React.useState(false) + return ( - + { + return !column.isResizing + }} + >
+
+ +
setMenuSupressed(true)} + onMouseLeave={() => setMenuSupressed(false)} className={styles.columnResizer} style={{ height: resizerHeight }} /> @@ -144,6 +190,7 @@ interface TableHeaderProps { column: HeaderGroup columns: HeaderGroup[] sorts: SortDefinition[] + filters: FilterDefinition[] orderedColumns: Column[] onDragOver: OnDragOver onDragStart: OnDragStart @@ -153,6 +200,7 @@ interface TableHeaderProps { export const TableHeader: React.FC = ({ column, columns, + filters, sorts, orderedColumns, onDragOver, @@ -161,6 +209,7 @@ export const TableHeader: React.FC = ({ }) => { const baseColumn = column.placeholderOf || column const sort = sorts.find(sort => sort.path === baseColumn.id) + const filter = filters.find(({ path }) => path === column.id) const isSortable = !column.placeholderOf && !['id', 'timestamp'].includes(column.id) && @@ -176,30 +225,6 @@ export const TableHeader: React.FC = ({ return possibleOrders[`${sort?.descending}`] })() - const removeColumnSort = () => { - sendMessage({ - payload: column.id, - type: MessageFromWebviewType.REMOVE_COLUMN_SORT - }) - } - - const setColumnSort = (selectedSort: SortOrder) => { - if (selectedSort === SortOrder.NONE) { - removeColumnSort() - return - } - - const payload: SortDefinition = { - descending: selectedSort === SortOrder.DESCENDING, - path: column.id - } - - sendMessage({ - payload, - type: MessageFromWebviewType.SORT_COLUMN - }) - } - const contextMenuOptions: MessagesMenuOptionProps[] = React.useMemo(() => { const menuOptions: MessagesMenuOptionProps[] = [ { @@ -231,24 +256,50 @@ export const TableHeader: React.FC = ({ columns={columns} orderedColumns={orderedColumns} sortOrder={sortOrder} + sortEnabled={isSortable} + hasFilter={!!filter} onDragOver={onDragOver} onDragStart={onDragStart} onDrop={onDrop} menuDisabled={!isSortable && column.group !== ColumnType.PARAMS} menuContent={
- {isSortable && ( -
- { - setColumnSort(order) - }} - /> - -
- )} + +
} /> diff --git a/webview/src/experiments/components/table/styles.module.scss b/webview/src/experiments/components/table/styles.module.scss index e96f90c5b5..d0c2a55c82 100644 --- a/webview/src/experiments/components/table/styles.module.scss +++ b/webview/src/experiments/components/table/styles.module.scss @@ -27,7 +27,6 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; %truncateLeftParent { overflow: hidden; text-overflow: ellipsis; - direction: rtl; } %truncateLeftChild { @@ -55,6 +54,30 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; } } +.iconMenu { + position: absolute; + left: 0; + bottom: 0; + padding-bottom: 3.5px; + + ul[role='menu'] { + background-color: $header-bg-color; + padding-left: 2px; + margin: 0; + border: none; + + button { + width: 13px; + height: 11px; + svg { + fill: currentColor; + transform: scale(0.7); + // stroke-width: 2px; + } + } + } +} + // Concrete selectors @keyframes spin { @@ -260,6 +283,7 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; span[draggable='true'] { display: block; + cursor: grab; } } @@ -282,7 +306,7 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; } &:last-child { - font-size: 0.9rem; + font-size: 0.7rem; .headerCell { text-align: right; @@ -408,14 +432,6 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; border-bottom: none; } } - - .sortingHeaderCellAsc { - border-top: 2px solid $fg-color; - } - - .sortingHeaderCellDesc { - border-bottom: 2px solid $fg-color; - } } .webviewHeader { diff --git a/webview/src/shared/components/contextMenu/ContextMenu.tsx b/webview/src/shared/components/contextMenu/ContextMenu.tsx index b689871477..2a47972c53 100644 --- a/webview/src/shared/components/contextMenu/ContextMenu.tsx +++ b/webview/src/shared/components/contextMenu/ContextMenu.tsx @@ -52,6 +52,7 @@ export const ContextMenu: React.FC = ({ = ({ items }) => { }} >
    - {items.map(item => ( - - ))} + {items + .filter(({ hidden }) => !hidden) + .map(item => ( + + ))}
diff --git a/webview/src/shared/components/iconMenu/IconMenuItem.tsx b/webview/src/shared/components/iconMenu/IconMenuItem.tsx index 2cd163c1d1..e2ed54a0ce 100644 --- a/webview/src/shared/components/iconMenu/IconMenuItem.tsx +++ b/webview/src/shared/components/iconMenu/IconMenuItem.tsx @@ -9,6 +9,7 @@ export interface IconMenuItemProps { onClick?: () => void onClickNode?: React.ReactNode tooltip: string + hidden?: boolean } export interface IconMenuItemAllProps extends IconMenuItemProps { From c01838356c91d9a3845a370b1589c8e71c1a08d7 Mon Sep 17 00:00:00 2001 From: Wolmir Nemitz Date: Wed, 8 Jun 2022 19:50:18 -0300 Subject: [PATCH 2/9] Fix filters prop dependencies --- extension/src/test/suite/experiments/index.test.ts | 2 ++ .../src/test/suite/experiments/model/filterBy/tree.test.ts | 2 ++ webview/src/experiments/components/table/Table.test.tsx | 1 + webview/src/experiments/components/table/TableHeader.tsx | 3 +-- webview/src/test/sort.ts | 1 + 5 files changed, 7 insertions(+), 2 deletions(-) diff --git a/extension/src/test/suite/experiments/index.test.ts b/extension/src/test/suite/experiments/index.test.ts index ec617dc378..64f315cff4 100644 --- a/extension/src/test/suite/experiments/index.test.ts +++ b/extension/src/test/suite/experiments/index.test.ts @@ -125,6 +125,7 @@ suite('Experiments Test Suite', () => { columnOrder: [], columnWidths: {}, columns: columnsFixture, + filters: [], hasCheckpoints: true, hasColumns: true, hasRunningExperiment: true, @@ -683,6 +684,7 @@ suite('Experiments Test Suite', () => { columnOrder: [], columnWidths: {}, columns: [], + filters: [], hasCheckpoints: true, hasColumns: true, hasRunningExperiment: true, diff --git a/extension/src/test/suite/experiments/model/filterBy/tree.test.ts b/extension/src/test/suite/experiments/model/filterBy/tree.test.ts index b46814b50c..04e9c1bae7 100644 --- a/extension/src/test/suite/experiments/model/filterBy/tree.test.ts +++ b/extension/src/test/suite/experiments/model/filterBy/tree.test.ts @@ -109,6 +109,7 @@ suite('Experiments Filter By Tree Test Suite', () => { columnOrder: [], columnWidths: {}, columns: columnsFixture, + filters: [], hasCheckpoints: true, hasColumns: true, hasRunningExperiment: true, @@ -138,6 +139,7 @@ suite('Experiments Filter By Tree Test Suite', () => { columnOrder: [], columnWidths: {}, columns: columnsFixture, + filters: [], hasCheckpoints: true, hasColumns: true, hasRunningExperiment: true, diff --git a/webview/src/experiments/components/table/Table.test.tsx b/webview/src/experiments/components/table/Table.test.tsx index 22cd03ef03..2954ee4c8a 100644 --- a/webview/src/experiments/components/table/Table.test.tsx +++ b/webview/src/experiments/components/table/Table.test.tsx @@ -105,6 +105,7 @@ describe('Table', () => { columnOrder: [], columnWidths: {}, columns: [], + filters: [], hasCheckpoints: false, hasColumns: true, hasRunningExperiment: false, diff --git a/webview/src/experiments/components/table/TableHeader.tsx b/webview/src/experiments/components/table/TableHeader.tsx index 506f2d85f1..bd5b8cecd9 100644 --- a/webview/src/experiments/components/table/TableHeader.tsx +++ b/webview/src/experiments/components/table/TableHeader.tsx @@ -4,7 +4,7 @@ import { Column, ColumnType } from 'dvc/src/experiments/webview/contract' -import React, { useEffect } from 'react' +import React from 'react' import { HeaderGroup } from 'react-table' import cx from 'classnames' import { MessageFromWebviewType } from 'dvc/src/webview/contract' @@ -12,7 +12,6 @@ import { VSCodeDivider } from '@vscode/webview-ui-toolkit/react' import { FilterDefinition } from 'dvc/src/experiments/model/filterBy' import styles from './styles.module.scss' import { countUpperLevels, isFirstLevelHeader } from '../../util/columns' -import { sendMessage } from '../../../shared/vscode' import { ContextMenu } from '../../../shared/components/contextMenu/ContextMenu' import { Draggable, diff --git a/webview/src/test/sort.ts b/webview/src/test/sort.ts index 6df8195de6..5c7a53d7f6 100644 --- a/webview/src/test/sort.ts +++ b/webview/src/test/sort.ts @@ -35,6 +35,7 @@ export const tableData: TableData = { columnOrder: [], columnWidths: {}, columns, + filters: [], hasCheckpoints: false, hasColumns: true, hasRunningExperiment: false, From e00975d7750788783e65793e0e1c1551ae589680 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 9 Jun 2022 11:38:32 +1000 Subject: [PATCH 3/9] fix tests --- extension/src/experiments/index.ts | 2 +- extension/src/experiments/model/index.ts | 4 ++ extension/src/experiments/webview/contract.ts | 3 +- .../experiments/model/filterBy/tree.test.ts | 2 +- .../components/table/MergeHeaderGroups.tsx | 3 +- .../components/table/Table.test.tsx | 46 ++++++++----------- .../components/table/TableHead.tsx | 3 +- .../components/table/TableHeader.tsx | 25 ++++++---- webview/src/stories/Table.stories.tsx | 1 + 9 files changed, 44 insertions(+), 45 deletions(-) diff --git a/extension/src/experiments/index.ts b/extension/src/experiments/index.ts index 7c29be638c..fc60f3fcb9 100644 --- a/extension/src/experiments/index.ts +++ b/extension/src/experiments/index.ts @@ -440,7 +440,7 @@ export class Experiments extends BaseRepository { columnOrder: this.columns.getColumnOrder(), columnWidths: this.columns.getColumnWidths(), columns: this.columns.getSelected(), - filters: this.experiments.getFilters(), + filters: this.experiments.getFilterPaths(), hasCheckpoints: this.hasCheckpoints(), hasColumns: this.columns.hasColumns(), hasRunningExperiment: this.experiments.hasRunningExperiment(), diff --git a/extension/src/experiments/model/index.ts b/extension/src/experiments/model/index.ts index d191bb0d45..4ad929ea5a 100644 --- a/extension/src/experiments/model/index.ts +++ b/extension/src/experiments/model/index.ts @@ -162,6 +162,10 @@ export class ExperimentsModel extends ModelWithPersistence { return [...this.filters.values()] } + public getFilterPaths() { + return [...this.filters.values()].map(({ path }) => path) + } + public canAutoApplyFilters(...filterIdsToRemove: string[]): boolean { if (!this.useFiltersForSelection) { return true diff --git a/extension/src/experiments/webview/contract.ts b/extension/src/experiments/webview/contract.ts index 2dd9cbac61..98d05047d6 100644 --- a/extension/src/experiments/webview/contract.ts +++ b/extension/src/experiments/webview/contract.ts @@ -1,5 +1,4 @@ import { BaseExperimentFields, ValueTree } from '../../cli/reader' -import { FilterDefinition } from '../model/filterBy' import { SortDefinition } from '../model/sortBy' export interface MetricOrParamColumns { @@ -61,7 +60,7 @@ export type TableData = { hasRunningExperiment: boolean rows: Row[] sorts: SortDefinition[] - filters: FilterDefinition[] + filters: string[] } export type InitiallyUndefinedTableData = TableData | undefined diff --git a/extension/src/test/suite/experiments/model/filterBy/tree.test.ts b/extension/src/test/suite/experiments/model/filterBy/tree.test.ts index 04e9c1bae7..006674acca 100644 --- a/extension/src/test/suite/experiments/model/filterBy/tree.test.ts +++ b/extension/src/test/suite/experiments/model/filterBy/tree.test.ts @@ -109,7 +109,7 @@ suite('Experiments Filter By Tree Test Suite', () => { columnOrder: [], columnWidths: {}, columns: columnsFixture, - filters: [], + filters: [accuracyPath], hasCheckpoints: true, hasColumns: true, hasRunningExperiment: true, diff --git a/webview/src/experiments/components/table/MergeHeaderGroups.tsx b/webview/src/experiments/components/table/MergeHeaderGroups.tsx index c6f2a5bd2a..35ac703c6e 100644 --- a/webview/src/experiments/components/table/MergeHeaderGroups.tsx +++ b/webview/src/experiments/components/table/MergeHeaderGroups.tsx @@ -3,7 +3,6 @@ import cx from 'classnames' import { SortDefinition } from 'dvc/src/experiments/model/sortBy' import { Experiment, Column } from 'dvc/src/experiments/webview/contract' import { HeaderGroup } from 'react-table' -import { FilterDefinition } from 'dvc/src/experiments/model/filterBy' import { TableHeader } from './TableHeader' import styles from './styles.module.scss' import { @@ -16,7 +15,7 @@ export const MergedHeaderGroups: React.FC<{ headerGroup: HeaderGroup columns: HeaderGroup[] sorts: SortDefinition[] - filters: FilterDefinition[] + filters: string[] orderedColumns: Column[] onDragUpdate: OnDragOver onDragStart: OnDragStart diff --git a/webview/src/experiments/components/table/Table.test.tsx b/webview/src/experiments/components/table/Table.test.tsx index 2954ee4c8a..4d7c36d9f0 100644 --- a/webview/src/experiments/components/table/Table.test.tsx +++ b/webview/src/experiments/components/table/Table.test.tsx @@ -10,6 +10,7 @@ import React from 'react' import { TableInstance } from 'react-table' import tableDataFixture from 'dvc/src/test/fixtures/expShow/tableData' import { Table } from './Table' +import { SortOrderLabel } from './TableHeader' import styles from './styles.module.scss' import { ExperimentsTable } from '../Experiments' import * as ColumnOrder from '../../hooks/useColumnOrder' @@ -135,11 +136,22 @@ describe('Table', () => { }) describe('Sorting through the UI', () => { + const mockColumnName = 'C' const mockColumnPath = 'params:C' const findSortableColumn = async () => await screen.findByTestId(`header-${mockColumnPath}`) + const clickOnSortOption = async (optionLabel: SortOrderLabel) => { + const column = await screen.findByText(mockColumnName) + fireEvent.contextMenu(column, { + bubbles: true + }) + + const sortOption = await screen.findByText(optionLabel) + fireEvent.click(sortOption) + } + describe('Sortable column', () => { it('should not not have a sorting indicator if it is not sorted yet', () => { renderExperimentsTable() @@ -148,31 +160,9 @@ describe('Table', () => { expect(sortIcons.length).toBe(0) }) - it('should add an ascending sort to the column, if it is not sorted yet', async () => { - renderExperimentsTable() - const column = await findSortableColumn() - - fireEvent.click(column) - - expect(mockedPostMessage).toBeCalledWith({ - payload: { - descending: false, - path: mockColumnPath - }, - type: MessageFromWebviewType.SORT_COLUMN - }) - }) - - it('should work with the keyboard as well', async () => { + it('should be able to add an ascending sort to the column, if it is not sorted yet', async () => { renderExperimentsTable() - const column = await findSortableColumn() - - fireEvent.keyDown(column, { - bubbles: true, - code: 'Enter', - key: 'Enter', - keyCode: 13 - }) + await clickOnSortOption(SortOrderLabel.ASCENDING) expect(mockedPostMessage).toBeCalledWith({ payload: { @@ -183,7 +173,7 @@ describe('Table', () => { }) }) - it('should add a descending sort to the column, if it has an ascending sort', async () => { + it('should add a descending sort to the column, when clicking on the descending option', async () => { renderExperimentsTable({ ...sortingTableDataFixture, sorts: [ @@ -197,7 +187,7 @@ describe('Table', () => { const column = await findSortableColumn() expect(column).toHaveClass('sortingHeaderCellAsc') - fireEvent.click(column) + await clickOnSortOption(SortOrderLabel.DESCENDING) expect(mockedPostMessage).toBeCalledWith({ payload: { @@ -208,7 +198,7 @@ describe('Table', () => { }) }) - it('should remove the column sort if it has a descending sort', async () => { + it('should remove the column sort if the remove option is selected', async () => { renderExperimentsTable({ ...sortingTableDataFixture, sorts: [ @@ -222,7 +212,7 @@ describe('Table', () => { const column = await findSortableColumn() expect(column).toHaveClass('sortingHeaderCellDesc') - fireEvent.click(column) + await clickOnSortOption(SortOrderLabel.NONE) expect(mockedPostMessage).toBeCalledWith({ payload: mockColumnPath, diff --git a/webview/src/experiments/components/table/TableHead.tsx b/webview/src/experiments/components/table/TableHead.tsx index 52e30aeae5..7c5299dc28 100644 --- a/webview/src/experiments/components/table/TableHead.tsx +++ b/webview/src/experiments/components/table/TableHead.tsx @@ -3,7 +3,6 @@ import { Experiment, Column } from 'dvc/src/experiments/webview/contract' import React, { useRef } from 'react' import { HeaderGroup, TableInstance } from 'react-table' import { MessageFromWebviewType } from 'dvc/src/webview/contract' -import { FilterDefinition } from 'dvc/src/experiments/model/filterBy' import styles from './styles.module.scss' import { MergedHeaderGroups } from './MergeHeaderGroups' import { useColumnOrder } from '../../hooks/useColumnOrder' @@ -18,7 +17,7 @@ interface TableHeadProps { instance: TableInstance columns: Column[] sorts: SortDefinition[] - filters: FilterDefinition[] + filters: string[] } export const TableHead: React.FC = ({ diff --git a/webview/src/experiments/components/table/TableHeader.tsx b/webview/src/experiments/components/table/TableHeader.tsx index bd5b8cecd9..f525fbf821 100644 --- a/webview/src/experiments/components/table/TableHeader.tsx +++ b/webview/src/experiments/components/table/TableHeader.tsx @@ -9,7 +9,6 @@ import { HeaderGroup } from 'react-table' import cx from 'classnames' import { MessageFromWebviewType } from 'dvc/src/webview/contract' import { VSCodeDivider } from '@vscode/webview-ui-toolkit/react' -import { FilterDefinition } from 'dvc/src/experiments/model/filterBy' import styles from './styles.module.scss' import { countUpperLevels, isFirstLevelHeader } from '../../util/columns' import { ContextMenu } from '../../../shared/components/contextMenu/ContextMenu' @@ -31,6 +30,12 @@ export enum SortOrder { NONE = 'none' } +export enum SortOrderLabel { + ASCENDING = 'Sort Ascending', + DESCENDING = 'Sort Descending', + NONE = 'Remove Sort' +} + export const ColumnDragHandle: React.FC<{ disabled: boolean column: HeaderGroup @@ -92,6 +97,7 @@ const TableHeaderCell: React.FC<{ onDragOver, onDragStart, onDrop + // eslint-disable-next-line sonarjs/cognitive-complexity }) => { const isPlaceholder = !!column.placeholderOf const canResize = column.canResize && !isPlaceholder @@ -134,13 +140,13 @@ const TableHeaderCell: React.FC<{ (sortOrder === SortOrder.DESCENDING && AllIcons.DOWN_ARROW) || AllIcons.UP_ARROW, onClick: () => {}, - tooltip: 'Sorted column' + tooltip: 'Sorted By' }, { hidden: !hasFilter, icon: AllIcons.LINES, onClick: () => {}, - tooltip: 'Filtered column' + tooltip: 'Filtered By' } ] @@ -189,7 +195,7 @@ interface TableHeaderProps { column: HeaderGroup columns: HeaderGroup[] sorts: SortDefinition[] - filters: FilterDefinition[] + filters: string[] orderedColumns: Column[] onDragOver: OnDragOver onDragStart: OnDragStart @@ -208,7 +214,8 @@ export const TableHeader: React.FC = ({ }) => { const baseColumn = column.placeholderOf || column const sort = sorts.find(sort => sort.path === baseColumn.id) - const filter = filters.find(({ path }) => path === column.id) + + const hasFilter = !!(column.id && filters.includes(column.id)) const isSortable = !column.placeholderOf && !['id', 'timestamp'].includes(column.id) && @@ -256,7 +263,7 @@ export const TableHeader: React.FC = ({ orderedColumns={orderedColumns} sortOrder={sortOrder} sortEnabled={isSortable} - hasFilter={!!filter} + hasFilter={hasFilter} onDragOver={onDragOver} onDragStart={onDragStart} onDrop={onDrop} @@ -269,7 +276,7 @@ export const TableHeader: React.FC = ({ options={[ { id: SortOrder.ASCENDING, - label: 'Sort Ascending', + label: SortOrderLabel.ASCENDING, message: { payload: { descending: false, @@ -280,7 +287,7 @@ export const TableHeader: React.FC = ({ }, { id: SortOrder.DESCENDING, - label: 'Sort Descending', + label: SortOrderLabel.DESCENDING, message: { payload: { descending: true, @@ -291,7 +298,7 @@ export const TableHeader: React.FC = ({ }, { id: SortOrder.NONE, - label: 'Remove Sort', + label: SortOrderLabel.NONE, message: { payload: column.id, type: MessageFromWebviewType.REMOVE_COLUMN_SORT diff --git a/webview/src/stories/Table.stories.tsx b/webview/src/stories/Table.stories.tsx index c426155bb5..e541533a55 100644 --- a/webview/src/stories/Table.stories.tsx +++ b/webview/src/stories/Table.stories.tsx @@ -17,6 +17,7 @@ const tableData: TableData = { 'params:params.yaml:dvc_logs_dir': 300 }, columns: columnsFixture, + filters: ['params:params.yaml:lr'], hasCheckpoints: true, hasColumns: true, hasRunningExperiment: true, From 5304e575aace4185c78f436393c0a1077ddeda04 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 9 Jun 2022 11:53:41 +1000 Subject: [PATCH 4/9] add filters to deeply nested storybook --- extension/src/experiments/model/index.ts | 2 +- extension/src/test/fixtures/expShow/deeplyNested.ts | 5 ++++- webview/src/experiments/components/table/styles.module.scss | 1 - 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/extension/src/experiments/model/index.ts b/extension/src/experiments/model/index.ts index 4ad929ea5a..22d3c30861 100644 --- a/extension/src/experiments/model/index.ts +++ b/extension/src/experiments/model/index.ts @@ -163,7 +163,7 @@ export class ExperimentsModel extends ModelWithPersistence { } public getFilterPaths() { - return [...this.filters.values()].map(({ path }) => path) + return this.getFilters().map(({ path }) => path) } public canAutoApplyFilters(...filterIdsToRemove: string[]): boolean { diff --git a/extension/src/test/fixtures/expShow/deeplyNested.ts b/extension/src/test/fixtures/expShow/deeplyNested.ts index 59c9200399..f35f4913ed 100644 --- a/extension/src/test/fixtures/expShow/deeplyNested.ts +++ b/extension/src/test/fixtures/expShow/deeplyNested.ts @@ -281,7 +281,10 @@ const deeplyNestedTableData: TableData = { changes: [], columnOrder: [], columnWidths: {}, - filters: [], + filters: [ + 'params:params.yaml:nested1.doubled', + 'params:params.yaml:nested1%2Enested2%2Enested3.nested4.nested5b.doubled' + ], hasCheckpoints: false, hasRunningExperiment: false, sorts: [ diff --git a/webview/src/experiments/components/table/styles.module.scss b/webview/src/experiments/components/table/styles.module.scss index d0c2a55c82..0943dc2c80 100644 --- a/webview/src/experiments/components/table/styles.module.scss +++ b/webview/src/experiments/components/table/styles.module.scss @@ -72,7 +72,6 @@ $workspace-row-edge-margin: $edge-padding - $cell-padding; svg { fill: currentColor; transform: scale(0.7); - // stroke-width: 2px; } } } From 495aa910cb56ba80e3c598c60aee127b9e30f36f Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 9 Jun 2022 12:50:31 +1000 Subject: [PATCH 5/9] refactor table head for complexity --- .../components/table/TableHeader.tsx | 149 ++++++++++-------- 1 file changed, 84 insertions(+), 65 deletions(-) diff --git a/webview/src/experiments/components/table/TableHeader.tsx b/webview/src/experiments/components/table/TableHeader.tsx index f525fbf821..1725baca79 100644 --- a/webview/src/experiments/components/table/TableHeader.tsx +++ b/webview/src/experiments/components/table/TableHeader.tsx @@ -21,7 +21,6 @@ import { import { MessagesMenu } from '../../../shared/components/messagesMenu/MessagesMenu' import { MessagesMenuOptionProps } from '../../../shared/components/messagesMenu/MessagesMenuOption' import { IconMenu } from '../../../shared/components/iconMenu/IconMenu' -import { IconMenuItemProps } from '../../../shared/components/iconMenu/IconMenuItem' import { AllIcons } from '../../../shared/components/Icon' export enum SortOrder { @@ -30,6 +29,12 @@ export enum SortOrder { NONE = 'none' } +const possibleOrders = { + false: SortOrder.ASCENDING, + true: SortOrder.DESCENDING, + undefined: SortOrder.NONE +} as const + export enum SortOrderLabel { ASCENDING = 'Sort Ascending', DESCENDING = 'Sort Descending', @@ -73,6 +78,64 @@ export const ColumnDragHandle: React.FC<{ ) } +const calcResizerHeight = ( + isPlaceholder: boolean, + orderedColumns: Column[], + column: HeaderGroup, + columns: HeaderGroup[] +) => { + const nbUpperLevels = isPlaceholder + ? 0 + : countUpperLevels(orderedColumns, column, columns, 0) + return 100 + nbUpperLevels * 92 + '%' +} + +const getHeaderPropsArgs = ( + column: HeaderGroup, + sortEnabled: boolean, + sortOrder: SortOrder +) => { + return { + className: cx( + styles.th, + column.placeholderOf ? styles.placeholderHeaderCell : styles.headerCell, + { + [styles.paramHeaderCell]: column.group === ColumnType.PARAMS, + [styles.metricHeaderCell]: column.group === ColumnType.METRICS, + [styles.depHeaderCell]: column.group === ColumnType.DEPS, + [styles.firstLevelHeader]: isFirstLevelHeader(column.id), + [styles.leafHeader]: column.headers === undefined, + [styles.menuEnabled]: sortEnabled, + [styles.sortingHeaderCellAsc]: + sortOrder === SortOrder.ASCENDING && !column.parent?.placeholderOf, + [styles.sortingHeaderCellDesc]: + sortOrder === SortOrder.DESCENDING && !column.placeholderOf + } + ) + } +} + +const getIconMenuItems = ( + sortEnabled: boolean, + sortOrder: SortOrder, + hasFilter: boolean +) => [ + { + hidden: !sortEnabled || sortOrder === SortOrder.NONE, + icon: + (sortOrder === SortOrder.DESCENDING && AllIcons.DOWN_ARROW) || + AllIcons.UP_ARROW, + onClick: () => {}, + tooltip: 'Table Sorted By' + }, + { + hidden: !hasFilter, + icon: AllIcons.LINES, + onClick: () => {}, + tooltip: 'Table Filtered By' + } +] + const TableHeaderCell: React.FC<{ column: HeaderGroup columns: HeaderGroup[] @@ -97,82 +160,46 @@ const TableHeaderCell: React.FC<{ onDragOver, onDragStart, onDrop - // eslint-disable-next-line sonarjs/cognitive-complexity }) => { - const isPlaceholder = !!column.placeholderOf - const canResize = column.canResize && !isPlaceholder - const nbUpperLevels = isPlaceholder - ? 0 - : countUpperLevels(orderedColumns, column, columns, 0) - const resizerHeight = 100 + nbUpperLevels * 92 + '%' - - const sortingClasses = () => ({ - [styles.sortingHeaderCellAsc]: - sortOrder === 'ascending' && !column.parent?.placeholderOf, - [styles.sortingHeaderCellDesc]: - sortOrder === 'descending' && !column.placeholderOf - }) + const [menuSuppressed, setMenuSuppressed] = React.useState(false) - const headerPropsArgs = () => { - return { - className: cx( - styles.th, - column.placeholderOf ? styles.placeholderHeaderCell : styles.headerCell, - { - [styles.paramHeaderCell]: column.group === ColumnType.PARAMS, - [styles.metricHeaderCell]: column.group === ColumnType.METRICS, - [styles.depHeaderCell]: column.group === ColumnType.DEPS, - [styles.firstLevelHeader]: isFirstLevelHeader(column.id), - [styles.leafHeader]: column.headers === undefined, - [styles.menuEnabled]: sortEnabled, - ...sortingClasses() - } - ) - } - } const isDraggable = !column.placeholderOf && !['id', 'timestamp'].includes(column.id) - const menuItems: IconMenuItemProps[] = [ - { - hidden: !sortEnabled || sortOrder === SortOrder.NONE, - icon: - (sortOrder === SortOrder.DESCENDING && AllIcons.DOWN_ARROW) || - AllIcons.UP_ARROW, - onClick: () => {}, - tooltip: 'Sorted By' - }, - { - hidden: !hasFilter, - icon: AllIcons.LINES, - onClick: () => {}, - tooltip: 'Filtered By' - } - ] - - const [menuSupressed, setMenuSupressed] = React.useState(false) + const isPlaceholder = !!column.placeholderOf + const canResize = column.canResize && !isPlaceholder + const resizerHeight = calcResizerHeight( + isPlaceholder, + orderedColumns, + column, + columns + ) return ( { return !column.isResizing }} >
- +
setMenuSupressed(true)} - onMouseLeave={() => setMenuSupressed(false)} + onMouseEnter={() => setMenuSuppressed(true)} + onMouseLeave={() => setMenuSuppressed(false)} className={styles.columnResizer} style={{ height: resizerHeight }} /> @@ -221,15 +248,7 @@ export const TableHeader: React.FC = ({ !['id', 'timestamp'].includes(column.id) && !column.columns - const sortOrder: SortOrder = (() => { - const possibleOrders = { - false: SortOrder.ASCENDING, - true: SortOrder.DESCENDING, - undefined: SortOrder.NONE - } - - return possibleOrders[`${sort?.descending}`] - })() + const sortOrder: SortOrder = possibleOrders[`${sort?.descending}`] const contextMenuOptions: MessagesMenuOptionProps[] = React.useMemo(() => { const menuOptions: MessagesMenuOptionProps[] = [ From 985c3c93ba08c16179441d259ad30307ff0997b6 Mon Sep 17 00:00:00 2001 From: Matt Seddon Date: Thu, 9 Jun 2022 13:38:24 +1000 Subject: [PATCH 6/9] fix dev server chaos --- webview/webpack.config.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/webview/webpack.config.ts b/webview/webpack.config.ts index e8a3d5e122..1ff8980c0e 100644 --- a/webview/webpack.config.ts +++ b/webview/webpack.config.ts @@ -1,4 +1,5 @@ import { resolve } from 'path' +import type { Configuration as DevServerConfiguration } from 'webpack-dev-server' import { CleanWebpackPlugin } from 'clean-webpack-plugin' import ForkTsCheckerWebpackPlugin from 'fork-ts-checker-webpack-plugin' @@ -7,17 +8,20 @@ const r = (file: string) => resolve(__dirname, file) const styleLoader = 'style-loader' const cssLoader = 'css-loader' -export default { - devServer: { - disableHostCheck: true, - headers: { - 'Access-Control-Allow-Headers': - 'X-Requested-With, content-type, Authorization', - 'Access-Control-Allow-Methods': 'GET, POST, PUT, DELETE, PATCH, OPTIONS', - 'Access-Control-Allow-Origin': '*' - }, - hot: true +const devServer: DevServerConfiguration = { + allowedHosts: 'all', + headers: { + 'Access-Control-Allow-Headers': + 'X-Requested-With, content-type, Authorization', + 'Access-Control-Allow-Methods': 'GET, POST, PUT, DELETE, PATCH, OPTIONS', + 'Access-Control-Allow-Origin': '*' }, + hot: false, + liveReload: true +} + +export default { + devServer, devtool: 'source-map', entry: { experiments: { dependOn: 'react', import: r('src/experiments/index.tsx') }, From a92ef06c32624babf314ab47301259c7930e672d Mon Sep 17 00:00:00 2001 From: Wolmir Nemitz Date: Thu, 9 Jun 2022 16:35:24 -0300 Subject: [PATCH 7/9] Fix PR change requests --- .../components/table/TableHeader.tsx | 24 +++++++------------ .../components/iconMenu/IconMenuItem.tsx | 3 ++- .../components/iconMenu/styles.module.scss | 5 +++- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/webview/src/experiments/components/table/TableHeader.tsx b/webview/src/experiments/components/table/TableHeader.tsx index 1725baca79..e9616fe974 100644 --- a/webview/src/experiments/components/table/TableHeader.tsx +++ b/webview/src/experiments/components/table/TableHeader.tsx @@ -24,9 +24,9 @@ import { IconMenu } from '../../../shared/components/iconMenu/IconMenu' import { AllIcons } from '../../../shared/components/Icon' export enum SortOrder { - ASCENDING = 'ascending', - DESCENDING = 'descending', - NONE = 'none' + ASCENDING = 'Sort Ascending', + DESCENDING = 'Sort Descending', + NONE = 'Remove Sort' } const possibleOrders = { @@ -35,12 +35,6 @@ const possibleOrders = { undefined: SortOrder.NONE } as const -export enum SortOrderLabel { - ASCENDING = 'Sort Ascending', - DESCENDING = 'Sort Descending', - NONE = 'Remove Sort' -} - export const ColumnDragHandle: React.FC<{ disabled: boolean column: HeaderGroup @@ -179,9 +173,6 @@ const TableHeaderCell: React.FC<{ { - return !column.isResizing - }} >
= ({ = ({ } }, { + hidden: sortOrder === SortOrder.DESCENDING, id: SortOrder.DESCENDING, - label: SortOrderLabel.DESCENDING, + label: SortOrder.DESCENDING, message: { payload: { descending: true, @@ -316,8 +309,9 @@ export const TableHeader: React.FC = ({ } }, { + hidden: sortOrder === SortOrder.NONE, id: SortOrder.NONE, - label: SortOrderLabel.NONE, + label: SortOrder.NONE, message: { payload: column.id, type: MessageFromWebviewType.REMOVE_COLUMN_SORT diff --git a/webview/src/shared/components/iconMenu/IconMenuItem.tsx b/webview/src/shared/components/iconMenu/IconMenuItem.tsx index e2ed54a0ce..42737bc04e 100644 --- a/webview/src/shared/components/iconMenu/IconMenuItem.tsx +++ b/webview/src/shared/components/iconMenu/IconMenuItem.tsx @@ -1,4 +1,5 @@ import React from 'react' +import cx from 'classnames' import { TippyProps } from '@tippyjs/react' import styles from './styles.module.scss' import { Icon, IconValues } from '../Icon' @@ -29,7 +30,7 @@ export const IconMenuItem: React.FC = ({