Skip to content

Commit

Permalink
Use only one observer for exp column
Browse files Browse the repository at this point in the history
  • Loading branch information
julieg18 committed Jul 19, 2022
1 parent 407c39c commit 903dd87
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 137 deletions.
22 changes: 5 additions & 17 deletions webview/src/experiments/components/table/Cell.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import React from 'react'
import { useInView } from 'react-intersection-observer'
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, WithTableRoot, RowProp } from './interfaces'
import { CellProp, RowProp } from './interfaces'
import ClockIcon from '../../../shared/components/icons/Clock'
import { clickAndEnterProps } from '../../../util/props'
import { StarFull, StarEmpty } from '../../../shared/components/icons'
Expand Down Expand Up @@ -130,38 +129,27 @@ export const RowActions: React.FC<RowActionsProps> = ({

export const FirstCell: React.FC<
CellProp &
RowActionsProps &
WithTableRoot & {
RowActionsProps & {
bulletColor?: string
toggleExperiment: () => void
isRowSelected: boolean
toggleRowSelection: () => void
toggleStarred: () => void
}
> = ({ cell, bulletColor, toggleExperiment, root, ...rowActionsProps }) => {
> = ({ cell, bulletColor, toggleExperiment, ...rowActionsProps }) => {
const { row, isPlaceholder } = cell
const {
original: { queued }
} = row

const {
subRowStates: { plotSelections }
} = rowActionsProps

const [ref, needsShadow] = useInView({
root,
rootMargin: '0px 0px 0px -15px',
threshold: 1
})

return (
<div
ref={ref}
{...cell.getCellProps({
className: cx(
styles.td,
styles.experimentCell,
isPlaceholder && styles.groupPlaceholder,
needsShadow && styles.withExpCellShadow
isPlaceholder && styles.groupPlaceholder
)
})}
>
Expand Down
33 changes: 18 additions & 15 deletions webview/src/experiments/components/table/MergeHeaderGroups.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,25 @@ import { Experiment, Column } from 'dvc/src/experiments/webview/contract'
import { HeaderGroup } from 'react-table'
import { TableHeader } from './TableHeader'
import styles from './styles.module.scss'
import { WithTableRoot } from './interfaces'
import {
OnDragOver,
OnDragStart,
OnDrop
} from '../../../shared/components/dragDrop/DragDropWorkbench'

export const MergedHeaderGroups: React.FC<
{
headerGroup: HeaderGroup<Experiment>
columns: HeaderGroup<Experiment>[]
sorts: SortDefinition[]
filters: string[]
orderedColumns: Column[]
onDragUpdate: OnDragOver
onDragStart: OnDragStart
onDragEnd: OnDrop
} & WithTableRoot
> = ({
export const MergedHeaderGroups: React.FC<{
headerGroup: HeaderGroup<Experiment>
columns: HeaderGroup<Experiment>[]
sorts: SortDefinition[]
filters: string[]
orderedColumns: Column[]
onDragUpdate: OnDragOver
onDragStart: OnDragStart
onDragEnd: OnDrop
isFirst: boolean
setExpColumnNeedsShadow: (needsShadow: boolean) => void
root: HTMLElement | null
}> = ({
headerGroup,
sorts,
filters,
Expand All @@ -32,7 +32,9 @@ export const MergedHeaderGroups: React.FC<
onDragUpdate,
onDragEnd,
onDragStart,
root
root,
isFirst,
setExpColumnNeedsShadow
}) => {
return (
<div
Expand All @@ -42,7 +44,8 @@ export const MergedHeaderGroups: React.FC<
>
{headerGroup.headers.map((column: HeaderGroup<Experiment>, ind) => (
<TableHeader
isFirst={ind === 0}
isFirst={isFirst && ind === 0}
setExpColumnNeedsShadow={setExpColumnNeedsShadow}
key={column.id}
orderedColumns={orderedColumns}
column={column}
Expand Down
10 changes: 3 additions & 7 deletions webview/src/experiments/components/table/Row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react'
import cx from 'classnames'
import { Experiment } from 'dvc/src/experiments/webview/contract'
import { MessageFromWebviewType } from 'dvc/src/webview/contract'
import { RowProp, WithChanges, WithTableRoot } from './interfaces'
import { RowProp, WithChanges } from './interfaces'
import styles from './styles.module.scss'
import { FirstCell, CellWrapper } from './Cell'
import { RowSelectionContext } from './RowSelectionContext'
Expand Down Expand Up @@ -307,18 +307,15 @@ export type BatchSelectionProp = {
}

export const RowContent: React.FC<
RowProp & { className?: string } & WithChanges &
BatchSelectionProp &
WithTableRoot
RowProp & { className?: string } & WithChanges & BatchSelectionProp
> = ({
row,
className,
changes,
contextMenuDisabled,
projectHasCheckpoints,
hasRunningExperiment,
batchRowSelection,
root
batchRowSelection
}): JSX.Element => {
const {
getRowProps,
Expand Down Expand Up @@ -409,7 +406,6 @@ export const RowContent: React.FC<
data-testid={isWorkspace && 'workspace-row'}
>
<FirstCell
root={root}
cell={firstCell}
bulletColor={displayColor}
starred={starred}
Expand Down
40 changes: 16 additions & 24 deletions webview/src/experiments/components/table/Table.tsx
Original file line number Diff line number Diff line change
@@ -1,33 +1,25 @@
import React, { useRef } from 'react'
import React, { useRef, useState } from 'react'
import cx from 'classnames'
import styles from './styles.module.scss'
import { TableHead } from './TableHead'
import { BatchSelectionProp, RowContent } from './Row'
import {
InstanceProp,
RowProp,
TableProps,
WithChanges,
WithTableRoot
} from './interfaces'
import { InstanceProp, RowProp, TableProps, WithChanges } from './interfaces'
import { RowSelectionContext } from './RowSelectionContext'
import { useClickOutside } from '../../../shared/hooks/useClickOutside'

export const NestedRow: React.FC<
RowProp & InstanceProp & BatchSelectionProp & WithTableRoot
RowProp & InstanceProp & BatchSelectionProp
> = ({
row,
instance,
contextMenuDisabled,
projectHasCheckpoints,
hasRunningExperiment,
batchRowSelection,
root
batchRowSelection
}) => {
instance.prepareRow(row)
return (
<RowContent
root={root}
row={row}
className={styles.nestedRow}
contextMenuDisabled={contextMenuDisabled}
Expand All @@ -39,15 +31,14 @@ export const NestedRow: React.FC<
}

export const ExperimentGroup: React.FC<
RowProp & InstanceProp & BatchSelectionProp & WithTableRoot
RowProp & InstanceProp & BatchSelectionProp
> = ({
row,
instance,
contextMenuDisabled,
projectHasCheckpoints,
hasRunningExperiment,
batchRowSelection,
root
batchRowSelection
}) => {
instance.prepareRow(row)
return (
Expand All @@ -58,7 +49,6 @@ export const ExperimentGroup: React.FC<
)}
>
<NestedRow
root={root}
row={row}
instance={instance}
contextMenuDisabled={contextMenuDisabled}
Expand All @@ -69,7 +59,6 @@ export const ExperimentGroup: React.FC<
{row.isExpanded &&
row.subRows.map(row => (
<NestedRow
root={root}
row={row}
instance={instance}
key={row.id}
Expand All @@ -84,16 +73,15 @@ export const ExperimentGroup: React.FC<
}

export const TableBody: React.FC<
RowProp & InstanceProp & WithChanges & BatchSelectionProp & WithTableRoot
RowProp & InstanceProp & WithChanges & BatchSelectionProp
> = ({
row,
instance,
changes,
contextMenuDisabled,
projectHasCheckpoints,
hasRunningExperiment,
batchRowSelection,
root
batchRowSelection
}) => {
instance.prepareRow(row)
return (
Expand All @@ -109,7 +97,6 @@ export const TableBody: React.FC<
})}
>
<RowContent
root={root}
row={row}
projectHasCheckpoints={projectHasCheckpoints}
hasRunningExperiment={hasRunningExperiment}
Expand All @@ -120,7 +107,6 @@ export const TableBody: React.FC<
{row.isExpanded &&
row.subRows.map(subRow => (
<ExperimentGroup
root={root}
row={subRow}
instance={instance}
key={subRow.values.id}
Expand Down Expand Up @@ -151,6 +137,7 @@ export const Table: React.FC<TableProps & WithChanges> = ({

const { clearSelectedRows, batchSelection, lastSelectedRow } =
React.useContext(RowSelectionContext)
const [expColumnNeedsShadow, setExpColumnNeedsShadow] = useState(false)

const tableRef = useRef<HTMLDivElement>(null)

Expand Down Expand Up @@ -192,7 +179,12 @@ export const Table: React.FC<TableProps & WithChanges> = ({
return (
<div className={styles.tableContainer}>
<div
{...getTableProps({ className: styles.table })}
{...getTableProps({
className: cx(
styles.table,
expColumnNeedsShadow && styles.withExpColumnShadow
)
})}
ref={tableRef}
tabIndex={0}
role="tree"
Expand All @@ -209,6 +201,7 @@ export const Table: React.FC<TableProps & WithChanges> = ({
filters={filters}
columns={columns}
root={tableRef.current}
setExpColumnNeedsShadow={setExpColumnNeedsShadow}
/>
{rows.map(row => (
<TableBody
Expand All @@ -219,7 +212,6 @@ export const Table: React.FC<TableProps & WithChanges> = ({
hasRunningExperiment={hasRunningExperiment}
projectHasCheckpoints={hasCheckpoints}
batchRowSelection={batchRowSelection}
root={tableRef.current}
/>
))}
</div>
Expand Down
8 changes: 6 additions & 2 deletions webview/src/experiments/components/table/TableHead.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ interface TableHeadProps {
filteredCounts: FilteredCounts
filters: string[]
root: HTMLElement | null
setExpColumnNeedsShadow: (needsShadow: boolean) => void
}

export const TableHead = ({
Expand All @@ -37,7 +38,8 @@ export const TableHead = ({
filteredCounts,
filters,
columns,
sorts
sorts,
setExpColumnNeedsShadow
}: TableHeadProps) => {
const orderedColumns = useColumnOrder(columns, columnOrder)
const allHeaders: HeaderGroup<Experiment>[] = []
Expand Down Expand Up @@ -105,7 +107,7 @@ export const TableHead = ({
filters={filters}
filteredCounts={filteredCounts}
/>
{headerGroups.map(headerGroup => (
{headerGroups.map((headerGroup, ind) => (
// eslint-disable-next-line react/jsx-key
<MergedHeaderGroups
{...headerGroup.getHeaderGroupProps()}
Expand All @@ -118,6 +120,8 @@ export const TableHead = ({
onDragUpdate={onDragUpdate}
onDragEnd={onDragEnd}
root={root}
isFirst={ind === 0}
setExpColumnNeedsShadow={setExpColumnNeedsShadow}
/>
))}
</div>
Expand Down
Loading

0 comments on commit 903dd87

Please sign in to comment.