Skip to content

Commit 8241e0c

Browse files
robertpiosikrobertpiosik
authored
Introduce review changes feature (#396)
* Enable interactive review of chat-generated code changes before application * Integrate user review of patch changes before applying them * Present a single diff review for all patches before applying changes * Migrate the file change review process to a custom in-webview UI * Enable granular file selection for change review acceptance * Extract the file review functionality into a dedicated component * Streamline file review decisions by consolidating acceptance and navigation actions into a single, structured message type * Adjust padding in UI components and refactor IconButton import * Introduce a UI component to review and manage proposed file changes
1 parent 8380346 commit 8241e0c

23 files changed

Lines changed: 772 additions & 211 deletions

File tree

‎packages/shared/src/types/file-in-review.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
export type FileInReview = {
2+
file_path: string
3+
workspace_name?: string
4+
is_new?: boolean
5+
}

‎packages/ui/src/components/editor/Checkbox/Checkbox.module.scss‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
outline: none;
1717
}
1818

19+
&:disabled {
20+
opacity: var(--disabled-opacity);
21+
cursor: default;
22+
}
23+
1924
&:checked {
2025
border-color: var(--vscode-inputOption-activeBorder);
2126
color: var(--vscode-inputOption-activeForeground);

‎packages/ui/src/components/editor/Configurations/Configurations.module.scss‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,8 @@
2525
justify-content: space-between;
2626
align-items: center;
2727
gap: var(--padding-4px);
28-
height: 22px;
28+
height: var(--list-item-height);
2929
padding: 0 var(--padding-12px);
30-
cursor: pointer;
3130

3231
&:hover {
3332
background-color: var(--vscode-list-hoverBackground);

‎packages/ui/src/components/editor/Configurations/Configurations.tsx‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
import styles from './Configurations.module.scss'
2-
import { Button } from '../Button/Button'
32
import { DEFAULT_TEMPERATURE } from '@shared/constants/api-tools'
43
import cn from 'classnames'
54
import { useState } from 'react'
6-
import { TextButton } from '../TextButton'
75
import { IconButton } from '../IconButton/IconButton'
86

97
export namespace Configurations {

‎packages/ui/src/components/editor/IconButton/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export * from './IconButton'

‎packages/ui/src/components/editor/Page/Page.module.scss‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,3 @@
3131
}
3232
}
3333
}
34-
35-
.content__inner {
36-
padding: var(--padding-12px);
37-
}

‎packages/ui/src/components/editor/Page/Page.tsx‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import styles from './Page.module.scss'
2-
import { IconButton } from '../IconButton/IconButton'
32
import { Scrollable } from '../Scrollable'
3+
import { IconButton } from '../IconButton'
44

