Add tooltips for +Block buttons
We try to show the full syntax path from root
until the chosen block.
Change-Id: I6ad29abeae3a730e267c7bd5fe6feab97797871d
diff --git a/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts
index 1448129..60b1a1a 100644
--- a/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts
+++ b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls.ts
@@ -22,6 +22,7 @@
import '@polymer/paper-icon-button/paper-icon-button';
import '@polymer/paper-item/paper-item';
import '@polymer/paper-listbox/paper-listbox';
+import '@polymer/paper-tooltip/paper-tooltip.js';
import '../../shared/gr-button/gr-button';
import {pluralize} from '../../../utils/string-util';
@@ -43,23 +44,27 @@
/**
* Traverses a hierarchical structure of syntax blocks and
* finds the most local/nested block that can be associated line.
- * It finds the closest block that contains the whole line.
+ * It finds the closest block that contains the whole line and
+ * returns the whole path from the syntax layer (blocks) sent as parameter
+ * to the most nested block - the complete path from the top to bottom layer of
+ * a syntax tree. Example: [myNamepace, MyClass, myMethod1, aLocalFunctionInsideMethod1]
*
* @param lineNum line number for the targeted line.
* @param blocks Blocks for a specific syntax level in the file (to allow recursive calls)
- * @returns
*/
-function findMostNestedContainingBlock(
+function findBlockTreePathForLine(
lineNum: number,
blocks?: SyntaxBlock[]
-): SyntaxBlock | undefined {
+): SyntaxBlock[] {
const containingBlock = blocks?.find(
({range}) => range.start_line < lineNum && range.end_line > lineNum
);
- const containingChildBlock = containingBlock
- ? findMostNestedContainingBlock(lineNum, containingBlock?.children)
- : undefined;
- return containingChildBlock || containingBlock;
+ if (!containingBlock) return [];
+ const innerPathInChild = findBlockTreePathForLine(
+ lineNum,
+ containingBlock?.children
+ );
+ return [containingBlock].concat(innerPathInChild);
}
@customElement('gr-context-controls')
@@ -116,6 +121,9 @@
.belowButton {
top: calc(100% + var(--divider-border));
}
+ .breadcrumbTooltip {
+ white-space: nowrap;
+ }
`;
// To pass CSS mixins for @apply to Polymer components, they need to be
@@ -194,7 +202,11 @@
/**
* Creates a specific expansion button (e.g. +X common lines, +10, +Block).
*/
- private createContextButton(type: ContextButtonType, linesToExpand: number) {
+ private createContextButton(
+ type: ContextButtonType,
+ linesToExpand: number,
+ tooltipText?: string
+ ) {
let text = '';
let groups: GrDiffGroup[] = []; // The groups that replace this one if tapped.
let ariaLabel = '';
@@ -257,6 +269,11 @@
groups
);
+ const tooltip = tooltipText
+ ? html`<paper-tooltip offset="10"
+ ><div class="breadcrumbTooltip">${tooltipText}</div></paper-tooltip
+ >`
+ : undefined;
const button = html` <gr-button
class="${classes}"
link="true"
@@ -265,6 +282,7 @@
@click="${expandHandler}"
>
<span class="showContext">${text}</span>
+ ${tooltip}
</gr-button>`;
return button;
}
@@ -385,13 +403,19 @@
) {
assertIsDefined(this.diff, 'diff');
const syntaxTree = this.diff!.meta_b.syntax_tree;
- const containingBlock = findMostNestedContainingBlock(
+ const outlineSyntaxPath = findBlockTreePathForLine(
referenceLine,
syntaxTree
);
let linesToExpand = numLines;
- if (containingBlock) {
- const {range} = containingBlock;
+ let tooltipText = `${linesToExpand} common lines`;
+ if (outlineSyntaxPath.length) {
+ const {range} = outlineSyntaxPath[outlineSyntaxPath.length - 1];
+ // Create breadcrumb string:
+ // myNamepace > MyClass > myMethod1 > aLocalFunctionInsideMethod1 > (anonymous)
+ tooltipText = outlineSyntaxPath
+ .map(b => b.name || '(anonymous)')
+ .join(' > ');
const targetLine =
buttonType === ContextButtonType.BLOCK_ABOVE
? range.end_line
@@ -401,7 +425,7 @@
linesToExpand = distanceToTargetLine;
}
}
- return this.createContextButton(buttonType, linesToExpand);
+ return this.createContextButton(buttonType, linesToExpand, tooltipText);
}
private contextRange() {
diff --git a/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts
index 37b8723..d866c1a 100644
--- a/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-context-controls/gr-context-controls_test.ts
@@ -23,7 +23,7 @@
import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line';
import {GrDiffGroup, GrDiffGroupType} from '../gr-diff/gr-diff-group';
-import {DiffFileMetaInfo, DiffInfo} from '../../../api/diff';
+import {DiffFileMetaInfo, DiffInfo, SyntaxBlock} from '../../../api/diff';
const blankFixture = fixtureFromElement('div');
@@ -56,6 +56,7 @@
test('no +10 buttons for 10 or less lines', async () => {
element.contextGroups = createContextGroups({count: 10});
+
await flush();
const buttons = element.shadowRoot!.querySelectorAll(
@@ -68,6 +69,7 @@
test('context control at the top', async () => {
element.contextGroups = createContextGroups({offset: 0, count: 20});
element.showBelow = true;
+
await flush();
const buttons = element.shadowRoot!.querySelectorAll(
@@ -86,6 +88,7 @@
element.contextGroups = createContextGroups({offset: 10, count: 20});
element.showAbove = true;
element.showBelow = true;
+
await flush();
const buttons = element.shadowRoot!.querySelectorAll(
@@ -105,6 +108,7 @@
test('context control at the bottom', async () => {
element.contextGroups = createContextGroups({offset: 30, count: 20});
element.showAbove = true;
+
await flush();
const buttons = element.shadowRoot!.querySelectorAll(
@@ -119,13 +123,18 @@
assert.include([...buttons[1].classList.values()], 'aboveButton');
});
- test('context control with block expansion at the top', async () => {
+ function prepareForBlockExpansion(syntaxTree: SyntaxBlock[]) {
element.renderPreferences!.use_block_expansion = true;
element.diff!.meta_b = ({
- syntax_tree: [],
+ syntax_tree: syntaxTree,
} as any) as DiffFileMetaInfo;
+ }
+
+ test('context control with block expansion at the top', async () => {
+ prepareForBlockExpansion([]);
element.contextGroups = createContextGroups({offset: 0, count: 20});
element.showBelow = true;
+
await flush();
const fullExpansionButtons = element.shadowRoot!.querySelectorAll(
@@ -140,7 +149,10 @@
assert.equal(fullExpansionButtons.length, 1);
assert.equal(partialExpansionButtons.length, 1);
assert.equal(blockExpansionButtons.length, 1);
- assert.equal(blockExpansionButtons[0].textContent!.trim(), '+Block');
+ assert.equal(
+ blockExpansionButtons[0].querySelector('span')!.textContent!.trim(),
+ '+Block'
+ );
assert.include(
[...blockExpansionButtons[0].classList.values()],
'belowButton'
@@ -148,14 +160,11 @@
});
test('context control with block expansion in the middle', async () => {
- element.renderPreferences!.use_block_expansion = true;
- element.diff!.meta_b = ({
- syntax_tree: [],
- } as any) as DiffFileMetaInfo;
-
+ prepareForBlockExpansion([]);
element.contextGroups = createContextGroups({offset: 10, count: 20});
element.showAbove = true;
element.showBelow = true;
+
await flush();
const fullExpansionButtons = element.shadowRoot!.querySelectorAll(
@@ -170,8 +179,14 @@
assert.equal(fullExpansionButtons.length, 1);
assert.equal(partialExpansionButtons.length, 2);
assert.equal(blockExpansionButtons.length, 2);
- assert.equal(blockExpansionButtons[0].textContent!.trim(), '+Block');
- assert.equal(blockExpansionButtons[1].textContent!.trim(), '+Block');
+ assert.equal(
+ blockExpansionButtons[0].querySelector('span')!.textContent!.trim(),
+ '+Block'
+ );
+ assert.equal(
+ blockExpansionButtons[1].querySelector('span')!.textContent!.trim(),
+ '+Block'
+ );
assert.include(
[...blockExpansionButtons[0].classList.values()],
'aboveButton'
@@ -183,12 +198,10 @@
});
test('context control with block expansion at the bottom', async () => {
- element.renderPreferences!.use_block_expansion = true;
- element.diff!.meta_b = ({
- syntax_tree: [],
- } as any) as DiffFileMetaInfo;
+ prepareForBlockExpansion([]);
element.contextGroups = createContextGroups({offset: 30, count: 20});
element.showAbove = true;
+
await flush();
const fullExpansionButtons = element.shadowRoot!.querySelectorAll(
@@ -203,10 +216,162 @@
assert.equal(fullExpansionButtons.length, 1);
assert.equal(partialExpansionButtons.length, 1);
assert.equal(blockExpansionButtons.length, 1);
- assert.equal(blockExpansionButtons[0].textContent!.trim(), '+Block');
+ assert.equal(
+ blockExpansionButtons[0].querySelector('span')!.textContent!.trim(),
+ '+Block'
+ );
assert.include(
[...blockExpansionButtons[0].classList.values()],
'aboveButton'
);
});
+
+ test('+ Block tooltip tooltip shows syntax block containing the target lines above and below', async () => {
+ prepareForBlockExpansion([
+ {
+ name: 'aSpecificFunction',
+ range: {start_line: 1, start_column: 0, end_line: 25, end_column: 0},
+ children: [],
+ },
+ {
+ name: 'anotherFunction',
+ range: {start_line: 26, start_column: 0, end_line: 50, end_column: 0},
+ children: [],
+ },
+ ]);
+ element.contextGroups = createContextGroups({offset: 10, count: 20});
+ element.showAbove = true;
+ element.showBelow = true;
+
+ await flush();
+
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ assert.equal(
+ blockExpansionButtons[0]
+ .querySelector('.breadcrumbTooltip')!
+ .textContent?.trim(),
+ 'aSpecificFunction'
+ );
+ assert.equal(
+ blockExpansionButtons[1]
+ .querySelector('.breadcrumbTooltip')!
+ .textContent?.trim(),
+ 'anotherFunction'
+ );
+ });
+
+ test('+Block tooltip shows nested syntax blocks as breadcrumbs', async () => {
+ prepareForBlockExpansion([
+ {
+ name: 'aSpecificNamespace',
+ range: {start_line: 1, start_column: 0, end_line: 200, end_column: 0},
+ children: [
+ {
+ name: 'MyClass',
+ range: {
+ start_line: 2,
+ start_column: 0,
+ end_line: 100,
+ end_column: 0,
+ },
+ children: [
+ {
+ name: 'aMethod',
+ range: {
+ start_line: 5,
+ start_column: 0,
+ end_line: 80,
+ end_column: 0,
+ },
+ children: [],
+ },
+ ],
+ },
+ ],
+ },
+ ]);
+ element.contextGroups = createContextGroups({offset: 10, count: 20});
+ element.showAbove = true;
+ element.showBelow = true;
+
+ await flush();
+
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ assert.equal(
+ blockExpansionButtons[0]
+ .querySelector('.breadcrumbTooltip')!
+ .textContent?.trim(),
+ 'aSpecificNamespace > MyClass > aMethod'
+ );
+ });
+
+ test('+Block tooltip shows (anonymous) for empty blocks', async () => {
+ prepareForBlockExpansion([
+ {
+ name: 'aSpecificNamespace',
+ range: {start_line: 1, start_column: 0, end_line: 200, end_column: 0},
+ children: [
+ {
+ name: '',
+ range: {
+ start_line: 2,
+ start_column: 0,
+ end_line: 100,
+ end_column: 0,
+ },
+ children: [
+ {
+ name: 'aMethod',
+ range: {
+ start_line: 5,
+ start_column: 0,
+ end_line: 80,
+ end_column: 0,
+ },
+ children: [],
+ },
+ ],
+ },
+ ],
+ },
+ ]);
+ element.contextGroups = createContextGroups({offset: 10, count: 20});
+ element.showAbove = true;
+ element.showBelow = true;
+ await flush();
+
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ assert.equal(
+ blockExpansionButtons[0]
+ .querySelector('.breadcrumbTooltip')!
+ .textContent?.trim(),
+ 'aSpecificNamespace > (anonymous) > aMethod'
+ );
+ });
+
+ test('+Block tooltip shows "all common lines" for empty syntax tree', async () => {
+ prepareForBlockExpansion([]);
+
+ element.contextGroups = createContextGroups({offset: 10, count: 20});
+ element.showAbove = true;
+ element.showBelow = true;
+ await flush();
+
+ const blockExpansionButtons = element.shadowRoot!.querySelectorAll(
+ '.blockExpansion gr-button'
+ );
+ // querySelector('.breadcrumbTooltip')!.textContent!.trim()
+ assert.equal(
+ blockExpansionButtons[0]
+ .querySelector('.breadcrumbTooltip')!
+ .textContent?.trim(),
+ '20 common lines'
+ );
+ });
});
