Add gr-focus-layer to gr-diff
This layer is responsible for applying the class to elements within
components when the corresponding line is out of focus.
The layer is partially based on the gr-coverage layer. For each line,
the layer checks whether it is in focus or out of focus. If it is out of
focus, the diff colors are saturated, and no green or red colors are
rendered for those diffs.
If diffRangesToFocus is undefined, the layer is treated as disabled, and
no logic is applied.
Change-Id: I4074e8e3eab90284243c284cb1c9b626b4628c42
Screencast: https://screencast.googleplex.com/cast/NDY2NTU4MDEwMjYxNTA0MHxjOGM4ZjBhOC0yMA
Release-Notes: skip
Google-Bug-Id: b/345126277
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts
index 22a7694..98a2093 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff-styles.ts
@@ -367,10 +367,16 @@
gr-diff-row td.sign.add.no-intraline-info,
gr-diff-section tbody.delta.total gr-diff-row td.content.add div.contentText {
background-color: var(--dark-add-highlight-color);
+ &:has(.is-out-of-focus-range) {
+ background-color: transparent;
+ }
}
gr-diff-row td.content.add div.contentText,
gr-diff-row td.sign.add {
background-color: var(--light-add-highlight-color);
+ &:has(.is-out-of-focus-range) {
+ background-color: transparent;
+ }
}
/* If there are no intraline info, consider everything changed */
gr-diff-row td.content.remove div.contentText .intraline,
@@ -382,10 +388,16 @@
div.contentText,
gr-diff-row td.sign.remove.no-intraline-info {
background-color: var(--dark-remove-highlight-color);
+ &:has(.is-out-of-focus-range) {
+ background-color: transparent;
+ }
}
gr-diff-row td.content.remove div.contentText,
gr-diff-row td.sign.remove {
background-color: var(--light-remove-highlight-color);
+ &:has(.is-out-of-focus-range) {
+ background-color: transparent;
+ }
}
gr-diff-element table.responsive gr-diff-row td.content div.contentText {
white-space: break-spaces;
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index f3534c2..59dad46 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -75,6 +75,7 @@
grDiffTextStyles,
} from './gr-diff-styles';
import {GrCoverageLayer} from '../gr-coverage-layer/gr-coverage-layer';
+import {GrFocusLayer} from '../gr-focus-layer/gr-focus-layer';
import {
GrAnnotationImpl,
getStringLength,
@@ -275,6 +276,8 @@
private coverageLayerRight = new GrCoverageLayer(Side.RIGHT);
+ private focusLayer = new GrFocusLayer();
+
private rangeLayer = new GrRangedCommentLayer();
@state() groups: GrDiffGroup[] = [];
@@ -440,6 +443,9 @@
if (changedProperties.has('lineOfInterest')) {
this.lineOfInterestChanged();
}
+ if (changedProperties.has('diffRangesToFocus')) {
+ this.updateFocusRanges(this.diffRangesToFocus);
+ }
}
protected override async getUpdateComplete(): Promise<boolean> {
@@ -720,6 +726,10 @@
this.coverageLayerRight.setRanges(rs.filter(r => r?.side === Side.RIGHT));
}
+ private updateFocusRanges(rs?: DiffRangesToFocus) {
+ this.focusLayer.setRanges(rs);
+ }
+
private onDiffContextExpanded = (
e: CustomEvent<DiffContextExpandedEventDetail>
) => {
@@ -746,6 +756,7 @@
this.rangeLayer,
this.coverageLayerLeft,
this.coverageLayerRight,
+ this.focusLayer,
];
this.layersChanged();
}
diff --git a/polygerrit-ui/app/embed/diff/gr-focus-layer/gr-focus-layer.ts b/polygerrit-ui/app/embed/diff/gr-focus-layer/gr-focus-layer.ts
new file mode 100644
index 0000000..b3d780e
--- /dev/null
+++ b/polygerrit-ui/app/embed/diff/gr-focus-layer/gr-focus-layer.ts
@@ -0,0 +1,151 @@
+/**
+ * @license
+ * Copyright 2024 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import {DiffRangesToFocus, GrDiffLine, Side} from '../../../api/diff';
+import {DiffLayer, DiffLayerListener} from '../../../types/types';
+
+// A range of lines in a diff.
+export type Range = {
+ start: number;
+ end: number;
+};
+
+export class GrFocusLayer implements DiffLayer {
+ private diffRangesToFocus?: DiffRangesToFocus;
+
+ /**
+ * Diff Ranges which were unfocused(colors are saturated) in previous call.
+ */
+ private previousUnfocusedRanges?: DiffRangesToFocus;
+
+ /**
+ * Has any line been annotated already in the lifetime of this layer?
+ * If not, then `setRanges()` does not have to call `notify()` and thus
+ * trigger re-rendering of the affected diff rows.
+ */
+ // visible for testing
+ annotated = false;
+
+ private listeners: DiffLayerListener[] = [];
+
+ addListener(listener: DiffLayerListener) {
+ this.listeners.push(listener);
+ }
+
+ removeListener(listener: DiffLayerListener) {
+ this.listeners = this.listeners.filter(f => f !== listener);
+ }
+
+ setRanges(diffRangesToFocus?: DiffRangesToFocus) {
+ if (!this.previousUnfocusedRanges && !diffRangesToFocus) return;
+ this.diffRangesToFocus = diffRangesToFocus;
+
+ // If ranges are set before any diff row was rendered, then great, no need
+ // to notify and re-render.
+ if (this.annotated) {
+ this.notify({
+ left: [
+ ...(this.previousUnfocusedRanges?.left ?? []),
+ ...(diffRangesToFocus?.left ?? []),
+ ],
+ right: [
+ ...(this.previousUnfocusedRanges?.right ?? []),
+ ...(diffRangesToFocus?.right ?? []),
+ ],
+ });
+ }
+ this.previousUnfocusedRanges = undefined;
+ }
+
+ private notify(ranges: DiffRangesToFocus) {
+ for (const r of ranges.left) {
+ for (const l of this.listeners) l(r.start, r.end, Side.LEFT);
+ }
+ for (const r of ranges.right) {
+ for (const l of this.listeners) l(r.start, r.end, Side.RIGHT);
+ }
+ }
+
+ /**
+ * Layer method to add is-out-of-focus-range to a textElement
+ * if line is out of focus.
+ *
+ * @param textEl The gr-text element for this line.
+ * @param lineNumberEl The <td> element with the line number.
+ * @param _line Not used for this layer. (unused parameter)
+ * @param side The side of the diff.
+ */
+ annotate(
+ textEl: HTMLElement,
+ lineNumberEl: HTMLElement,
+ _line: GrDiffLine,
+ side: Side
+ ) {
+ if (!lineNumberEl || !textEl || !this.diffRangesToFocus) {
+ return;
+ }
+ let elementLineNumber = -1;
+ const dataValue = lineNumberEl.getAttribute('data-value');
+ if (dataValue) {
+ elementLineNumber = Number(dataValue);
+ }
+ if (!elementLineNumber || elementLineNumber < 1) return;
+
+ let focusedRanges: Range[] = [];
+ if (side === Side.LEFT) {
+ focusedRanges = this.diffRangesToFocus.left;
+ } else if (side === Side.RIGHT) {
+ focusedRanges = this.diffRangesToFocus.right;
+ }
+ // TODO(anuragpathak): Optimize this using the same approach as gr-coverage-layer.ts
+ if (
+ !focusedRanges.some(
+ range =>
+ elementLineNumber >= range.start && elementLineNumber <= range.end
+ )
+ ) {
+ textEl.classList.add('is-out-of-focus-range');
+ this.updateUnfocusedRanges(elementLineNumber, side);
+ }
+ }
+
+ private updateUnfocusedRanges(lineNumber: number, side: Side) {
+ this.previousUnfocusedRanges = {
+ left:
+ side === Side.LEFT
+ ? this.addToRange(lineNumber, this.previousUnfocusedRanges?.left)
+ : this.previousUnfocusedRanges?.left ?? [],
+ right:
+ side === Side.RIGHT
+ ? this.addToRange(lineNumber, this.previousUnfocusedRanges?.right)
+ : this.previousUnfocusedRanges?.right ?? [],
+ };
+ }
+
+ private addToRange(lineNumber: number, ranges?: Range[]) {
+ const previousRange: Range[] = [];
+ if (ranges) {
+ previousRange.push(...ranges);
+ }
+ let lastEntryInRange = previousRange.pop();
+ if (lastEntryInRange) {
+ if (lastEntryInRange.end + 1 === lineNumber) {
+ lastEntryInRange = {start: lastEntryInRange.start, end: lineNumber};
+ previousRange.push(lastEntryInRange);
+ } else {
+ previousRange.push(lastEntryInRange, {
+ start: lineNumber,
+ end: lineNumber,
+ });
+ }
+ } else {
+ previousRange.push({
+ start: lineNumber,
+ end: lineNumber,
+ });
+ }
+ return previousRange;
+ }
+}
diff --git a/polygerrit-ui/app/embed/diff/gr-focus-layer/gr-focus-layer_test.ts b/polygerrit-ui/app/embed/diff/gr-focus-layer/gr-focus-layer_test.ts
new file mode 100644
index 0000000..fc61d21
--- /dev/null
+++ b/polygerrit-ui/app/embed/diff/gr-focus-layer/gr-focus-layer_test.ts
@@ -0,0 +1,144 @@
+/**
+ * @license
+ * Copyright 2024 Google LLC
+ * SPDX-License-Identifier: Apache-2.0
+ */
+import '../../../test/common-test-setup';
+
+import {assert} from '@open-wc/testing';
+import {SinonStub} from 'sinon';
+
+import {DiffRangesToFocus, Side, GrDiffLineType} from '../../../api/diff';
+
+import {GrFocusLayer} from './gr-focus-layer';
+import {GrDiffLine} from '../gr-diff/gr-diff-line';
+
+const ONE_RANGE: DiffRangesToFocus = {
+ left: [{start: 1, end: 2}],
+ right: [{start: 9, end: 10}],
+};
+
+const RANGES: DiffRangesToFocus = {
+ left: [
+ {start: 1, end: 2},
+ {start: 13, end: 14},
+ {start: 25, end: 76},
+ ],
+ right: [
+ {start: 1, end: 2},
+ {start: 23, end: 24},
+ {start: 55, end: 65},
+ ],
+};
+
+suite('gr-focus-layer', () => {
+ let layer: GrFocusLayer;
+ const line = new GrDiffLine(GrDiffLineType.ADD);
+
+ function createLineElement(lineNumber: number, side: Side) {
+ const lineNumberEl = document.createElement('div');
+ lineNumberEl.setAttribute('data-side', side);
+ lineNumberEl.setAttribute('data-value', lineNumber.toString());
+ lineNumberEl.className = side;
+ return lineNumberEl;
+ }
+
+ function createTextElement() {
+ const textElement = document.createElement('div');
+ textElement.innerText = 'A line of code';
+ return textElement;
+ }
+
+ suite('setRanges and notify', () => {
+ let listener: SinonStub;
+
+ setup(() => {
+ layer = new GrFocusLayer();
+ listener = sinon.stub();
+ layer.addListener(listener);
+ });
+
+ test('empty ranges do not notify', () => {
+ layer.annotated = true;
+ layer.setRanges();
+ assert.isFalse(listener.called);
+ });
+
+ test('do not notify while annotated is false', () => {
+ layer.setRanges(RANGES);
+ assert.isFalse(listener.called);
+ });
+
+ test('initial ranges', () => {
+ layer.annotated = true;
+ layer.setRanges(ONE_RANGE);
+ assert.isTrue(listener.called);
+ assert.equal(listener.callCount, 2);
+ assert.equal(listener.getCall(0).args[0], 1);
+ assert.equal(listener.getCall(0).args[1], 2);
+ assert.equal(listener.getCall(1).args[0], 9);
+ assert.equal(listener.getCall(1).args[1], 10);
+ });
+
+ test('old ranges and new range', () => {
+ layer.annotated = true;
+ layer.setRanges(ONE_RANGE);
+ listener.reset();
+ layer.annotate(
+ createTextElement(),
+ createLineElement(100, Side.RIGHT),
+ line,
+ Side.RIGHT
+ );
+ layer.annotate(
+ createTextElement(),
+ createLineElement(101, Side.RIGHT),
+ line,
+ Side.RIGHT
+ );
+ layer.setRanges(RANGES);
+ assert.isTrue(listener.called);
+ assert.equal(listener.callCount, 7);
+ assert.equal(listener.getCall(3).args[0], 100);
+ assert.equal(listener.getCall(3).args[1], 101);
+ assert.equal(listener.getCall(3).args[2], Side.RIGHT);
+ });
+ });
+
+ suite('annotate', () => {
+ function hasOutOfFocusClass(lineNumber: number, side: Side) {
+ const textEl = createTextElement();
+ layer.annotate(textEl, createLineElement(lineNumber, side), line, side);
+ return textEl.classList.contains('is-out-of-focus-range');
+ }
+
+ setup(() => {
+ layer = new GrFocusLayer();
+ layer.setRanges(RANGES);
+ });
+
+ test('line 1-2 are focussed on both sides', () => {
+ assert.isFalse(hasOutOfFocusClass(1, Side.LEFT));
+ assert.isFalse(hasOutOfFocusClass(2, Side.RIGHT));
+ assert.isFalse(hasOutOfFocusClass(1, Side.LEFT));
+ assert.isFalse(hasOutOfFocusClass(2, Side.RIGHT));
+ });
+
+ test('line 3-12 are not focussed on left side', () => {
+ for (let index = 3; index < 12; index++) {
+ assert.isTrue(hasOutOfFocusClass(index, Side.LEFT));
+ }
+ });
+
+ test('line 3-22 are not focussed on right side', () => {
+ for (let index = 3; index < 22; index++) {
+ assert.isTrue(hasOutOfFocusClass(index, Side.RIGHT));
+ }
+ });
+
+ test('line 13-14 are focussed on left side', () => {
+ assert.isFalse(hasOutOfFocusClass(13, Side.LEFT));
+ assert.isFalse(hasOutOfFocusClass(14, Side.LEFT));
+ });
+ });
+});
