Skip to content

Commit c489df8

Browse files
suejung-sentrysuejung-sentry
authored
fix: Update viewFile toggle in commit detail page (#3623)
1 parent 82938e3 commit c489df8

File tree

8 files changed

+216
-268
lines changed

8 files changed

+216
-268
lines changed

‎src/pages/CommitDetailPage/CommitCoverage/routes/FilesChangedTab/FilesChangedTable/FilesChangedTable.test.tsx

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,8 @@ describe('FilesChangedTable', () => {
134134
const { queryClient } = setup(mockData)
135135
render(<FilesChangedTable />, { wrapper: wrapper(queryClient) })
136136

137-
const link = await screen.findByRole('link', {
138-
name: 'src/index2.py',
139-
})
140-
expect(link).toBeInTheDocument()
141-
expect(link).toHaveAttribute(
142-
'href',
143-
'/gh/vax/keyleth/commit/123/blob/src/index2.py'
144-
)
137+
const text = await screen.findByText('src/index2.py')
138+
expect(text).toBeInTheDocument()
145139
})
146140

147141
it('renders coverage', async () => {

‎src/pages/CommitDetailPage/CommitCoverage/routes/FilesChangedTab/FilesChangedTable/FilesChangedTable.tsx

Lines changed: 52 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import { useLocation, useParams } from 'react-router-dom'
1919

2020
import { ImpactedFileType, useCommit } from 'services/commit'
2121
import { OrderingDirection, OrderingParameter } from 'services/pull/usePull'
22-
import A from 'ui/A'
2322
import Icon from 'ui/Icon'
2423
import Spinner from 'ui/Spinner'
2524
import TotalsNumber from 'ui/TotalsNumber'
@@ -71,7 +70,7 @@ function getFilter(sorting: Array<{ id: string; desc: boolean }>) {
7170
return undefined
7271
}
7372

74-
function getColumns({ commitId }: { commitId: string }) {
73+
function getColumns() {
7574
return [
7675
columnHelper.accessor('headName', {
7776
id: 'name',
@@ -83,42 +82,14 @@ function getColumns({ commitId }: { commitId: string }) {
8382
return (
8483
<div className="flex flex-row items-center break-all">
8584
{!isDeletedFile ? (
86-
<span
87-
data-action="clickable"
88-
data-testid="file-diff-expand"
89-
className={cs(
90-
'inline-flex items-center gap-1 font-sans hover:underline focus:ring-2',
91-
{
92-
'text-ds-blue-default': row.getIsExpanded(),
93-
}
94-
)}
95-
{...{
96-
onClick: row.getToggleExpandedHandler(),
97-
}}
98-
>
99-
<Icon
100-
size="md"
101-
name={row.getIsExpanded() ? 'chevronDown' : 'chevronRight'}
102-
variant="solid"
103-
/>
104-
</span>
85+
<Icon
86+
size="md"
87+
name={row.getIsExpanded() ? 'chevronDown' : 'chevronRight'}
88+
variant="solid"
89+
className="flex-none"
90+
/>
10591
) : null}
106-
{isDeletedFile ? (
107-
<>{headName}</>
108-
) : (
109-
/* @ts-expect-error - A hasn't been typed yet */
110-
<A
111-
to={{
112-
pageName: 'commitFileDiff',
113-
options: {
114-
commit: commitId,
115-
tree: headName,
116-
},
117-
}}
118-
>
119-
{headName}
120-
</A>
121-
)}
92+
<span>{headName}</span>
12293
{row.original?.isCriticalFile ? (
12394
<span className="ml-2 h-fit flex-none rounded border border-ds-gray-tertiary p-1 text-xs text-ds-gray-senary">
12495
Critical file
@@ -286,7 +257,7 @@ export default function FilesChangedTable() {
286257
}, [commit?.compareWithParent])
287258

288259
const table = useReactTable({
289-
columns: getColumns({ commitId: commitSha }),
260+
columns: getColumns(),
290261
data: filesChanged,
291262
state: {
292263
expanded,
@@ -297,7 +268,7 @@ export default function FilesChangedTable() {
297268
getCoreRowModel: getCoreRowModel(),
298269
getSortedRowModel: getSortedRowModel(),
299270
getExpandedRowModel: getExpandedRowModel(),
300-
getRowCanExpand: () => true,
271+
getRowCanExpand: (row) => row.original?.headCoverage !== null, // deleted files are not expandable
301272
})
302273

303274
if (isLoading || commit?.state === 'pending') {
@@ -321,7 +292,7 @@ export default function FilesChangedTable() {
321292
}
322293

323294
return (
324-
<div className="filelistui" data-highlight-row="onHover">
295+
<div className="filelistui">
325296
<div>
326297
{table.getHeaderGroups().map((headerGroup) => (
327298
<div key={headerGroup.id} className="filelistui-thead">
@@ -364,33 +335,47 @@ export default function FilesChangedTable() {
364335
})}
365336
</div>
366337
))}
367-
{table.getRowModel().rows.map((row, i) => (
368-
<Fragment key={i}>
369-
<div className="filelistui-row">
370-
{row.getVisibleCells().map((cell) => {
371-
return (
372-
<div
373-
key={cell.id}
374-
{...(isNumericValue(cell.column.id)
375-
? {
376-
'data-type': 'numeric',
377-
}
378-
: {})}
379-
className={cs({
380-
'w-6/12': cell.column.id === 'name',
381-
'w-2/12 justify-end flex': cell.column.id !== 'name',
382-
})}
383-
>
384-
{flexRender(cell.column.columnDef.cell, cell.getContext())}
385-
</div>
386-
)
387-
})}
388-
</div>
389-
<div data-expanded={row.getIsExpanded()}>
390-
{row.getIsExpanded() ? <RenderSubComponent row={row} /> : null}
391-
</div>
392-
</Fragment>
393-
))}
338+
{table.getRowModel().rows.map((row, i) => {
339+
const isDeletedFile = row.original?.headCoverage === null
340+
return (
341+
<Fragment key={i}>
342+
<div
343+
className={cs('filelistui-row', {
344+
'cursor-pointer': !isDeletedFile,
345+
'cursor-default': isDeletedFile,
346+
})}
347+
data-testid="file-diff-expand"
348+
onClick={() => !isDeletedFile && row.toggleExpanded()}
349+
data-highlight-row={!isDeletedFile ? 'onHover' : undefined}
350+
>
351+
{row.getVisibleCells().map((cell) => {
352+
return (
353+
<div
354+
key={cell.id}
355+
{...(isNumericValue(cell.column.id)
356+
? {
357+
'data-type': 'numeric',
358+
}
359+
: {})}
360+
className={cs({
361+
'w-6/12': cell.column.id === 'name',
362+
'w-2/12 justify-end flex': cell.column.id !== 'name',
363+
})}
364+
>
365+
{flexRender(
366+
cell.column.columnDef.cell,
367+
cell.getContext()
368+
)}
369+
</div>
370+
)
371+
})}
372+
</div>
373+
<div data-expanded={row.getIsExpanded()}>
374+
{row.getIsExpanded() ? <RenderSubComponent row={row} /> : null}
375+
</div>
376+
</Fragment>
377+
)
378+
})}
394379
</div>
395380
</div>
396381
)

‎src/pages/CommitDetailPage/CommitCoverage/routes/IndirectChangesTab/IndirectChangesTable/IndirectChangesTable.test.tsx

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -123,14 +123,8 @@ describe('IndirectChangesTable', () => {
123123
const { queryClient } = setup(mockData)
124124
render(<IndirectChangesTable />, { wrapper: wrapper(queryClient) })
125125

126-
const link = await screen.findByRole('link', {
127-
name: 'src/index2.py',
128-
})
126+
const link = await screen.findByText('src/index2.py')
129127
expect(link).toBeInTheDocument()
130-
expect(link).toHaveAttribute(
131-
'href',
132-
'/gh/codecov/cool-repo/commit/123/blob/src/index2.py'
133-
)
134128
})
135129

136130
it('renders coverage', async () => {

‎src/pages/CommitDetailPage/CommitCoverage/routes/IndirectChangesTab/IndirectChangesTable/IndirectChangesTable.tsx

Lines changed: 54 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import { useLocation, useParams } from 'react-router-dom'
1717

1818
import ToggleHeader from 'pages/CommitDetailPage/Header/ToggleHeader/ToggleHeader'
1919
import { ImpactedFileType, useCommit } from 'services/commit/useCommit'
20-
import A from 'ui/A'
2120
import Icon from 'ui/Icon'
2221
import Spinner from 'ui/Spinner'
2322
import TotalsNumber from 'ui/TotalsNumber'
@@ -28,44 +27,26 @@ const columnHelper = createColumnHelper<ImpactedFileType>()
2827

2928
const isNumericValue = (value: string) => value === 'head' || value === 'change'
3029

31-
function getColumns({ commitid }: { commitid: string }) {
30+
function getColumns() {
3231
return [
3332
columnHelper.accessor('headName', {
3433
id: 'name',
3534
header: 'Name',
3635
cell: ({ getValue, row }) => {
3736
const headName = getValue()
37+
const isDeletedFile = row.original?.headCoverage === null
3838

3939
return (
40-
<div
41-
className="flex cursor-pointer items-center gap-2"
42-
data-testid="file-diff-expand"
43-
onClick={() => row.toggleExpanded()}
44-
>
45-
<span
46-
className={cs({
47-
'text-ds-blue-darker': row.getIsExpanded(),
48-
'text-current': !row.getIsExpanded(),
49-
})}
50-
>
40+
<div className="flex items-center gap-2">
41+
{!isDeletedFile ? (
5142
<Icon
5243
size="md"
5344
name={row.getIsExpanded() ? 'chevronDown' : 'chevronRight'}
5445
variant="solid"
46+
className="flex-none"
5547
/>
56-
</span>
57-
<div className="flex flex-col break-all">
58-
<A
59-
hook="commit-file-diff"
60-
isExternal={false}
61-
to={{
62-
pageName: 'commitFileDiff',
63-
options: { commit: commitid, tree: headName },
64-
}}
65-
>
66-
{headName}
67-
</A>
68-
</div>
48+
) : null}
49+
<span>{headName}</span>
6950
{row.original?.isCriticalFile ? (
7051
<span className="flex-none self-center rounded border border-ds-gray-tertiary p-1 text-xs text-ds-gray-senary">
7152
Critical file
@@ -183,14 +164,15 @@ export default function FilesChangedTable() {
183164
}, [commitData?.commit?.compareWithParent])
184165

185166
const table = useReactTable({
186-
columns: getColumns({ commitid: commitSha }),
167+
columns: getColumns(),
187168
data,
188169
state: {
189170
expanded,
190171
},
191172
onExpandedChange: setExpanded,
192173
getCoreRowModel: getCoreRowModel(),
193174
getExpandedRowModel: getExpandedRowModel(),
175+
getRowCanExpand: (row) => row.original?.headCoverage !== null, // deleted files are not expandable
194176
})
195177

196178
if (commitData?.commit?.state === 'pending') {
@@ -223,7 +205,7 @@ export default function FilesChangedTable() {
223205
return (
224206
<>
225207
<ToggleHeader />
226-
<div className="filelistui" data-highlight-row="onHover">
208+
<div className="filelistui">
227209
<div>
228210
{table.getHeaderGroups().map((headerGroup) => (
229211
<div key={headerGroup.id} className="filelistui-thead">
@@ -250,39 +232,50 @@ export default function FilesChangedTable() {
250232
{isLoading ? (
251233
<Loader />
252234
) : (
253-
table.getRowModel().rows.map((row, i) => (
254-
<Fragment key={i}>
255-
<div className="filelistui-row">
256-
{row.getVisibleCells().map((cell) => {
257-
return (
258-
<div
259-
key={cell.id}
260-
{...(isNumericValue(cell.column.id)
261-
? {
262-
'data-type': 'numeric',
263-
}
264-
: {})}
265-
className={cs({
266-
'w-8/12': cell.column.id === 'name',
267-
'flex justify-end': cell.column.id !== 'name',
268-
'w-3/12': cell.column.id === 'change',
269-
})}
270-
>
271-
{flexRender(
272-
cell.column.columnDef.cell,
273-
cell.getContext()
274-
)}
275-
</div>
276-
)
277-
})}
278-
</div>
279-
<div data-expanded={row.getIsExpanded()}>
280-
{row.getIsExpanded() ? (
281-
<RenderSubComponent row={row} />
282-
) : null}
283-
</div>
284-
</Fragment>
285-
))
235+
table.getRowModel().rows.map((row, i) => {
236+
const isDeletedFile = row.original?.headCoverage === null
237+
return (
238+
<Fragment key={i}>
239+
<div
240+
className={cs('filelistui-row', {
241+
'cursor-pointer': !isDeletedFile,
242+
'cursor-default': isDeletedFile,
243+
})}
244+
data-testid="file-diff-expand"
245+
onClick={() => !isDeletedFile && row.toggleExpanded()}
246+
data-highlight-row={!isDeletedFile ? 'onHover' : undefined}
247+
>
248+
{row.getVisibleCells().map((cell) => {
249+
return (
250+
<div
251+
key={cell.id}
252+
{...(isNumericValue(cell.column.id)
253+
? {
254+
'data-type': 'numeric',
255+
}
256+
: {})}
257+
className={cs({
258+
'w-8/12': cell.column.id === 'name',
259+
'flex justify-end': cell.column.id !== 'name',
260+
'w-3/12': cell.column.id === 'change',
261+
})}
262+
>
263+
{flexRender(
264+
cell.column.columnDef.cell,
265+
cell.getContext()
266+
)}
267+
</div>
268+
)
269+
})}
270+
</div>
271+
<div data-expanded={row.getIsExpanded()}>
272+
{row.getIsExpanded() ? (
273+
<RenderSubComponent row={row} />
274+
) : null}
275+
</div>
276+
</Fragment>
277+
)
278+
})
286279
)}
287280
</div>
288281
</div>

‎src/pages/PullRequestPage/PullCoverage/routes/FilesChangedTab/FilesChanged/FilesChangedTable/FilesChangedTable.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ describe('FilesChangedTable', () => {
254254
const { queryClient } = setup()
255255
render(<FilesChangedTable />, { wrapper: wrapper(queryClient) })
256256

257-
const path = await screen.findByRole('link', { name: 'flag1/mafs.js' })
257+
const path = await screen.findByText('flag1/mafs.js')
258258
expect(path).toBeInTheDocument()
259259
})
260260

0 commit comments

Comments
 (0)