Remove lit diff `isVisible` optimization temporarily
The optimization is still causing some problems, and thus it gets in the
way of the migration from legacy diff to lit diff. Let's first make a
smooth transition, and then come back later to the separate concern of
improving the diff performance.
Release-Notes: skip
Google-Bug-Id: b/237393560
Change-Id: If428ab744e3729dae30c4f9ce3d669ae3d544263
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
index 4783042..f1ce0e3 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row.ts
@@ -60,14 +60,6 @@
layers: DiffLayer[] = [];
/**
- * While not visible we are trying to optimize rendering performance by
- * rendering a simpler version of the diff. Once this has become true it
- * cannot be set back to false.
- */
- @state()
- isVisible = false;
-
- /**
* Semantic DOM diff testing does not work with just table fragments, so when
* running such tests the render() method has to wrap the DOM in a proper
* <table> element.
@@ -120,7 +112,6 @@
* `this.layersApplied = true`.
*/
private async updateLayers(side: Side) {
- if (!this.isVisible) return;
const line = this.line(side);
const contentEl = this.contentRef(side).value;
const lineNumberEl = this.lineNumberRef(side).value;
@@ -139,25 +130,8 @@
this.layersApplied = true;
}
- private renderInvisible() {
- return html`
- <tr>
- <td class="gr-diff blame"></td>
- <td class="gr-diff left"></td>
- <td class="gr-diff left content">
- <div>${this.left?.text ?? ''}</div>
- </td>
- <td class="gr-diff right"></td>
- <td class="gr-diff right content">
- <div>${this.right?.text ?? ''}</div>
- </td>
- </tr>
- `;
- }
-
override render() {
if (!this.left || !this.right) return;
- if (!this.isVisible) return this.renderInvisible();
const row = html`
<tr
${ref(this.tableRowRef)}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
index 4e8bb62..da3230f 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-row_test.ts
@@ -15,7 +15,6 @@
setup(async () => {
element = await fixture<GrDiffRow>(html`<gr-diff-row></gr-diff-row>`);
- element.isVisible = true;
element.addTableWrapperForTesting = true;
await element.updateComplete;
});
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
index 16c7cb3..e0dd6bc 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section.ts
@@ -22,7 +22,6 @@
import '../gr-context-controls/gr-context-controls';
import '../gr-range-header/gr-range-header';
import './gr-diff-row';
-import {whenVisible} from '../../../utils/dom-util';
@customElement('gr-diff-section')
export class GrDiffSection extends LitElement {
@@ -42,13 +41,6 @@
layers: DiffLayer[] = [];
/**
- * While not visible we are trying to optimize rendering performance by
- * rendering a simpler version of the diff.
- */
- @state()
- isVisible = false;
-
- /**
* Semantic DOM diff testing does not work with just table fragments, so when
* running such tests the render() method has to wrap the DOM in a proper
* <table> element.
@@ -67,12 +59,6 @@
return this;
}
- override connectedCallback() {
- super.connectedCallback();
- // TODO: Refine this obviously simplistic approach to optimized rendering.
- whenVisible(this.parentElement!, () => (this.isVisible = true), 1000);
- }
-
override render() {
if (!this.group) return;
const extras: string[] = [];
@@ -100,7 +86,6 @@
.layers=${this.layers}
.lineLength=${this.diffPrefs?.line_length ?? 80}
.tabSize=${this.diffPrefs?.tab_size ?? 2}
- .isVisible=${this.isVisible}
>
</gr-diff-row>
`;
diff --git a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
index 363b001..14efa22 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff-builder/gr-diff-section_test.ts
@@ -19,7 +19,6 @@
html`<gr-diff-section></gr-diff-section>`
);
element.addTableWrapperForTesting = true;
- element.isVisible = true;
await element.updateComplete;
});
