Expand details for token highlight events
We want to receive more details from token highlight to better react to
highlighting events. Adding range of the highlighted token in the text content
and which side triggered the highlighting (left/right).
We also want to improve the distinction between a new highlight
and clearing a highlight.
Change-Id: I9b566912be95ff852f78d2be939bfb0ac3de5624
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index 1453fd0..ee579ff 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -53,30 +53,30 @@
}
/**
+ * Represents a "generic" text range in the code (e.g. text selection)
+ */
+interface TextRange {
+ /** first line of the range (1-based inclusive). */
+ start_line: number;
+ /** first column of the range (in the first line) (1-based inclusive). */
+ start_column: number;
+ /** last line of the range (1-based inclusive). */
+ end_line: number;
+ /** last column of the range (in the end line) (1-based inclusive). */
+ end_column: number;
+}
+
+/**
* Represents a syntax block in a code (e.g. method, function, class, if-else).
*/
export declare interface SyntaxBlock {
/** Name of the block (e.g. name of the method/class)*/
name: string;
- /** Where does this block syntatically starts and ends (line number and column).*/
- range: {
- /** first line of the block (1-based inclusive). */
- start_line: number;
- /**
- * column of the range start inside the first line (e.g. "{" character ending a function/method)
- * (1-based inclusive).
- */
- start_column: number;
- /**
- * last line of the block (1-based inclusive).
- */
- end_line: number;
- /**
- * column of the block end inside the end line (e.g. "}" character ending a function/method)
- * (1-based inclusive).
- */
- end_column: number;
- };
+ /**
+ * Where does this block syntatically starts and ends (line number and
+ * column).
+ */
+ range: TextRange;
/** Sub-blocks of the current syntax block (e.g. methods of a class) */
children: SyntaxBlock[];
}
@@ -210,15 +210,22 @@
}
/**
- * Listens to changes in token highlighting - when a new token starts or stopped being highlighted.
- * Examples:
- * - Token highlighted: ('myFunctionName', 12, [Element]).
- * - Token unhighlighted: (undefined, 0, undefined).
+ * Event details when a token is highlighted.
*/
-export type TokenHighlightedListener = (
- newHighlight: string | undefined,
- newLineNumber: number,
- hoveredElement?: Element
+export declare interface TokenHighlightEventDetails {
+ token: string;
+ element: Element;
+ side: Side;
+ range: TextRange;
+}
+
+/**
+ * Listens to changes in token highlighting - when a new token starts or stopped
+ * being highlighted. undefined is sent if the event is about a clear in
+ * highlighting.
+ */
+export type TokenHighlightListener = (
+ tokenHighlightEvent?: TokenHighlightEventDetails
) => void;
export declare interface ImageDiffPreferences {
diff --git a/polygerrit-ui/app/api/embed.ts b/polygerrit-ui/app/api/embed.ts
index fed724e..520aeec 100644
--- a/polygerrit-ui/app/api/embed.ts
+++ b/polygerrit-ui/app/api/embed.ts
@@ -24,7 +24,7 @@
DiffLayer,
GrAnnotation,
GrDiffCursor,
- TokenHighlightedListener,
+ TokenHighlightListener,
} from './diff';
declare global {
@@ -35,7 +35,7 @@
TokenHighlightLayer: {
new (
container?: HTMLElement,
- listener?: TokenHighlightedListener
+ listener?: TokenHighlightListener
): DiffLayer;
};
};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts
index 480e26c..de7d007 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer.ts
@@ -15,9 +15,17 @@
* limitations under the License.
*/
import {DiffLayer, DiffLayerListener} from '../../../types/types';
-import {GrDiffLine, Side, TokenHighlightedListener} from '../../../api/diff';
+import {GrDiffLine, Side, TokenHighlightListener} from '../../../api/diff';
+import {assertIsDefined} from '../../../utils/common-util';
import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
import {debounce, DelayedTask} from '../../../utils/async-util';
+
+import {
+ getLineElByChild,
+ getSideByLineEl,
+ getPreviousContentNodes,
+} from '../gr-diff/gr-diff-utils';
+
import {
getLineNumberByChild,
lineNumberToNumber,
@@ -66,7 +74,7 @@
private currentHighlight?: string;
/** Trigger when a new token starts or stoped being highlighted.*/
- private readonly tokenHighlightedListener?: TokenHighlightedListener;
+ private readonly tokenHighlightListener?: TokenHighlightListener;
/**
* The line of the currently highlighted token. We store this in order to
@@ -100,9 +108,9 @@
constructor(
container: HTMLElement = document.documentElement,
- tokenHighlightedListener?: TokenHighlightedListener
+ tokenHighlightListener?: TokenHighlightListener
) {
- this.tokenHighlightedListener = tokenHighlightedListener;
+ this.tokenHighlightListener = tokenHighlightListener;
container.addEventListener('click', e => {
this.handleContainerClick(e);
});
@@ -260,18 +268,42 @@
const oldLineNumber = this.currentHighlightLineNumber;
this.currentHighlight = newHighlight;
this.currentHighlightLineNumber = newLineNumber;
-
- if (this.tokenHighlightedListener) {
- this.tokenHighlightedListener(
- newHighlight,
- newLineNumber,
- newHoveredElement
- );
- }
+ this.triggerTokenHighlightEvent(
+ newHighlight,
+ newLineNumber,
+ newHoveredElement
+ );
this.notifyForToken(oldHighlight, oldLineNumber);
this.notifyForToken(newHighlight, newLineNumber);
}
+ triggerTokenHighlightEvent(
+ token: string | undefined,
+ line: number,
+ element: Element | undefined
+ ) {
+ if (!this.tokenHighlightListener) {
+ return;
+ }
+ if (!token || !element) {
+ this.tokenHighlightListener(undefined);
+ return;
+ }
+ const previousTextLength = getPreviousContentNodes(element)
+ .map(sib => sib.textContent!.length)
+ .reduce((partial_sum, a) => partial_sum + a, 0);
+ const lineEl = getLineElByChild(element);
+ assertIsDefined(lineEl, 'Line element should be found!');
+ const side = getSideByLineEl(lineEl);
+ const range = {
+ start_line: line,
+ start_column: previousTextLength + 1, // 1-based inclusive
+ end_line: line,
+ end_column: previousTextLength + token.length, // 1-based inclusive
+ };
+ this.tokenHighlightListener({token, element, side, range});
+ }
+
getSortedLinesForSide(
lineMapping: Map<string, Set<number>>,
token: string | undefined,
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts
index 4f44665..2993d35 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts
@@ -17,7 +17,7 @@
import '../../../test/common-test-setup-karma';
import * as MockInteractions from '@polymer/iron-test-helpers/mock-interactions';
-import {Side} from '../../../api/diff';
+import {Side, TokenHighlightEventDetails} from '../../../api/diff';
import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line.js';
import {HOVER_DELAY_MS, TokenHighlightLayer} from './token-highlight-layer';
import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
@@ -66,14 +66,12 @@
let container: HTMLElement;
let listener: MockListener;
let highlighter: TokenHighlightLayer;
- let tokenHighlightingCalls: any[] = [];
+ let tokenHighlightingCalls: {details?: TokenHighlightEventDetails}[] = [];
- function tokenHighlightedListener(
- newHighlight: string | undefined,
- newLineNumber: number,
- hoveredElement?: Element
+ function tokenHighlightListener(
+ highlightDetails?: TokenHighlightEventDetails
) {
- tokenHighlightingCalls.push({newHighlight, newLineNumber, hoveredElement});
+ tokenHighlightingCalls.push({details: highlightDetails});
}
setup(async () => {
@@ -81,7 +79,7 @@
tokenHighlightingCalls = [];
container = document.createElement('div');
document.body.appendChild(container);
- highlighter = new TokenHighlightLayer(container, tokenHighlightedListener);
+ highlighter = new TokenHighlightLayer(container, tokenHighlightListener);
highlighter.addListener((...args) => listener.notify(...args));
});
@@ -107,10 +105,13 @@
const lineId = createLineId();
const template = html`
<div class="line">
- <div data-value=${line} class="lineNum"></div>
- <div id=${lineId} class="line-content">${text}</div>
+ <div data-value=${line} class="lineNum right"></div>
+ <div class="content">
+ <div id=${lineId} class="contentText">${text}</div>
+ </div>
</div>
`;
+
const div = document.createElement('div');
render(template, div);
container.appendChild(div);
@@ -277,19 +278,16 @@
assert.equal(tokenHighlightingCalls.length, 0);
clock.tick(HOVER_DELAY_MS);
assert.equal(tokenHighlightingCalls.length, 1);
- assert.deepEqual(tokenHighlightingCalls[0], {
- newHighlight: 'words',
- newLineNumber: 1,
- hoveredElement: words1,
+ assert.deepEqual(tokenHighlightingCalls[0].details, {
+ token: 'words',
+ side: Side.RIGHT,
+ element: words1,
+ range: {start_line: 1, start_column: 5, end_line: 1, end_column: 9},
});
MockInteractions.click(container);
assert.equal(tokenHighlightingCalls.length, 2);
- assert.deepEqual(tokenHighlightingCalls[1], {
- newHighlight: undefined,
- newLineNumber: 0,
- hoveredElement: undefined,
- });
+ assert.deepEqual(tokenHighlightingCalls[1].details, undefined);
});
test('clicking clears highlight', async () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
index fada9cb..7393606 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
@@ -129,6 +129,29 @@
rootId: string;
}
+const VISIBLE_TEXT_NODE_TYPES = [Node.TEXT_NODE, Node.ELEMENT_NODE];
+
+export function getPreviousContentNodes(node?: Node | null) {
+ const sibs = [];
+ while (node) {
+ const {parentNode, previousSibling} = node;
+ const topContentLevel =
+ parentNode &&
+ (parentNode as HTMLElement).classList.contains('contentText');
+ let previousEl: Node | undefined | null;
+ if (previousSibling) {
+ previousEl = previousSibling;
+ } else if (!topContentLevel) {
+ previousEl = parentNode?.previousSibling;
+ }
+ if (previousEl && VISIBLE_TEXT_NODE_TYPES.includes(previousEl.nodeType)) {
+ sibs.push(previousEl);
+ }
+ node = previousEl;
+ }
+ return sibs;
+}
+
export function isThreadEl(node: Node): node is GrDiffThreadElement {
return (
node.nodeType === Node.ELEMENT_NODE &&