55
type Props = {
66
title?: string
@@ -17,13 +17,15 @@ export const Page: React.FC<Props> = (props) => {
1717
{props.title && (
1818
<div className={styles.header__title}>{props.title}</div>
1919
)}
20-
{props.on_back_click && (
21-
<IconButton
22-
codicon_icon="chevron-left"
23-
on_click={props.on_back_click}
24-
title="Return to previous screen"
25-
/>
26-
)}
20+
<div>
21+
{props.on_back_click && (
22+
<IconButton
23+
codicon_icon="chevron-left"
24+
on_click={props.on_back_click}
25+
title="Return to previous screen"
26+
/>
27+
)}
28+
</div>
2729
<div className={styles.header__right}>
2830
<div className={styles.header__right__slot}>{props.header_slot}</div>
2931
{props.on_close_click && (
@@ -35,9 +37,7 @@ export const Page: React.FC<Props> = (props) => {
3537
)}
3638
</div>
3739
</div>
38-
<Scrollable>
39-
<div className={styles.content__inner}>{props.children}</div>
40-
</Scrollable>
40+
<Scrollable>{props.children}</Scrollable>
4141
</div>
4242
)
4343
}

‎packages/ui/src/components/editor/Presets/Presets.module.scss‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@
2626
align-items: center;
2727
gap: var(--padding-4px);
2828
padding: 0 var(--padding-12px);
29-
cursor: pointer;
30-
height: 22px;
29+
height: var(--list-item-height);
3130

3231
&:hover {
3332
background-color: var(--vscode-list-hoverBackground);
@@ -56,13 +55,17 @@
5655
&__left {
5756
display: flex;
5857
align-items: center;
58+
height: 100%;
5959
gap: var(--padding-5px);
6060
overflow: hidden;
6161
text-overflow: ellipsis;
6262
white-space: nowrap;
6363
position: relative;
6464

6565
&__drag_handle {
66+
display: flex;
67+
align-items: center;
68+
height: 100%;
6669
cursor: grab;
6770
opacity: 0.5;
6871

‎packages/ui/src/components/editor/ReviewChanges/ReviewChanges.module.scss‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
.container {
2+
display: flex;
3+
flex-direction: column;
4+
gap: var(--padding-8px);
5+
}
6+
7+
.list {
8+
padding: 0 var(--spacing-s);
9+
}
10+
11+
.item {
12+
display: flex;
13+
align-items: center;
14+
gap: var(--padding-6px);
15+
padding: 0 var(--padding-12px);
16+
height: var(--list-item-height);
17+
&:hover {
18+
background-color: var(--vscode-list-hoverBackground);
19+
}
20+
21+
&--selected {
22+
background-color: var(--vscode-list-inactiveSelectionBackground);
23+
24+
&:hover {
25+
background-color: var(--vscode-list-inactiveSelectionBackground);
26+
}
27+
}
28+
29+
&__label {
30+
text-overflow: ellipsis;
31+
overflow: hidden;
32+
white-space: nowrap;
33+
34+
> span:last-child {
35+
opacity: 0.7;
36+
margin-left: 0.5em;
37+
font-size: 0.9em;
38+
}
39+
}
40+
}
41+
42+
.footer {
43+
display: flex;
44+
gap: var(--padding-8px);
45+
padding: var(--padding-4px) var(--padding-12px) var(--padding-12px)
46+
var(--padding-12px);
47+
}

‎packages/ui/src/components/editor/ReviewChanges/ReviewChanges.tsx‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
import { FC, useState, useEffect } from 'react'
2+
import { FileInReview } from '@shared/types/file-in-review'
3+
import cn from 'classnames'
4+
import styles from './ReviewChanges.module.scss'
5+
import { Button } from '../Button'
6+
import { Checkbox } from '../Checkbox'
7+
8+
type CheckedFileToReview = FileInReview & { is_checked: boolean }
9+
10+
type Props = {
11+
files: FileInReview[]
12+
on_reject: () => void
13+
on_accept: (files: FileInReview[]) => void
14+
on_focus_file: (file: { file_path: string; workspace_name?: string }) => void
15+
}
16+
17+
export const ReviewChanges: FC<Props> = ({
18+
files,
19+
on_reject,
20+
on_accept,
21+
on_focus_file
22+
}) => {
23+
const [files_to_review, set_files_to_review] = useState<
24+
CheckedFileToReview[]
25+
>([])
26+
const [last_clicked_file_index, set_last_clicked_file_index] = useState(0)
27+
28+
useEffect(() => {
29+
set_files_to_review(files.map((f) => ({ ...f, is_checked: true })))
30+
set_last_clicked_file_index(0)
31+
}, [files])
32+
33+
const handle_accept = () => {
34+
const accepted_files = files_to_review.filter((f) => f.is_checked)
35+
on_accept(accepted_files)
36+
}
37+
38+
return (
39+
<div className={styles.container}>
40+
<div className={styles.list}>
41+
{files_to_review.map((file, index) => {
42+
const last_slash_index = file.file_path.lastIndexOf('/')
43+
const file_name = file.file_path.substring(last_slash_index + 1)
44+
const dir_path =
45+
last_slash_index > -1
46+
? file.file_path.substring(0, last_slash_index)
47+
: ''
48+
return (
49+
<div
50+
key={`${file.workspace_name ?? ''}:${file.file_path}`}
51+
className={cn(styles.item, {
52+
[styles['item--selected']]: index == last_clicked_file_index
53+
})}
54+
role="button"
55+
>
56+
<Checkbox
57+
checked={file.is_checked}
58+
on_change={(checked) => {
59+
set_files_to_review((prev) =>
60+
prev.map((f, i) =>
61+
i == index ? { ...f, is_checked: checked } : f
62+
)
63+
)
64+
}}
65+
disabled={files_to_review.length == 1}
66+
/>
67+
<div
68+
className={styles['item__label']}
69+
onClick={() => {
70+
set_last_clicked_file_index(index)
71+
on_focus_file({
72+
file_path: file.file_path,
73+
workspace_name: file.workspace_name
74+
})
75+
}}
76+
title={file.file_path}
77+
>
78+
<span>
79+
{file.is_new ? 'New: ' : ''} {file_name}
80+
</span>
81+
<span>
82+
{file.workspace_name ? `${file.workspace_name}/` : ''}
83+
{dir_path}
84+
</span>
85+
</div>
86+
</div>
87+
)
88+
})}
89+
</div>
90+
<div className={styles.footer}>
91+
<Button
92+
on_click={handle_accept}
93+
disabled={files_to_review.filter((f) => f.is_checked).length == 0}
94+
>{`Accept ${
95+
files_to_review.length > 1 ? 'Selected Edits' : 'Edit'
96+
}`}</Button>
97+
</div>
98+
</div>
99+
)
100+
}

0 commit comments

Comments
 (0)