Skip to content

Commit f735741

Browse files
authored
fix: delete modal closing prematurely, rows keeps refetching unnecess… (#941)
## Problem 1. Delete row modal closes immediately after Delete is clicked, it does not show the loading bar like it should 2. Rows are refetched each time GET_TABLE query is refetched. The happens when the following is changed: column width, column ordering, collaborator changes, etc... ## Solution 1. Re-use the delete modal in the ContextProviderContext 2. Prevent calling refetch() after GET_TABLE is called Fixes PLU-465 Fixes PLU-463
1 parent 588b0bb commit f735741

File tree

5 files changed

+83
-101
lines changed

5 files changed

+83
-101
lines changed

packages/frontend/src/pages/Tile/components/Table.tsx

Lines changed: 60 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -206,54 +206,55 @@ export default function Table(): JSX.Element {
206206
const virtualRows = rowVirtualizer.getVirtualItems()
207207

208208
return (
209-
<Box position="relative">
210-
<Flex
211-
overflow="auto"
212-
w="100%"
213-
ref={parentRef}
214-
minH="300px"
215-
flexDir="column"
216-
position="relative"
217-
// so that search bar results dont get blocked by header or footer
218-
scrollPaddingTop={ROW_HEIGHT.HEADER + 'px'}
219-
scrollPaddingBottom={ROW_HEIGHT.FOOTER + ROW_HEIGHT.DEFAULT + 'px'}
220-
// so highlighted element doesnt get blocked by checkbox column
221-
scrollPaddingLeft={60}
222-
onClick={onBlurClick}
223-
className={styles.table}
224-
>
209+
<ContextMenuContextProvider
210+
removeRows={removeRows}
211+
rowSelection={rowSelection}
212+
clearRowSelection={() => setRowSelection({})}
213+
>
214+
<Box position="relative">
225215
<Flex
216+
overflow="auto"
217+
w="100%"
218+
ref={parentRef}
219+
minH="300px"
226220
flexDir="column"
227-
w="fit-content"
228-
minW="100%"
229-
flex={1}
230-
ref={containerRef}
221+
position="relative"
222+
// so that search bar results dont get blocked by header or footer
223+
scrollPaddingTop={ROW_HEIGHT.HEADER + 'px'}
224+
scrollPaddingBottom={ROW_HEIGHT.FOOTER + ROW_HEIGHT.DEFAULT + 'px'}
225+
// so highlighted element doesnt get blocked by checkbox column
226+
scrollPaddingLeft={60}
227+
onClick={onBlurClick}
228+
className={styles.table}
231229
>
232230
<Flex
233-
bgColor={HEADER_COLOR.DEFAULT}
234-
alignItems="stretch"
235-
position="sticky"
236-
top={0}
237-
h={`${ROW_HEIGHT.HEADER}px`}
238-
maxH={`${ROW_HEIGHT.HEADER}px`}
239-
zIndex={Z_INDEX.FOOTER}
240-
borderTopWidth="1px"
241-
borderBottomWidth="1px"
242-
borderColor={BORDER_COLOR.DEFAULT}
243-
marginBottom="1px"
244-
fontWeight="bold"
245-
>
246-
<TableHeader
247-
setColumnOrder={setColumnOrder}
248-
headers={table.getFlatHeaders()}
249-
tableState={table.getState()}
250-
/>
251-
</Flex>
252-
<ContextMenuContextProvider
253-
removeRows={removeRows}
254-
rowSelection={rowSelection}
255-
clearRowSelection={() => setRowSelection({})}
231+
flexDir="column"
232+
w="fit-content"
233+
minW="100%"
234+
flex={1}
235+
ref={containerRef}
256236
>
237+
<Flex
238+
bgColor={HEADER_COLOR.DEFAULT}
239+
alignItems="stretch"
240+
position="sticky"
241+
top={0}
242+
h={`${ROW_HEIGHT.HEADER}px`}
243+
maxH={`${ROW_HEIGHT.HEADER}px`}
244+
zIndex={Z_INDEX.FOOTER}
245+
borderTopWidth="1px"
246+
borderBottomWidth="1px"
247+
borderColor={BORDER_COLOR.DEFAULT}
248+
marginBottom="1px"
249+
fontWeight="bold"
250+
>
251+
<TableHeader
252+
setColumnOrder={setColumnOrder}
253+
headers={table.getFlatHeaders()}
254+
tableState={table.getState()}
255+
/>
256+
</Flex>
257+
257258
<Box h={rowVirtualizer.getTotalSize()} ref={childRef}>
258259
{virtualRows.map((virtualRow) => {
259260
const row = rows[virtualRow.index] as Row<GenericRowData>
@@ -271,23 +272,22 @@ export default function Table(): JSX.Element {
271272
}
272273
})}
273274
</Box>
274-
</ContextMenuContextProvider>
275-
</Flex>
276-
{mode === 'edit' && (
277-
<TableRow
278-
tableMeta={table.options.meta as TableMeta<GenericRowData>}
279-
row={newRow}
280-
stickyBottom
275+
</Flex>
276+
{mode === 'edit' && (
277+
<TableRow
278+
tableMeta={table.options.meta as TableMeta<GenericRowData>}
279+
row={newRow}
280+
stickyBottom
281+
/>
282+
)}
283+
<TableFooter
284+
rowCount={rows.length}
285+
rowSelection={rowSelection}
286+
parentRef={parentRef}
281287
/>
282-
)}
283-
<TableFooter
284-
removeRows={removeRows}
285-
rowCount={rows.length}
286-
rowSelection={rowSelection}
287-
parentRef={parentRef}
288-
/>
289-
</Flex>
290-
<SearchBar table={table} rowVirtualizer={rowVirtualizer} />
291-
</Box>
288+
</Flex>
289+
<SearchBar table={table} rowVirtualizer={rowVirtualizer} />
290+
</Box>
291+
</ContextMenuContextProvider>
292292
)
293293
}
Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,18 @@
1-
import { useState } from 'react'
21
import { BiTrash } from 'react-icons/bi'
32
import { Button } from '@chakra-ui/react'
43

54
import { ROW_HEIGHT } from '@/pages/Tile/constants'
65

6+
import { useContextMenuContext } from '../../contexts/ContextMenuContext'
77
import { useTableContext } from '../../contexts/TableContext'
88

9-
import DeleteRowsModal from './DeleteRowsModal'
10-
11-
interface DeleteRowsButtonProps {
12-
rowSelection: Record<string, boolean>
13-
removeRows: (rowIds: string[]) => void
14-
}
15-
16-
export default function DeleteRowsButton({
17-
rowSelection,
18-
removeRows,
19-
}: DeleteRowsButtonProps) {
9+
export default function DeleteRowsButton() {
2010
const { mode } = useTableContext()
21-
const isViewMode = mode === 'view'
22-
23-
const rowsSelected = Object.keys(rowSelection)
11+
const { onDeleteRows } = useContextMenuContext()
2412

25-
const [isDialogOpen, setIsDialogOpen] = useState(false)
13+
const isViewMode = mode === 'view'
2614

27-
if (isViewMode || !rowsSelected.length) {
15+
if (isViewMode) {
2816
return null
2917
}
3018

@@ -42,18 +30,10 @@ export default function DeleteRowsButton({
4230
h="100%"
4331
colorScheme="critical"
4432
leftIcon={<BiTrash />}
45-
onClick={() => setIsDialogOpen(true)}
33+
onClick={() => onDeleteRows()}
4634
>
47-
Delete
35+
Delete rows
4836
</Button>
49-
{/* lazy load the dialog */}
50-
{isDialogOpen && (
51-
<DeleteRowsModal
52-
rowIdsToDelete={rowsSelected}
53-
removeRows={removeRows}
54-
onClose={() => setIsDialogOpen(false)}
55-
/>
56-
)}
5737
</div>
5838
)
5939
}

packages/frontend/src/pages/Tile/components/TableFooter/index.tsx

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,12 @@ import DeleteRowsButton from './DeleteRowsButton'
99
import RowCount from './RowCount'
1010

1111
interface TableFooterProps {
12-
removeRows: (rows: string[]) => void
1312
rowCount: number
1413
rowSelection: Record<string, boolean>
1514
parentRef: React.RefObject<HTMLDivElement>
1615
}
1716

18-
function TableFooter({
19-
removeRows,
20-
rowCount,
21-
rowSelection,
22-
parentRef,
23-
}: TableFooterProps) {
17+
function TableFooter({ rowCount, rowSelection, parentRef }: TableFooterProps) {
2418
return (
2519
<Flex
2620
w="100%"
@@ -38,7 +32,7 @@ function TableFooter({
3832
>
3933
<Flex>
4034
<RowCount rowCount={rowCount} rowSelection={rowSelection} />
41-
<DeleteRowsButton rowSelection={rowSelection} removeRows={removeRows} />
35+
{Object.keys(rowSelection).length > 0 && <DeleteRowsButton />}
4236
</Flex>
4337
<Flex>
4438
<Button

packages/frontend/src/pages/Tile/contexts/ContextMenuContext.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import DeleteRowsModal from '../components/TableFooter/DeleteRowsModal'
2020

2121
interface ContextMenuContextProps {
2222
onRightClick: (rowId: string, pos: [number, number]) => void
23-
onDeleteRows: (rowIdsToDelete: string[]) => void
23+
onDeleteRows: () => void
2424
}
2525

2626
const ContextMenuContext = createContext<ContextMenuContextProps | null>(null)
@@ -72,6 +72,11 @@ export const ContextMenuContextProvider = ({
7272
[clearRowSelection, rowsSelected],
7373
)
7474

75+
const onDeleteRows = useCallback(() => {
76+
setRowIdsSelected(rowsSelected)
77+
setIsDeleteModalOpen(true)
78+
}, [rowsSelected])
79+
7580
const onRowIdCopy = useCallback(
7681
(rowId: string) => {
7782
navigator.clipboard.writeText(rowId)
@@ -84,7 +89,7 @@ export const ContextMenuContextProvider = ({
8489
<ContextMenuContext.Provider
8590
value={{
8691
onRightClick,
87-
onDeleteRows: () => setIsDeleteModalOpen(true),
92+
onDeleteRows,
8893
}}
8994
>
9095
{children}

packages/frontend/src/pages/Tile/index.tsx

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { ITableMetadata } from '@plumber/types'
22

3+
import { useEffect } from 'react'
34
import { useParams } from 'react-router-dom'
45
import { ApolloError, useQuery } from '@apollo/client'
56
import { Center, Flex } from '@chakra-ui/react'
@@ -43,13 +44,15 @@ export default function Tile(): JSX.Element | null {
4344
headers: { 'x-tiles-view-key': urlViewOnlyKey },
4445
}
4546
: undefined,
46-
onCompleted: () => {
47-
// only start fetching rows after table metadata is loaded
48-
refetch()
49-
},
5047
})
5148
const ownRole = getTableData?.getTable?.role
5249

50+
useEffect(() => {
51+
if (isGetTableCalled) {
52+
refetch()
53+
}
54+
}, [isGetTableCalled, refetch])
55+
5356
// On first load, show loading spinner
5457
if (isTableLoading && !isGetTableCalled) {
5558
return (

0 commit comments

Comments
 (0)