Skip to content

Commit

Permalink
[Changes] Add CombinedDiffView and render it when patching enabled
Browse files Browse the repository at this point in the history
Bug: 393265936
Change-Id: Ia0ff8777371da43d4d8f13a617ac391c6d7fa438
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6291327
Reviewed-by: Alex Rudenko <[email protected]>
Commit-Queue: Ergün Erdoğmuş <[email protected]>
  • Loading branch information
ergunsh authored and Devtools-frontend LUCI CQ committed Feb 21, 2025
1 parent b0e0d4c commit 6ec9229
Show file tree
Hide file tree
Showing 8 changed files with 401 additions and 16 deletions.
2 changes: 2 additions & 0 deletions config/gni/devtools_grd_files.gni
Original file line number Diff line number Diff line change
Expand Up @@ -1343,8 +1343,10 @@ grd_files_debug_sources = [
"front_end/panels/browser_debugger/xhrBreakpointsSidebarPane.css.js",
"front_end/panels/changes/ChangesSidebar.js",
"front_end/panels/changes/ChangesView.js",
"front_end/panels/changes/CombinedDiffView.js",
"front_end/panels/changes/changesSidebar.css.js",
"front_end/panels/changes/changesView.css.js",
"front_end/panels/changes/combinedDiffView.css.js",
"front_end/panels/console/ConsoleContextSelector.js",
"front_end/panels/console/ConsoleFilter.js",
"front_end/panels/console/ConsoleFormat.js",
Expand Down
9 changes: 8 additions & 1 deletion front_end/panels/changes/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ generate_css("css_files") {
sources = [
"changesSidebar.css",
"changesView.css",
"combinedDiffView.css",
]
}

devtools_module("changes") {
sources = [
"ChangesSidebar.ts",
"ChangesView.ts",
"CombinedDiffView.ts",
]

deps = [
Expand Down Expand Up @@ -67,10 +69,15 @@ devtools_entrypoint("meta") {
ts_library("unittests") {
testonly = true

sources = [ "changes.test.ts" ]
sources = [
"CombinedDiffView.test.ts",
"changes.test.ts",
]

deps = [
":bundle",
"../../models/workspace:bundle",
"../../models/workspace_diff:bundle",
"../../testing",
]
}
50 changes: 35 additions & 15 deletions front_end/panels/changes/ChangesView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as Common from '../../core/common/common.js';
import * as Host from '../../core/host/host.js';
import * as i18n from '../../core/i18n/i18n.js';
import type * as Platform from '../../core/platform/platform.js';
import * as Root from '../../core/root/root.js';
import type * as Formatter from '../../models/formatter/formatter.js';
import type * as Workspace from '../../models/workspace/workspace.js';
import * as WorkspaceDiff from '../../models/workspace_diff/workspace_diff.js';
Expand All @@ -19,6 +20,7 @@ import * as VisualLogging from '../../ui/visual_logging/visual_logging.js';

import {ChangesSidebar, Events} from './ChangesSidebar.js';
import changesViewStyles from './changesView.css.js';
import * as CombinedDiffView from './CombinedDiffView.js';

const CHANGES_VIEW_URL = 'https://developer.chrome.com/docs/devtools/changes' as Platform.DevToolsPath.UrlString;

Expand Down Expand Up @@ -79,8 +81,9 @@ export class ChangesView extends UI.Widget.VBox {
#learnMoreLinkElement?: HTMLElement;
private readonly diffContainer: HTMLElement;
private readonly toolbar: UI.Toolbar.Toolbar;
private readonly diffStats: UI.Toolbar.ToolbarText;
private readonly diffView: DiffView.DiffView.DiffView;
private readonly diffStats?: UI.Toolbar.ToolbarText;
private readonly diffView?: DiffView.DiffView.DiffView;
private readonly combinedDiffView?: CombinedDiffView.CombinedDiffView;

constructor() {
super(true);
Expand All @@ -98,6 +101,7 @@ export class ChangesView extends UI.Widget.VBox {

this.workspaceDiff = WorkspaceDiff.WorkspaceDiff.workspaceDiff();
this.changesSidebar = new ChangesSidebar(this.workspaceDiff);
// TODO(ergunsh): Add scroll to singular diffs when they are clicked on sidebar.
this.changesSidebar.addEventListener(
Events.SELECTED_UI_SOURCE_CODE_CHANGED, this.selectedUISourceCodeChanged, this);
splitWidget.setSidebarWidget(this.changesSidebar);
Expand All @@ -106,20 +110,29 @@ export class ChangesView extends UI.Widget.VBox {

this.diffContainer = mainWidget.element.createChild('div', 'diff-container');
UI.ARIAUtils.markAsTabpanel(this.diffContainer);
this.diffContainer.addEventListener('click', event => this.click(event));

this.diffView = this.diffContainer.appendChild(new DiffView.DiffView.DiffView());
if (shouldRenderCombinedDiffView()) {
// TODO(ergunsh): Handle clicks from CombinedDiffView too.
this.combinedDiffView = new CombinedDiffView.CombinedDiffView();
this.combinedDiffView.workspaceDiff = this.workspaceDiff;
this.combinedDiffView.show(this.diffContainer);
} else {
this.diffView = this.diffContainer.appendChild(new DiffView.DiffView.DiffView());
this.diffContainer.addEventListener('click', event => this.click(event));
}

this.toolbar = mainWidget.element.createChild('devtools-toolbar', 'changes-toolbar');
this.toolbar.setAttribute('jslog', `${VisualLogging.toolbar()}`);
this.toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('changes.revert'));
this.diffStats = new UI.Toolbar.ToolbarText('');
this.toolbar.appendToolbarItem(this.diffStats);

this.toolbar.appendToolbarItem(new UI.Toolbar.ToolbarSeparator());
this.toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('changes.copy', {
label: i18nLazyString(UIStrings.copy),
}));
if (!shouldRenderCombinedDiffView()) {
// TODO(ergunsh): We do not show the diff stats & the copy button for the combined view.
this.diffStats = new UI.Toolbar.ToolbarText('');
this.toolbar.appendToolbarItem(this.diffStats);

this.toolbar.appendToolbarItem(new UI.Toolbar.ToolbarSeparator());
this.toolbar.appendToolbarItem(UI.Toolbar.Toolbar.createActionButton('changes.copy', {
label: i18nLazyString(UIStrings.copy),
}));
}

this.hideDiff(i18nString(UIStrings.noChanges), i18nString(UIStrings.changesViewDescription), CHANGES_VIEW_URL);
this.selectedUISourceCodeChanged();
Expand Down Expand Up @@ -235,7 +248,7 @@ export class ChangesView extends UI.Widget.VBox {
}

private hideDiff(header: string, text: string, link?: Platform.DevToolsPath.UrlString): void {
this.diffStats.setText('');
this.diffStats?.setText('');
this.toolbar.setEnabled(false);
this.diffContainer.style.display = 'none';
this.emptyWidget.header = header;
Expand All @@ -257,12 +270,14 @@ export class ChangesView extends UI.Widget.VBox {
if (!diff || (diff.length === 1 && diff[0][0] === Diff.Diff.Operation.Equal)) {
this.hideDiff(i18nString(UIStrings.noChanges), i18nString(UIStrings.changesViewDescription), CHANGES_VIEW_URL);
} else {
this.diffStats.setText(diffStats(diff));
this.diffStats?.setText(diffStats(diff));
this.toolbar.setEnabled(true);
this.emptyWidget.hideWidget();
const mimeType = (this.selectedUISourceCode as Workspace.UISourceCode.UISourceCode).mimeType();
this.diffContainer.style.display = 'block';
this.diffView.data = {diff, mimeType};
if (this.diffView) {
this.diffView.data = {diff, mimeType};
}
}
}
}
Expand All @@ -284,3 +299,8 @@ export class ActionDelegate implements UI.ActionRegistration.ActionDelegate {
return false;
}
}

function shouldRenderCombinedDiffView(): boolean {
const {hostConfig} = Root.Runtime;
return Boolean(hostConfig.devToolsFreestyler?.patching);
}
108 changes: 108 additions & 0 deletions front_end/panels/changes/CombinedDiffView.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright 2025 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import * as Platform from '../../core/platform/platform.js';
import * as SDK from '../../core/sdk/sdk.js';
import * as Bindings from '../../models/bindings/bindings.js';
import * as Breakpoints from '../../models/breakpoints/breakpoints.js';
import * as Persistence from '../../models/persistence/persistence.js';
import * as Workspace from '../../models/workspace/workspace.js';
import * as WorkspaceDiff from '../../models/workspace_diff/workspace_diff.js';
import {describeWithEnvironment, updateHostConfig} from '../../testing/EnvironmentHelpers.js';
import {expectCall} from '../../testing/ExpectStubCall.js';
import {createFileSystemUISourceCode} from '../../testing/UISourceCodeHelpers.js';

import * as CombinedDiffView from './CombinedDiffView.js';

const {urlString} = Platform.DevToolsPath;

const URL = urlString`file:///tmp/example.html`;

function createWorkspace(): Workspace.Workspace.WorkspaceImpl {
return Workspace.Workspace.WorkspaceImpl.instance({forceNew: true});
}

function createWorkspaceDiff({workspace}: {workspace: Workspace.Workspace.WorkspaceImpl}):
WorkspaceDiff.WorkspaceDiff.WorkspaceDiffImpl {
const debuggerWorkspaceBinding = Bindings.DebuggerWorkspaceBinding.DebuggerWorkspaceBinding.instance({
forceNew: true,
targetManager: SDK.TargetManager.TargetManager.instance(),
resourceMapping:
new Bindings.ResourceMapping.ResourceMapping(SDK.TargetManager.TargetManager.instance(), workspace),
});
const breakpointManager = Breakpoints.BreakpointManager.BreakpointManager.instance({
forceNew: true,
targetManager: SDK.TargetManager.TargetManager.instance(),
workspace,
debuggerWorkspaceBinding,
});
Persistence.Persistence.PersistenceImpl.instance({forceNew: true, workspace, breakpointManager});
Persistence.NetworkPersistenceManager.NetworkPersistenceManager.instance({forceNew: true, workspace});
return new WorkspaceDiff.WorkspaceDiff.WorkspaceDiffImpl(workspace);
}

async function createCombinedDiffView({workspaceDiff}: {workspaceDiff: WorkspaceDiff.WorkspaceDiff.WorkspaceDiffImpl}) {
const view = sinon.stub<[CombinedDiffView.ViewInput, unknown, HTMLElement]>();
const combinedDiffView = new CombinedDiffView.CombinedDiffView(undefined, view);
combinedDiffView.workspaceDiff = workspaceDiff;

/**
* Triggers the action and returns args of the next view function
* call.
*/
async function expectViewUpdate(action: () => void) {
const result = expectCall(view);
action();
const viewArgs = await result;
return viewArgs[0];
}

const initialViewInput = await expectViewUpdate(() => {
combinedDiffView.markAsRoot();
combinedDiffView.show(document.body);
});

return {initialViewInput, combinedDiffView, view, expectViewUpdate};
}

describeWithEnvironment('CombinedDiffView', () => {
let workspaceDiff: WorkspaceDiff.WorkspaceDiff.WorkspaceDiffImpl;
let uiSourceCode: Workspace.UISourceCode.UISourceCode;
beforeEach(() => {
// This is needed for tracking file system UI source codes.
updateHostConfig({devToolsImprovedWorkspaces: {enabled: true}});
const workspace = createWorkspace();
workspaceDiff = createWorkspaceDiff({workspace});
({uiSourceCode} =
createFileSystemUISourceCode({url: URL, content: 'const data={original:true}', mimeType: 'text/javascript'}));
});

it('should render modified UISourceCode from a workspaceDiff on initial render', async () => {
uiSourceCode.setWorkingCopy('const data={original:false}');
const {initialViewInput} = await createCombinedDiffView({workspaceDiff});

assert.lengthOf(initialViewInput.singleDiffViewInputs, 1);
});

it('should render newly modified UISourceCode from a workspaceDiff', async () => {
const {initialViewInput, expectViewUpdate} = await createCombinedDiffView({workspaceDiff});
assert.lengthOf(initialViewInput.singleDiffViewInputs, 0);

const viewInput = await expectViewUpdate(() => {
uiSourceCode.setWorkingCopy('const data={original:false}');
});

assert.lengthOf(viewInput.singleDiffViewInputs, 1);
});

it('should re-render modified UISourceCode from a workspaceDiff', async () => {
uiSourceCode.setWorkingCopy('const data={original:false}');
const {initialViewInput, expectViewUpdate} = await createCombinedDiffView({workspaceDiff});
assert.lengthOf(initialViewInput.singleDiffViewInputs, 1);

await expectViewUpdate(() => {
uiSourceCode.setWorkingCopy('const data={modified:true}');
});
});
});
Loading

0 comments on commit 6ec9229

Please sign in to comment.