Skip to content

Commit 20755bc

Browse files
committed
Resolves #4675 virtualizes Commit Composer diff rendering
1 parent b156a10 commit 20755bc

File tree

3 files changed

+84
-20
lines changed

3 files changed

+84
-20
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
2222
- Adds `gitlens.graph.initialRowSelection` setting to specify whether to select the "Work in progress" (WIP) row instead of HEAD if there are uncommitted changes
2323
- Changes to use the "merge target" when we are creating pull requests ([#4709](https://github.com/gitkraken/vscode-gitlens/issues/4709))
2424
- Changes the minimum VS Code version to 1.95.0 ([#4691](https://github.com/gitkraken/vscode-gitlens/issues/4691))
25+
- Greatly improves performance of the _Commit Composer_ view by virtualizing file diffs ([#4675](https://github.com/gitkraken/vscode-gitlens/issues/4675))
2526

2627
### Fixed
2728

src/webviews/apps/plus/composer/components/diff/diff-file.ts

Lines changed: 80 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,39 @@ export class GlDiffFile extends LitElement {
5353
@state()
5454
private parsedDiff?: DiffFile[];
5555

56+
@state()
57+
private hasRendered = false;
58+
59+
@state()
60+
private _isVisible = false;
61+
62+
@property({ type: Boolean, reflect: true, attribute: 'is-visible' })
63+
get isVisible(): boolean {
64+
return this._isVisible;
65+
}
66+
67+
private set isVisible(value: boolean) {
68+
this._isVisible = value;
69+
}
70+
5671
// should only ever be one file
5772
get diffFile(): DiffFile | undefined {
5873
return this.parsedDiff?.[0];
5974
}
6075

6176
private diff2htmlUi?: Diff2HtmlUI;
77+
private intersectionObserver?: IntersectionObserver;
78+
79+
override connectedCallback() {
80+
super.connectedCallback?.();
81+
this.setupIntersectionObserver();
82+
}
83+
84+
override disconnectedCallback() {
85+
super.disconnectedCallback?.();
86+
this.intersectionObserver?.disconnect();
87+
this.intersectionObserver = undefined;
88+
}
6289

6390
override firstUpdated() {
6491
this.processDiff();
@@ -68,42 +95,78 @@ export class GlDiffFile extends LitElement {
6895
override updated(changedProperties: Map<string | number | symbol, unknown>) {
6996
super.updated(changedProperties);
7097

71-
if (changedProperties.has('filename') || changedProperties.has('hunks')) {
98+
if (changedProperties.has('diffText') || changedProperties.has('filename') || changedProperties.has('hunks')) {
7299
this.processDiff();
73100
}
74-
if (changedProperties.has('diffText') || changedProperties.has('defaultExpanded')) {
101+
102+
if (changedProperties.has('parsedDiff') || changedProperties.has('sideBySide')) {
103+
this.renderDiff(true);
104+
} else if (changedProperties.has('defaultExpanded') || changedProperties.has('isVisible')) {
75105
this.renderDiff();
76106
}
77107
}
78108

79109
override render() {
80-
return html`<div id="diff"></div>`;
110+
return html`<div id="diff" class="diff-container"></div>`;
81111
}
82112

83-
private renderDiff() {
84-
if (!this.parsedDiff || !this.filename) {
113+
private setupIntersectionObserver() {
114+
this.intersectionObserver = new IntersectionObserver(
115+
entries => {
116+
for (const entry of entries) {
117+
this.isVisible = entry.isIntersecting;
118+
}
119+
},
120+
{
121+
// Use a margin to start rendering slightly before the element enters the viewport
122+
rootMargin: '100px',
123+
},
124+
);
125+
this.intersectionObserver.observe(this);
126+
}
127+
128+
private clearDiff() {
129+
if (this.targetElement) {
85130
this.targetElement.innerHTML = '';
131+
}
132+
this.hasRendered = false;
133+
}
134+
135+
private renderDiff(force = false) {
136+
// If not visible or no data, clear and return
137+
if (!this.isVisible || !this.parsedDiff || !this.filename) {
138+
this.clearDiff();
86139
return;
87140
}
88-
const config: Diff2HtmlUIConfig = {
89-
colorScheme: ColorSchemeType.AUTO,
90-
outputFormat: this.sideBySide ? 'side-by-side' : 'line-by-line',
91-
drawFileList: false,
92-
highlight: false,
93-
// NOTE: Avoiding passing rawTemplates to Diff2HtmlUI to prevent Diff2Html from
94-
// compiling templates at runtime via Hogan.compile (which uses eval), which violates
95-
// the webview CSP (no 'unsafe-eval'). If we need to customize templates in the future,
96-
// switch to providing precompiled templates in the bundle instead of raw strings.
97-
compiledTemplates: compiledComposerTemplates,
98-
};
99-
this.diff2htmlUi = new Diff2HtmlUI(this.targetElement, this.parsedDiff, config);
141+
142+
// Don't re-render if already rendered
143+
if (this.hasRendered && !force) {
144+
return;
145+
}
146+
147+
if (!this.diff2htmlUi || force) {
148+
const config: Diff2HtmlUIConfig = {
149+
colorScheme: ColorSchemeType.AUTO,
150+
outputFormat: this.sideBySide ? 'side-by-side' : 'line-by-line',
151+
drawFileList: false,
152+
highlight: false,
153+
// NOTE: Avoiding passing rawTemplates to Diff2HtmlUI to prevent Diff2Html from
154+
// compiling templates at runtime via Hogan.compile (which uses eval), which violates
155+
// the webview CSP (no 'unsafe-eval'). If we need to customize templates in the future,
156+
// switch to providing precompiled templates in the bundle instead of raw strings.
157+
compiledTemplates: compiledComposerTemplates,
158+
};
159+
this.diff2htmlUi = new Diff2HtmlUI(this.targetElement, this.parsedDiff, config);
160+
}
100161
this.diff2htmlUi.draw();
101162
// this.diff2htmlUi.highlightCode();
102163

103164
const detailsElement = this.targetElement?.querySelector('details');
104165
if (detailsElement) {
105166
detailsElement.open = this.defaultExpanded;
106167
}
168+
169+
this.hasRendered = true;
107170
}
108171

109172
private processDiff() {

src/webviews/apps/plus/composer/components/diff/diff.css.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ export const hljsStyles = css`
9999
export const diff2htmlStyles = css`
100100
:host {
101101
--d2h-intrinsic-base-height: 3.5rem; /* header height */
102-
--d2h-intrinsic-container-offset-height: 10px; /* 10px scrollbar height + 2px vertical borders */
102+
--d2h-intrinsic-container-offset-height: 12px; /* 10px scrollbar height + 2px vertical borders */
103103
--d2h-intrinsic-line-count: 50;
104104
--d2h-intrinsic-line-height: calc(
105105
var(--editor-font-size) * 1.5
@@ -113,12 +113,12 @@ export const diff2htmlStyles = css`
113113
position: relative;
114114
}
115115
116-
.d2h-file-wrapper {
116+
.diff-container {
117117
content-visibility: auto;
118118
contain-intrinsic-size: auto var(--d2h-intrinsic-base-height);
119119
}
120120
121-
.d2h-file-wrapper[open] {
121+
.diff-container:has(.d2h-file-wrapper[open]) {
122122
contain-intrinsic-height: var(--d2h-intrinsic-height);
123123
}
124124

0 commit comments

Comments
 (0)