Skip to content

Commit af32496

Browse files
ergunshDevtools-frontend LUCI CQ
authored andcommitted
[PatchWidget] Render CombinedDiffView when there is a patchSuggestion
Drive-by: * Splitted rendering of the view into different functions. (Complexity for only 1 template was increasing) Bug: 393265936 Change-Id: I0029168e96259ed231be868b3c4ee340400f6480 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6324572 Commit-Queue: Ergün Erdoğmuş <[email protected]> Reviewed-by: Alex Rudenko <[email protected]>
1 parent 527f5c7 commit af32496

File tree

7 files changed

+172
-62
lines changed

7 files changed

+172
-62
lines changed

front_end/models/workspace_diff/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ devtools_entrypoint("bundle") {
2929

3030
visibility = [
3131
":*",
32+
"../../panels/ai_assistance/*",
3233
"../../panels/changes/*",
3334
"../../panels/elements/*",
3435
"../../panels/sources/*",

front_end/panels/ai_assistance/BUILD.gn

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ devtools_module("ai_assistance") {
5454
"../../models/text_utils:bundle",
5555
"../../models/trace:bundle",
5656
"../../models/workspace:bundle",
57+
"../../models/workspace_diff:bundle",
58+
"../../panels/changes:bundle",
5759
"../../panels/elements:bundle",
5860
"../../panels/network:bundle",
5961
"../../panels/sources:bundle",

front_end/panels/ai_assistance/PatchWidget.ts

Lines changed: 141 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@ import * as i18n from '../../core/i18n/i18n.js';
1212
import * as Root from '../../core/root/root.js';
1313
import * as Persistence from '../../models/persistence/persistence.js';
1414
import * as Workspace from '../../models/workspace/workspace.js';
15+
import * as WorkspaceDiff from '../../models/workspace_diff/workspace_diff.js';
1516
import * as Buttons from '../../ui/components/buttons/buttons.js';
1617
import * as UI from '../../ui/legacy/legacy.js';
17-
import {html, nothing, render} from '../../ui/lit/lit.js';
18+
import {html, type LitTemplate, nothing, render} from '../../ui/lit/lit.js';
19+
import * as VisualLogging from '../../ui/visual_logging/visual_logging.js';
20+
import * as ChangesPanel from '../changes/changes.js';
1821

1922
import {type ResponseData, ResponseType} from './agents/AiAgent.js';
2023
import {PatchAgent} from './agents/PatchAgent.js';
@@ -39,6 +42,14 @@ const UIStringsNotTranslate = {
3942
*@description Button text to change the selected workspace
4043
*/
4144
change: 'Change',
45+
/*
46+
*@description Button text to discard the suggested changes and not save them to file system
47+
*/
48+
discard: 'Discard',
49+
/*
50+
*@description Button text to save all the suggested changes to file system
51+
*/
52+
saveAll: 'Save all',
4253
/**
4354
*@description Button text while data is being loaded
4455
*/
@@ -47,6 +58,10 @@ const UIStringsNotTranslate = {
4758
*@description Label for the selected workspace/folder
4859
*/
4960
selectedFolder: 'Selected folder:',
61+
/**
62+
*@description Disclaimer text shown for using code snippets with caution
63+
*/
64+
codeDisclaimer: 'Use code snippets with caution',
5065
/**
5166
*@description Tooltip text for the info icon beside the "Apply to workspace" button
5267
*/
@@ -56,11 +71,14 @@ const UIStringsNotTranslate = {
5671
const lockedString = i18n.i18n.lockedString;
5772

5873
export interface ViewInput {
74+
workspaceDiff: WorkspaceDiff.WorkspaceDiff.WorkspaceDiffImpl;
5975
changeSummary?: string;
6076
patchSuggestion?: string;
6177
patchSuggestionLoading?: boolean;
6278
projectName?: string;
6379
onApplyToWorkspace?: () => void;
80+
onDiscard: () => void;
81+
onSaveAll: () => void;
6482
}
6583

6684
type View = (input: ViewInput, output: undefined, target: HTMLElement) => void;
@@ -79,6 +97,7 @@ export class PatchWidget extends UI.Widget.Widget {
7997
#project?: Workspace.Workspace.Project;
8098
#patchSuggestion?: string;
8199
#patchSuggestionLoading?: boolean;
100+
#workspaceDiff = WorkspaceDiff.WorkspaceDiff.workspaceDiff();
82101
#workspace = Workspace.Workspace.WorkspaceImpl.instance();
83102

84103
constructor(element?: HTMLElement, view?: View, opts?: {
@@ -92,68 +111,142 @@ export class PatchWidget extends UI.Widget.Widget {
92111
return;
93112
}
94113

114+
function renderHeader(): LitTemplate {
115+
if (input.patchSuggestionLoading) {
116+
return html`
117+
<devtools-spinner></devtools-spinner>
118+
<span class="header-text">
119+
${lockedString(UIStringsNotTranslate.loadingPatchSuggestion)}
120+
</span>
121+
`;
122+
}
123+
124+
if (input.patchSuggestion) {
125+
return html`
126+
<devtools-icon class="difference-icon" .name=${'difference'}></devtools-icon>
127+
<span class="header-text">
128+
${lockedString(`File changes in ${input.projectName}`)}
129+
</span>
130+
`;
131+
}
132+
133+
return html`
134+
<devtools-icon class="difference-icon" .name=${'pen-spark'}></devtools-icon>
135+
<span class="header-text">
136+
${lockedString(UIStringsNotTranslate.changeSummary)}
137+
</span>
138+
`;
139+
}
140+
141+
function renderContent(): LitTemplate {
142+
if (!input.changeSummary) {
143+
return nothing;
144+
}
145+
146+
if (input.patchSuggestion) {
147+
return html`<devtools-widget .widgetConfig=${UI.Widget.widgetConfig(ChangesPanel.CombinedDiffView.CombinedDiffView, {
148+
workspaceDiff: input.workspaceDiff,
149+
})}></devtools-widget>`;
150+
}
151+
152+
return html`<devtools-code-block
153+
.code=${input.changeSummary}
154+
.codeLang=${'css'}
155+
.displayNotice=${true}
156+
></devtools-code-block>`;
157+
}
158+
159+
function renderFooter(): LitTemplate {
160+
if (input.patchSuggestionLoading) {
161+
return nothing;
162+
}
163+
164+
if (input.patchSuggestion) {
165+
return html`
166+
<div class="footer">
167+
<x-link class="link disclaimer-link" href="https://support.google.com/legal/answer/13505487" jslog=${
168+
VisualLogging.link('code-disclaimer').track({
169+
click: true,
170+
})}>
171+
${lockedString(UIStringsNotTranslate.codeDisclaimer)}
172+
</x-link>
173+
<div class="save-or-discard-buttons">
174+
<devtools-button
175+
@click=${input.onDiscard}
176+
.jslogContext=${'discard'}
177+
.variant=${Buttons.Button.Variant.OUTLINED}>
178+
${lockedString(UIStringsNotTranslate.discard)}
179+
</devtools-button>
180+
<devtools-button
181+
@click=${input.onSaveAll}
182+
.jslogContext=${'save-all'}
183+
.variant=${Buttons.Button.Variant.PRIMARY}>
184+
${lockedString(UIStringsNotTranslate.saveAll)}
185+
</devtools-button>
186+
</div>
187+
</div>
188+
`;
189+
}
190+
191+
return html`
192+
<div class="footer">
193+
<div class="change-workspace">
194+
<div class="selected-folder">
195+
${lockedString(UIStringsNotTranslate.selectedFolder)} ${input.projectName}
196+
</div>
197+
<devtools-button
198+
@click=${onChangeWorkspaceClick}
199+
.jslogContext=${'change-workspace'}
200+
.variant=${Buttons.Button.Variant.TEXT}>
201+
${lockedString(UIStringsNotTranslate.change)}
202+
</devtools-button>
203+
</div>
204+
<div class="apply-to-workspace-container">
205+
<devtools-button
206+
class="apply-to-workspace"
207+
@click=${input.onApplyToWorkspace}
208+
.jslogContext=${'stage-to-workspace'}
209+
.variant=${Buttons.Button.Variant.OUTLINED}>
210+
${lockedString(UIStringsNotTranslate.applyToWorkspace)}
211+
</devtools-button>
212+
<devtools-icon aria-describedby="info-tooltip" .name=${'info'}></devtools-icon>
213+
<devtools-tooltip id="info-tooltip">${lockedString(UIStringsNotTranslate.applyToWorkspaceTooltip)}</devtools-tooltip>
214+
</div>
215+
</div>`;
216+
}
217+
95218
render(
96219
html`
97220
<details class="change-summary">
98221
<summary>
99-
${input.patchSuggestionLoading ? html`<devtools-spinner></devtools-spinner>` : html`<devtools-icon class="difference-icon" .name=${'pen-spark'}></devtools-icon>`}
100-
<span class="header-text">
101-
${input.patchSuggestionLoading ? lockedString(UIStringsNotTranslate.loadingPatchSuggestion) : lockedString(UIStringsNotTranslate.changeSummary)}
102-
</span>
222+
${renderHeader()}
103223
<devtools-icon
104224
class="arrow"
105225
.name=${'chevron-down'}
106226
></devtools-icon>
107227
</summary>
108-
<devtools-code-block
109-
.code=${input.changeSummary}
110-
.codeLang=${'css'}
111-
.displayNotice=${true}
112-
></devtools-code-block>
113-
${!input.patchSuggestionLoading ? html`<div class="workspace">
114-
<div class="change-workspace">
115-
<div class="selected-folder">
116-
${lockedString(UIStringsNotTranslate.selectedFolder)} ${input.projectName}
117-
</div>
118-
<devtools-button
119-
@click=${onChangeWorkspaceClick}
120-
.jslogContext=${'change-workspace'}
121-
.variant=${Buttons.Button.Variant.TEXT}>
122-
${lockedString(UIStringsNotTranslate.change)}
123-
</devtools-button>
124-
</div>
125-
<div class="apply-to-workspace-container">
126-
<devtools-button
127-
class="apply-to-workspace"
128-
@click=${input.onApplyToWorkspace}
129-
.jslogContext=${'stage-to-workspace'}
130-
.variant=${Buttons.Button.Variant.OUTLINED}>
131-
${lockedString(UIStringsNotTranslate.applyToWorkspace)}
132-
</devtools-button>
133-
<devtools-icon aria-describedby="info-tooltip" .name=${'info'}></devtools-icon>
134-
<devtools-tooltip id="info-tooltip">${lockedString(UIStringsNotTranslate.applyToWorkspaceTooltip)}</devtools-tooltip>
135-
</div>
136-
</div>` : nothing}
137-
${input.patchSuggestion ? html`<div class="patch-tmp-message">
138-
${input.patchSuggestion}
139-
</div>` : nothing}
228+
${renderContent()}
229+
${renderFooter()}
140230
</details>
141231
`,
142232
target,
143233
{host: target}
144234
);
145-
}) as View;
235+
});
146236
// clang-format on
147237
this.requestUpdate();
148238
}
149239

150240
override performUpdate(): void {
151241
const viewInput = {
242+
workspaceDiff: this.#workspaceDiff,
152243
changeSummary: this.changeSummary,
153244
patchSuggestion: this.#patchSuggestion,
154245
patchSuggestionLoading: this.#patchSuggestionLoading,
155246
projectName: this.#project?.displayName(),
156247
onApplyToWorkspace: this.#onApplyToWorkspace.bind(this),
248+
onDiscard: this.#onDiscard.bind(this),
249+
onSaveAll: this.#onSaveAll.bind(this),
157250
};
158251
this.#view(viewInput, undefined, this.contentElement);
159252
}
@@ -226,6 +319,16 @@ export class PatchWidget extends UI.Widget.Widget {
226319
this.requestUpdate();
227320
}
228321

322+
#onDiscard(): void {
323+
// TODO: Remove changes from the working copies as well.
324+
this.#patchSuggestion = undefined;
325+
this.requestUpdate();
326+
}
327+
328+
#onSaveAll(): void {
329+
// TODO: Handle saving all the files.
330+
}
331+
229332
async #applyPatch(changeSummary: string): Promise<ResponseData|undefined> {
230333
if (!this.#project) {
231334
throw new Error('Project does not exist');

front_end/panels/ai_assistance/components/chatView.css

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -831,37 +831,37 @@ main {
831831
--code-block-background-color: var(--sys-color-surface1);
832832
}
833833

834-
.workspace {
834+
.footer {
835835
display: flex;
836836
flex-direction: row;
837837
justify-content: space-between;
838-
margin: var(--sys-size-5);
839-
}
840-
841-
.selected-folder {
842-
margin: var(--sys-size-3) var(--sys-size-3) 0 0;
843-
}
838+
margin: var(--sys-size-5) 0;
844839

845-
.change-workspace {
846-
display: flex;
847-
flex-direction: row;
848-
}
840+
.disclaimer-link {
841+
align-self: center;
842+
}
849843

850-
.apply-to-workspace-container {
851-
display: flex;
852-
align-items: center;
853-
gap: var(--sys-size-3);
844+
.selected-folder {
845+
margin: var(--sys-size-3) var(--sys-size-3) 0 0;
846+
}
854847

855-
devtools-icon {
856-
/* var(--sys-size-8) is too small and var(--sys-size-9) is too big. */
857-
width: 18px;
858-
height: 18px;
859-
margin-left: var(--sys-size-2);
848+
.change-workspace {
849+
display: flex;
850+
flex-direction: row;
860851
}
861-
}
862852

863-
.patch-tmp-message {
864-
padding: var(--sys-size-4);
853+
.apply-to-workspace-container {
854+
display: flex;
855+
align-items: center;
856+
gap: var(--sys-size-3);
857+
858+
devtools-icon {
859+
/* var(--sys-size-8) is too small and var(--sys-size-9) is too big. */
860+
width: 18px;
861+
height: 18px;
862+
margin-left: var(--sys-size-2);
863+
}
864+
}
865865
}
866866
}
867867

front_end/panels/changes/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ devtools_entrypoint("bundle") {
4949
visibility = [
5050
":*",
5151
"../../entrypoints/*",
52+
"../../panels/ai_assistance/*",
5253
]
5354

5455
visibility += devtools_panels_visibility

front_end/panels/changes/changesView.css

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@
2020
& .widget:first-child {
2121
height: 100%;
2222
}
23+
24+
.combined-diff-view {
25+
padding: var(--sys-size-6);
26+
}
2327
}
2428

2529
:focus.selected {

front_end/panels/changes/combinedDiffView.css

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
*/
66

77
.combined-diff-view {
8-
padding: var(--sys-size-6);
98
display: flex;
109
flex-direction: column;
1110
gap: var(--sys-size-5);

0 commit comments

Comments
 (0)