Skip to content

Commit d048c30

Browse files
mattseddonmattseddon
authored
Add columns indicator (select columns) to table indicators (#4293)
1 parent e7b581f commit d048c30

File tree

2 files changed

+170
-108
lines changed

2 files changed

+170
-108
lines changed

‎webview/src/experiments/components/App.test.tsx‎

Lines changed: 146 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1584,136 +1584,178 @@ describe('App', () => {
15841584
expect(sortIndicator).toHaveTextContent('2')
15851585
jest.useRealTimers()
15861586
})
1587-
})
15881587

1589-
it('should show an indicator with the amount of applied filters', () => {
1590-
renderTable({
1591-
...tableDataFixture,
1592-
filters: []
1593-
})
1594-
jest.useFakeTimers()
1595-
const filterIndicator = screen.getByLabelText('filters')
1596-
expect(filterIndicator).toHaveTextContent('')
1588+
it('should show an indicator with the amount of applied filters', () => {
1589+
renderTable({
1590+
...tableDataFixture,
1591+
filters: []
1592+
})
1593+
jest.useFakeTimers()
1594+
const filterIndicator = screen.getByLabelText('filters')
1595+
expect(filterIndicator).toHaveTextContent('')
15971596

1598-
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
1597+
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
15991598

1600-
fireEvent.mouseEnter(filterIndicator)
1601-
advanceTimersByTime(1000)
1599+
fireEvent.mouseEnter(filterIndicator)
1600+
advanceTimersByTime(1000)
16021601

1603-
const tooltip = screen.getByRole('tooltip')
1602+
const tooltip = screen.getByRole('tooltip')
16041603

1605-
expect(tooltip).toHaveTextContent('Show Filters')
1604+
expect(tooltip).toHaveTextContent('Show Filters')
16061605

1607-
const { columns } = tableDataFixture
1608-
const firstFilterPath = columns[columns.length - 1].path
1609-
const secondFilterPath = columns[columns.length - 2].path
1610-
setTableData({
1611-
...tableDataFixture,
1612-
filters: [firstFilterPath]
1613-
})
1614-
expect(filterIndicator).toHaveTextContent('1')
1606+
const { columns } = tableDataFixture
1607+
const firstFilterPath = columns[columns.length - 1].path
1608+
const secondFilterPath = columns[columns.length - 2].path
1609+
setTableData({
1610+
...tableDataFixture,
1611+
filters: [firstFilterPath]
1612+
})
1613+
expect(filterIndicator).toHaveTextContent('1')
16151614

1616-
setTableData({
1617-
...tableDataFixture,
1618-
filters: [firstFilterPath, secondFilterPath]
1619-
})
1620-
expect(filterIndicator).toHaveTextContent('2')
1615+
setTableData({
1616+
...tableDataFixture,
1617+
filters: [firstFilterPath, secondFilterPath]
1618+
})
1619+
expect(filterIndicator).toHaveTextContent('2')
16211620

1622-
setTableData({
1623-
...tableDataFixture,
1624-
filters: [firstFilterPath, secondFilterPath]
1625-
})
1626-
expect(filterIndicator).toHaveTextContent('2')
1621+
setTableData({
1622+
...tableDataFixture,
1623+
filters: [firstFilterPath, secondFilterPath]
1624+
})
1625+
expect(filterIndicator).toHaveTextContent('2')
16271626

1628-
setTableData({
1629-
...tableDataFixture,
1630-
filters: []
1627+
setTableData({
1628+
...tableDataFixture,
1629+
filters: []
1630+
})
1631+
expect(filterIndicator).toHaveTextContent('')
1632+
expect(tooltip).not.toHaveTextContent('Experiment')
1633+
jest.useRealTimers()
16311634
})
1632-
expect(filterIndicator).toHaveTextContent('')
1633-
expect(tooltip).not.toHaveTextContent('Experiment')
1634-
jest.useRealTimers()
1635-
})
16361635

1637-
it('should show a tooltip for the branches indicator', () => {
1638-
renderTable({
1639-
...tableDataFixture
1640-
})
1641-
jest.useFakeTimers()
1642-
const branchesIndicator = screen.getByLabelText('branches')
1643-
expect(branchesIndicator).toHaveTextContent('')
1636+
it('should show a tooltip for the branches indicator', () => {
1637+
renderTable({
1638+
...tableDataFixture
1639+
})
1640+
jest.useFakeTimers()
1641+
const branchesIndicator = screen.getByLabelText('branches')
1642+
expect(branchesIndicator).toHaveTextContent('')
16441643

1645-
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
1644+
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
16461645

1647-
fireEvent.mouseEnter(branchesIndicator)
1648-
advanceTimersByTime(1000)
1646+
fireEvent.mouseEnter(branchesIndicator)
1647+
advanceTimersByTime(1000)
16491648

1650-
const tooltip = screen.getByRole('tooltip')
1649+
const tooltip = screen.getByRole('tooltip')
16511650

1652-
expect(tooltip).toHaveTextContent('Select Branches')
1653-
jest.useRealTimers()
1654-
})
1651+
expect(tooltip).toHaveTextContent('Select Branches')
1652+
jest.useRealTimers()
1653+
})
16551654

1656-
it('should show an indicator for the number of branches selected', () => {
1657-
const branches = ['main', 'other', 'third']
1655+
it('should show an indicator for the number of branches selected', () => {
1656+
const branches = ['main', 'other', 'third']
16581657

1659-
let workspace
1660-
const rowsWithoutWorkspace = []
1661-
for (const row of tableDataFixture.rows) {
1662-
if (row.id !== EXPERIMENT_WORKSPACE_ID) {
1663-
rowsWithoutWorkspace.push(row)
1664-
continue
1658+
let workspace
1659+
const rowsWithoutWorkspace = []
1660+
for (const row of tableDataFixture.rows) {
1661+
if (row.id !== EXPERIMENT_WORKSPACE_ID) {
1662+
rowsWithoutWorkspace.push(row)
1663+
continue
1664+
}
1665+
workspace = row
16651666
}
1666-
workspace = row
1667-
}
16681667

1669-
const multipleBranches = {
1670-
...tableDataFixture,
1671-
branches,
1672-
hasData: true,
1673-
rows: [
1674-
workspace as Commit,
1675-
...rowsWithoutWorkspace.map(row => ({
1676-
...row,
1677-
branch: branches[0],
1678-
subRows: undefined
1679-
})),
1680-
...rowsWithoutWorkspace.map(row => ({
1681-
...row,
1682-
branch: branches[1],
1683-
subRows: undefined
1684-
})),
1685-
...rowsWithoutWorkspace.map(row => ({
1686-
...row,
1687-
branch: branches[2],
1688-
subRows: undefined
1689-
}))
1690-
]
1691-
}
1668+
const multipleBranches = {
1669+
...tableDataFixture,
1670+
branches,
1671+
hasData: true,
1672+
rows: [
1673+
workspace as Commit,
1674+
...rowsWithoutWorkspace.map(row => ({
1675+
...row,
1676+
branch: branches[0],
1677+
subRows: undefined
1678+
})),
1679+
...rowsWithoutWorkspace.map(row => ({
1680+
...row,
1681+
branch: branches[1],
1682+
subRows: undefined
1683+
})),
1684+
...rowsWithoutWorkspace.map(row => ({
1685+
...row,
1686+
branch: branches[2],
1687+
subRows: undefined
1688+
}))
1689+
]
1690+
}
16921691

1693-
renderTable(multipleBranches)
1692+
renderTable(multipleBranches)
16941693

1695-
const [indicator] = screen.getAllByLabelText('branches')
1694+
const [indicator] = screen.getAllByLabelText('branches')
16961695

1697-
expect(indicator).toHaveTextContent(`${branches.length - 1}`)
1698-
})
1696+
expect(indicator).toHaveTextContent(`${branches.length - 1}`)
1697+
})
16991698

1700-
it('should send a message to focus the relevant tree when clicked', () => {
1701-
renderTable()
1702-
mockPostMessage.mockClear()
1703-
fireEvent.click(screen.getByLabelText('sorts'))
1704-
expect(mockPostMessage).toHaveBeenCalledWith({
1705-
type: MessageFromWebviewType.FOCUS_SORTS_TREE
1699+
it('should send a message to focus the relevant tree when clicked', () => {
1700+
renderTable()
1701+
mockPostMessage.mockClear()
1702+
fireEvent.click(screen.getByLabelText('sorts'))
1703+
expect(mockPostMessage).toHaveBeenCalledWith({
1704+
type: MessageFromWebviewType.FOCUS_SORTS_TREE
1705+
})
1706+
mockPostMessage.mockClear()
1707+
fireEvent.click(screen.getByLabelText('filters'))
1708+
expect(mockPostMessage).toHaveBeenCalledWith({
1709+
type: MessageFromWebviewType.FOCUS_FILTERS_TREE
1710+
})
1711+
1712+
mockPostMessage.mockClear()
1713+
fireEvent.click(screen.getByLabelText('selected for plots'))
1714+
expect(mockPostMessage).toHaveBeenCalledWith({
1715+
type: MessageFromWebviewType.OPEN_PLOTS_WEBVIEW
1716+
})
17061717
})
1707-
mockPostMessage.mockClear()
1708-
fireEvent.click(screen.getByLabelText('filters'))
1709-
expect(mockPostMessage).toHaveBeenCalledWith({
1710-
type: MessageFromWebviewType.FOCUS_FILTERS_TREE
1718+
1719+
it('should show an indicator with the amount of displayed columns', () => {
1720+
renderTable({
1721+
...tableDataFixture
1722+
})
1723+
jest.useFakeTimers()
1724+
const columnsIndicator = screen.getByLabelText('columns')
1725+
expect(columnsIndicator).toHaveTextContent('22')
1726+
1727+
expect(screen.queryByRole('tooltip')).not.toBeInTheDocument()
1728+
1729+
fireEvent.mouseEnter(columnsIndicator)
1730+
advanceTimersByTime(1000)
1731+
const tooltip = screen.getByRole('tooltip')
1732+
1733+
expect(tooltip).toHaveTextContent('Select Columns')
1734+
1735+
setTableData({
1736+
...tableDataFixture,
1737+
columns: tableDataFixture.columns.slice(1)
1738+
})
1739+
1740+
expect(columnsIndicator).toHaveTextContent('21')
1741+
1742+
setTableData({
1743+
...tableDataFixture,
1744+
columns: []
1745+
})
1746+
1747+
expect(columnsIndicator).toHaveTextContent('')
1748+
1749+
jest.useRealTimers()
17111750
})
17121751

1713-
mockPostMessage.mockClear()
1714-
fireEvent.click(screen.getByLabelText('selected for plots'))
1715-
expect(mockPostMessage).toHaveBeenCalledWith({
1716-
type: MessageFromWebviewType.OPEN_PLOTS_WEBVIEW
1752+
it('should send a message to select columns when the select columns icon is clicked', () => {
1753+
renderTable()
1754+
mockPostMessage.mockClear()
1755+
fireEvent.click(screen.getByLabelText('columns'))
1756+
expect(mockPostMessage).toHaveBeenCalledWith({
1757+
type: MessageFromWebviewType.SELECT_COLUMNS
1758+
})
17171759
})
17181760
})
17191761

‎webview/src/experiments/components/table/Indicators.tsx‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@ import {
66
focusFiltersTree,
77
focusSortsTree,
88
openPlotsWebview,
9-
selectBranches
9+
selectBranches,
10+
selectColumns
1011
} from '../../util/messages'
1112
import { Icon } from '../../../shared/components/Icon'
1213
import {
1314
Filter,
1415
GitMerge,
1516
GraphScatter,
17+
ListFilter,
1618
SortPrecedence
1719
} from '../../../shared/components/icons'
1820
import { ExperimentsState } from '../../store'
@@ -72,21 +74,30 @@ export const Indicators = () => {
7274
const filters = useSelector(
7375
(state: ExperimentsState) => state.tableData.filters
7476
)
77+
const filtersCount = filters?.length
78+
7579
const sorts = useSelector((state: ExperimentsState) => state.tableData.sorts)
80+
const sortsCount = sorts?.length
81+
7682
const selectedForPlotsCount = useSelector(
7783
(state: ExperimentsState) => state.tableData.selectedForPlotsCount
7884
)
85+
7986
const branchesSelected = useSelector(
8087
(state: ExperimentsState) =>
8188
Math.max(state.tableData.branches.filter(Boolean).length - 1, 0) // We always have one branch by default (the current one which is not selected) and undefined for the workspace
8289
)
83-
8490
const { hasBranchesToSelect } = useSelector(
8591
(state: ExperimentsState) => state.tableData
8692
)
8793

88-
const sortsCount = sorts?.length
89-
const filtersCount = filters?.length
94+
const columnsSelected = useSelector(
95+
(state: ExperimentsState) =>
96+
state.tableData.columns.filter(({ hasChildren }) => !hasChildren).length
97+
)
98+
const hasColumns = useSelector(
99+
(state: ExperimentsState) => state.tableData.hasColumns
100+
)
90101

91102
return (
92103
<div className={styles.tableIndicators}>
@@ -123,6 +134,15 @@ export const Indicators = () => {
123134
>
124135
<Icon width={16} height={16} icon={GitMerge} />
125136
</Indicator>
137+
<Indicator
138+
count={columnsSelected}
139+
aria-label="columns"
140+
onClick={selectColumns}
141+
tooltipContent="Select Columns"
142+
disabled={!hasColumns}
143+
>
144+
<Icon width={16} height={16} icon={ListFilter} />
145+
</Indicator>
126146
</div>
127147
)
128148
}

0 commit comments

Comments
 (0)