Merge "Track user interactions with manual and automatic blink buttons"
diff --git a/plugins/replication b/plugins/replication
index 75c44d0..0022a34 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 75c44d0b1dec203859112ad42074eb16839ea353
+Subproject commit 0022a34428cf8bfe4feb0935cdd20b0257bfc8a3
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index c2d3044..360ff5c 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -53,6 +53,35 @@
}
/**
+ * Represents a syntax block in a code (e.g. method, function, class, if-else).
+ */
+export 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;
+ };
+ /** Sub-blocks of the current syntax block (e.g. methods of a class) */
+ children: SyntaxBlock[];
+}
+
+/**
* The DiffFileMetaInfo entity contains meta information about a file diff.
* https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#diff-file-meta-info
*/
@@ -65,6 +94,12 @@
lines: number;
// TODO: Not documented.
language?: string;
+ /**
+ * The first level of syntax blocks tree (outline) within the current file.
+ * It contains an hierarchical structure where each block contains its
+ * sub-blocks (children).
+ */
+ syntax_tree?: SyntaxBlock[];
}
export declare type ChangeType =
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 827cbf1..b4d25a6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -122,6 +122,7 @@
DraftInfo,
isDraftThread,
isRobot,
+ isUnresolved,
} from '../../../utils/comment-util';
import {
PolymerDeepPropertyChange,
@@ -901,6 +902,13 @@
return false;
}
+ _computeShowUnresolved(threads?: CommentThread[]) {
+ // If all threads are resolved and the Comments Tab is opened then show
+ // all threads instead
+ if (!threads?.length) return true;
+ return threads.filter(thread => isUnresolved(thread)).length > 0;
+ }
+
_robotCommentCountPerPatchSet(threads: CommentThread[]) {
return threads.reduce((robotCommentCountMap, thread) => {
const comments = thread.comments;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
index ae4ad79..6025268 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.ts
@@ -570,7 +570,7 @@
logged-in="[[_loggedIn]]"
comment-tab-state="[[_tabState.commentTab]]"
only-show-robot-comments-with-human-reply=""
- unresolved-only
+ unresolved-only="[[_computeShowUnresolved(_commentThreads)]]"
show-comment-context
></gr-thread-list>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
index bb3c975..ca16ec0 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts
@@ -618,52 +618,20 @@
return changeComments.computeCommentsString(patchRange, file.__path, file);
}
- _computeDraftCount(
- changeComments?: ChangeComments,
- patchRange?: PatchRange,
- path?: string
- ) {
- if (
- changeComments === undefined ||
- patchRange === undefined ||
- path === undefined
- ) {
- return '';
- }
- return (
- changeComments.computeDraftCount({
- patchNum: patchRange.basePatchNum,
- path,
- }) +
- changeComments.computeDraftCount({
- patchNum: patchRange.patchNum,
- path,
- }) +
- changeComments.computePortedDraftCount(
- {
- patchNum: patchRange.patchNum,
- basePatchNum: patchRange.basePatchNum,
- },
- path
- )
- );
- }
-
/**
* Computes a string with the number of drafts.
*/
_computeDraftsString(
changeComments?: ChangeComments,
patchRange?: PatchRange,
- path?: string
+ file?: NormalizedFileInfo
) {
- const draftCount = this._computeDraftCount(
- changeComments,
+ const draftCount = changeComments?.computeDraftCountForFile(
patchRange,
- path
+ file
);
- if (draftCount === '') return draftCount;
- return pluralize(draftCount, 'draft');
+ if (draftCount === 0) return '';
+ return pluralize(Number(draftCount), 'draft');
}
/**
@@ -672,12 +640,11 @@
_computeDraftsStringMobile(
changeComments?: ChangeComments,
patchRange?: PatchRange,
- path?: string
+ file?: NormalizedFileInfo
) {
- const draftCount = this._computeDraftCount(
- changeComments,
+ const draftCount = changeComments?.computeDraftCountForFile(
patchRange,
- path
+ file
);
return draftCount === 0 ? '' : `${draftCount}d`;
}
@@ -688,23 +655,23 @@
_computeCommentsStringMobile(
changeComments?: ChangeComments,
patchRange?: PatchRange,
- path?: string
+ file?: NormalizedFileInfo
) {
if (
changeComments === undefined ||
patchRange === undefined ||
- path === undefined
+ file === undefined
) {
return '';
}
const commentThreadCount =
changeComments.computeCommentThreadCount({
patchNum: patchRange.basePatchNum,
- path,
+ path: file.__path,
}) +
changeComments.computeCommentThreadCount({
patchNum: patchRange.patchNum,
- path,
+ path: file.__path,
});
return commentThreadCount === 0 ? '' : `${commentThreadCount}c`;
}
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
index 59338df..40bd5bc 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.ts
@@ -423,8 +423,7 @@
<span class="drafts"
><!-- This comments ensure that span is empty when the function
returns empty string.
- -->[[_computeDraftsString(changeComments, patchRange,
- file.__path)]]<!-- This comments ensure that span is empty when
+ -->[[_computeDraftsString(changeComments, patchRange, file)]]<!-- This comments ensure that span is empty when
the function returns empty string.
--></span
>
@@ -450,14 +449,14 @@
><!-- This comments ensure that span is empty when the function
returns empty string.
-->[[_computeDraftsStringMobile(changeComments, patchRange,
- file.__path)]]<!-- This comments ensure that span is empty when
+ file)]]<!-- This comments ensure that span is empty when
the function returns empty string.
--></span
>
<span
><!--
-->[[_computeCommentsStringMobile(changeComments, patchRange,
- file.__path)]]<!--
+ file)]]<!--
--></span
>
<span class="noCommentsScreenReaderText">
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
index b8ba86c..dcc2e46 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.js
@@ -360,103 +360,103 @@
assert.equal(
element._computeCommentsStringMobile(element.changeComments, parentTo1
- , '/COMMIT_MSG'), '2c');
+ , {__path: '/COMMIT_MSG'}), '2c');
assert.equal(
element._computeCommentsStringMobile(element.changeComments, _1To2
- , '/COMMIT_MSG'), '3c');
+ , {__path: '/COMMIT_MSG'}), '3c');
assert.equal(
element._computeDraftsString(element.changeComments, parentTo1,
- 'unresolved.file'), '1 draft');
+ {__path: 'unresolved.file'}), '1 draft');
assert.equal(
element._computeDraftsString(element.changeComments, _1To2,
- 'unresolved.file'), '1 draft');
+ {__path: 'unresolved.file'}), '1 draft');
assert.equal(
element._computeDraftsStringMobile(element.changeComments, parentTo1,
- 'unresolved.file'), '1d');
+ {__path: 'unresolved.file'}), '1d');
assert.equal(
element._computeDraftsStringMobile(element.changeComments, _1To2,
- 'unresolved.file'), '1d');
+ {__path: 'unresolved.file'}), '1d');
assert.equal(
element._computeCommentsStringMobile(
element.changeComments,
parentTo1,
- 'myfile.txt'
+ {__path: 'myfile.txt'}
), '1c');
assert.equal(
element._computeCommentsStringMobile(element.changeComments, _1To2,
- 'myfile.txt'), '3c');
+ {__path: 'myfile.txt'}), '3c');
assert.equal(
element._computeDraftsString(element.changeComments, parentTo1,
- 'myfile.txt'), '');
+ {__path: 'myfile.txt'}), '');
assert.equal(
element._computeDraftsString(element.changeComments, _1To2,
- 'myfile.txt'), '');
+ {__path: 'myfile.txt'}), '');
assert.equal(
element._computeDraftsStringMobile(element.changeComments, parentTo1,
- 'myfile.txt'), '');
+ {__path: 'myfile.txt'}), '');
assert.equal(
element._computeDraftsStringMobile(element.changeComments, _1To2,
- 'myfile.txt'), '');
+ {__path: 'myfile.txt'}), '');
assert.equal(
element._computeCommentsStringMobile(
element.changeComments,
parentTo1,
- 'file_added_in_rev2.txt'
+ {__path: 'file_added_in_rev2.txt'}
), '');
assert.equal(
element._computeCommentsStringMobile(element.changeComments, _1To2,
- 'file_added_in_rev2.txt'), '');
+ {__path: 'file_added_in_rev2.txt'}), '');
assert.equal(
element._computeDraftsString(element.changeComments, parentTo1,
- 'file_added_in_rev2.txt'), '');
+ {__path: 'file_added_in_rev2.txt'}), '');
assert.equal(
element._computeDraftsString(element.changeComments, _1To2,
- 'file_added_in_rev2.txt'), '');
+ {__path: 'file_added_in_rev2.txt'}), '');
assert.equal(
element._computeDraftsStringMobile(element.changeComments, parentTo1,
- 'file_added_in_rev2.txt'), '');
+ {__path: 'file_added_in_rev2.txt'}), '');
assert.equal(
element._computeDraftsStringMobile(element.changeComments, _1To2,
- 'file_added_in_rev2.txt'), '');
+ {__path: 'file_added_in_rev2.txt'}), '');
assert.equal(
element._computeCommentsStringMobile(
element.changeComments,
parentTo2,
- '/COMMIT_MSG'
+ {__path: '/COMMIT_MSG'}
), '1c');
assert.equal(
element._computeCommentsStringMobile(element.changeComments, _1To2,
- '/COMMIT_MSG'), '3c');
+ {__path: '/COMMIT_MSG'}), '3c');
assert.equal(
element._computeDraftsString(element.changeComments, parentTo1,
- '/COMMIT_MSG'), '2 drafts');
+ {__path: '/COMMIT_MSG'}), '2 drafts');
assert.equal(
element._computeDraftsString(element.changeComments, _1To2,
- '/COMMIT_MSG'), '2 drafts');
+ {__path: '/COMMIT_MSG'}), '2 drafts');
assert.equal(
element._computeDraftsStringMobile(
element.changeComments,
parentTo1,
- '/COMMIT_MSG'
+ {__path: '/COMMIT_MSG'}
), '2d');
assert.equal(
element._computeDraftsStringMobile(element.changeComments, _1To2,
- '/COMMIT_MSG'), '2d');
+ {__path: '/COMMIT_MSG'}), '2d');
assert.equal(
element._computeCommentsStringMobile(
element.changeComments,
parentTo2,
- 'myfile.txt'
+ {__path: 'myfile.txt'}
), '2c');
assert.equal(
element._computeCommentsStringMobile(element.changeComments, _1To2,
- 'myfile.txt'), '3c');
+ {__path: 'myfile.txt'}), '3c');
assert.equal(
element._computeDraftsStringMobile(element.changeComments, parentTo2,
- 'myfile.txt'), '');
+ {__path: 'myfile.txt'}), '');
assert.equal(
element._computeDraftsStringMobile(element.changeComments, _1To2,
- 'myfile.txt'), '');
+ {__path: 'myfile.txt'}), '');
});
test('_reviewedTitle', () => {
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
index fed02a7..26af2a2 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
@@ -62,6 +62,7 @@
const LABEL_TITLE_SCORE_PATTERN = /^(-?)([A-Za-z0-9-]+?)([+-]\d+)?[.]?$/;
const UPLOADED_NEW_PATCHSET_PATTERN = /Uploaded patch set (\d+)./;
const MERGED_PATCHSET_PATTERN = /(\d+) is the latest approved patch-set/;
+const VOTE_RESET_TEXT = '0 (vote reset)';
declare global {
interface HTMLElementTagNameMap {
@@ -466,7 +467,7 @@
)
.map(ms => {
const label = ms?.[2];
- const value = ms?.[1] === '-' ? 'removed' : ms?.[3];
+ const value = ms?.[1] === '-' ? VOTE_RESET_TEXT : ms?.[3];
return {label, value};
});
}
@@ -479,7 +480,7 @@
if (!score.value) {
return '';
}
- if (score.value === 'removed') {
+ if (score.value.includes(VOTE_RESET_TEXT)) {
return 'removed';
}
const classes = [];
diff --git a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
index 8cc7a3f..73afde8 100644
--- a/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
+++ b/polygerrit-ui/app/elements/diff/gr-comment-api/gr-comment-api.ts
@@ -51,6 +51,7 @@
import {appContext} from '../../../services/app-context';
import {CommentSide, Side} from '../../../constants/constants';
import {pluralize} from '../../../utils/string-util';
+import {NormalizedFileInfo} from '../../change/gr-file-list/gr-file-list';
export type CommentIdToCommentThreadMap = {
[urlEncodedCommentId: string]: CommentThread;
@@ -522,6 +523,33 @@
.length;
}
+ computeDraftCountForFile(patchRange?: PatchRange, file?: NormalizedFileInfo) {
+ if (patchRange === undefined || file === undefined) {
+ return 0;
+ }
+ const getCommentForPath = (path?: string) => {
+ if (!path) return 0;
+ return (
+ this.computeDraftCount({
+ patchNum: patchRange.basePatchNum,
+ path,
+ }) +
+ this.computeDraftCount({
+ patchNum: patchRange.patchNum,
+ path,
+ }) +
+ this.computePortedDraftCount(
+ {
+ patchNum: patchRange.patchNum,
+ basePatchNum: patchRange.basePatchNum,
+ },
+ path
+ )
+ );
+ };
+ return getCommentForPath(file.__path) + getCommentForPath(file.old_path);
+ }
+
/**
* @param includeUnmodified Included unmodified status of the file in the
* comment string or not. For files we opt of chip instead of a string.
@@ -537,6 +565,14 @@
if (!patchRange) return '';
const threads = this.getThreadsBySideForFile({path}, patchRange);
+ if (changeFileInfo?.old_path) {
+ threads.push(
+ ...this.getThreadsBySideForFile(
+ {path: changeFileInfo.old_path},
+ patchRange
+ )
+ );
+ }
const commentThreadCount = threads.filter(thread => !isDraftThread(thread))
.length;
const unresolvedCount = threads.reduce((cnt, thread) => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
index 7c01a95..5ba3606 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
@@ -97,8 +97,13 @@
return section;
}
+ let diffInfo;
+ let renderPrefs;
+
setup(() => {
- builder = new GrDiffBuilder({content: []}, prefs, null, []);
+ diffInfo = {content: []};
+ renderPrefs = {};
+ builder = new GrDiffBuilder(diffInfo, prefs, null, [], renderPrefs);
});
test('no +10 buttons for 10 or less lines', () => {
@@ -149,6 +154,70 @@
assert.include([...buttons[0].classList.values()], 'aboveButton');
assert.include([...buttons[1].classList.values()], 'aboveButton');
});
+
+ suite('with block expansion', () => {
+ setup(() => {
+ builder._numLinesLeft = 50;
+ renderPrefs.use_block_expansion = true;
+ diffInfo.meta_b = {
+ syntax_tree: [],
+ };
+ });
+
+ test('context control with block expansion at the top', () => {
+ const section = createContextSectionForGroups({offset: 0, count: 20});
+
+ const fullExpansionButtons = section
+ .querySelectorAll('.fullExpansion gr-button');
+ const partialExpansionButtons = section
+ .querySelectorAll('.partialExpansion gr-button');
+ const blockExpansionButtons = section
+ .querySelectorAll('.blockExpansion gr-button');
+ assert.equal(fullExpansionButtons.length, 1);
+ assert.equal(partialExpansionButtons.length, 1);
+ assert.equal(blockExpansionButtons.length, 1);
+ assert.equal(blockExpansionButtons[0].textContent, '+Block');
+ assert.include([...blockExpansionButtons[0].classList.values()],
+ 'belowButton');
+ });
+
+ test('context control in the middle', () => {
+ const section = createContextSectionForGroups({offset: 10, count: 20});
+
+ const fullExpansionButtons = section
+ .querySelectorAll('.fullExpansion gr-button');
+ const partialExpansionButtons = section
+ .querySelectorAll('.partialExpansion gr-button');
+ const blockExpansionButtons = section
+ .querySelectorAll('.blockExpansion gr-button');
+ assert.equal(fullExpansionButtons.length, 1);
+ assert.equal(partialExpansionButtons.length, 2);
+ assert.equal(blockExpansionButtons.length, 2);
+ assert.equal(blockExpansionButtons[0].textContent, '+Block');
+ assert.equal(blockExpansionButtons[1].textContent, '+Block');
+ assert.include([...blockExpansionButtons[0].classList.values()],
+ 'aboveButton');
+ assert.include([...blockExpansionButtons[1].classList.values()],
+ 'belowButton');
+ });
+
+ test('context control at the bottom', () => {
+ const section = createContextSectionForGroups({offset: 30, count: 20});
+
+ const fullExpansionButtons = section
+ .querySelectorAll('.fullExpansion gr-button');
+ const partialExpansionButtons = section
+ .querySelectorAll('.partialExpansion gr-button');
+ const blockExpansionButtons = section
+ .querySelectorAll('.blockExpansion gr-button');
+ assert.equal(fullExpansionButtons.length, 1);
+ assert.equal(partialExpansionButtons.length, 1);
+ assert.equal(blockExpansionButtons.length, 1);
+ assert.equal(blockExpansionButtons[0].textContent, '+Block');
+ assert.include([...blockExpansionButtons[0].classList.values()],
+ 'aboveButton');
+ });
+ });
});
test('newlines 1', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
index c8b69f0..d5e6ecd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.ts
@@ -20,6 +20,7 @@
DiffContextExpandedExternalDetail,
MovedLinkClickedEventDetail,
RenderPreferences,
+ SyntaxBlock,
} from '../../../api/diff';
import {getBaseUrl} from '../../../utils/url-util';
import {GrDiffLine, GrDiffLineType, LineNumber} from '../gr-diff/gr-diff-line';
@@ -73,6 +74,19 @@
}
}
+function findMostNestedContainingBlock(
+ lineNum: number,
+ blocks?: SyntaxBlock[]
+): SyntaxBlock | undefined {
+ const containingBlock = blocks?.find(
+ ({range}) => range.start_line < lineNum && range.end_line > lineNum
+ );
+ const containingChildBlock = containingBlock
+ ? findMostNestedContainingBlock(lineNum, containingBlock?.children)
+ : undefined;
+ return containingChildBlock || containingBlock;
+}
+
export abstract class GrDiffBuilder {
private readonly _diff: DiffInfo;
@@ -306,6 +320,7 @@
);
}
+ // TODO(renanoliveira): Move context controls to polymer component (or at least a separate class).
_createContextControls(
section: HTMLElement,
contextGroups: GrDiffGroup[],
@@ -314,9 +329,9 @@
const leftStart = contextGroups[0].lineRange.left.start_line;
const leftEnd =
contextGroups[contextGroups.length - 1].lineRange.left.end_line;
- const numLines = leftEnd - leftStart + 1;
-
- if (numLines === 0) console.error('context group without lines');
+ const rightStart = contextGroups[0].lineRange.right.start_line;
+ const rightEnd =
+ contextGroups[contextGroups.length - 1].lineRange.right.end_line;
const firstGroupIsSkipped = !!contextGroups[0].skip;
const lastGroupIsSkipped = !!contextGroups[contextGroups.length - 1].skip;
@@ -335,7 +350,8 @@
contextGroups,
showAbove,
showBelow,
- numLines
+ rightStart,
+ rightEnd
)
);
if (showBelow) {
@@ -354,8 +370,12 @@
contextGroups: GrDiffGroup[],
showAbove: boolean,
showBelow: boolean,
- numLines: number
+ rightStart: number,
+ rightEnd: number
): HTMLElement {
+ const numLines = rightEnd - rightStart + 1;
+ if (numLines === 0) console.error('context group without lines');
+
const row = this._createElement('tr', 'contextDivider');
if (!(showAbove && showBelow)) {
row.classList.add('collapsed');
@@ -364,13 +384,55 @@
const element = this._createElement('td', 'dividerCell');
row.appendChild(element);
- const showAllContainer = this._createElement('div', 'aboveBelowButtons');
+ const showAllContainer = this._createExpandAllButtonContainer(
+ section,
+ contextGroups,
+ showAbove,
+ showBelow,
+ numLines
+ );
element.appendChild(showAllContainer);
+ const showPartialLinks = numLines > PARTIAL_CONTEXT_AMOUNT;
+ if (showPartialLinks) {
+ const partialExpansionContainer = this._createPartialExpansionButtons(
+ section,
+ contextGroups,
+ showAbove,
+ showBelow,
+ numLines
+ );
+ if (partialExpansionContainer) {
+ element.appendChild(partialExpansionContainer);
+ }
+ const blockExpansionContainer = this._createBlockExpansionButtons(
+ section,
+ contextGroups,
+ showAbove,
+ showBelow,
+ rightStart,
+ rightEnd,
+ numLines
+ );
+ if (blockExpansionContainer) {
+ element.appendChild(blockExpansionContainer);
+ }
+ }
+ return row;
+ }
+
+ private _createExpandAllButtonContainer(
+ section: HTMLElement,
+ contextGroups: GrDiffGroup[],
+ showAbove: boolean,
+ showBelow: boolean,
+ numLines: number
+ ) {
const showAllButton = this._createContextButton(
ContextButtonType.ALL,
section,
contextGroups,
+ numLines,
numLines
);
showAllButton.classList.add(
@@ -380,61 +442,131 @@
? 'aboveButton'
: 'belowButton'
);
+ const showAllContainer = this._createElement(
+ 'div',
+ 'aboveBelowButtons fullExpansion'
+ );
showAllContainer.appendChild(showAllButton);
+ return showAllContainer;
+ }
- const showPartialLinks = numLines > PARTIAL_CONTEXT_AMOUNT;
- if (showPartialLinks) {
- const container = this._createElement('div', 'aboveBelowButtons');
- if (showAbove) {
- container.appendChild(
- this._createContextButton(
- ContextButtonType.ABOVE,
- section,
- contextGroups,
- numLines
- )
- );
- }
- if (showBelow) {
- container.appendChild(
- this._createContextButton(
- ContextButtonType.BELOW,
- section,
- contextGroups,
- numLines
- )
- );
- }
- element.appendChild(container);
- if (this._renderPrefs?.use_block_expansion) {
- const blockExpansionContainer = this._createElement(
- 'div',
- 'aboveBelowButtons'
- );
- if (showAbove) {
- blockExpansionContainer.appendChild(
- this._createContextButton(
- ContextButtonType.BLOCK_ABOVE,
- section,
- contextGroups,
- numLines
- )
- );
- }
- if (showBelow) {
- blockExpansionContainer.appendChild(
- this._createContextButton(
- ContextButtonType.BLOCK_BELOW,
- section,
- contextGroups,
- numLines
- )
- );
- }
- element.appendChild(blockExpansionContainer);
+ private _createPartialExpansionButtons(
+ section: HTMLElement,
+ contextGroups: GrDiffGroup[],
+ showAbove: boolean,
+ showBelow: boolean,
+ numLines: number
+ ) {
+ let aboveButton;
+ let belowButton;
+ if (showAbove) {
+ aboveButton = this._createContextButton(
+ ContextButtonType.ABOVE,
+ section,
+ contextGroups,
+ numLines,
+ PARTIAL_CONTEXT_AMOUNT
+ );
+ }
+ if (showBelow) {
+ belowButton = this._createContextButton(
+ ContextButtonType.BELOW,
+ section,
+ contextGroups,
+ numLines,
+ PARTIAL_CONTEXT_AMOUNT
+ );
+ }
+ if (aboveButton || belowButton) {
+ const partialExpansionContainer = this._createElement(
+ 'div',
+ 'aboveBelowButtons partialExpansion'
+ );
+ aboveButton && partialExpansionContainer.appendChild(aboveButton);
+ belowButton && partialExpansionContainer.appendChild(belowButton);
+ return partialExpansionContainer;
+ }
+ return undefined;
+ }
+
+ private _createBlockExpansionButtons(
+ section: HTMLElement,
+ contextGroups: GrDiffGroup[],
+ showAbove: boolean,
+ showBelow: boolean,
+ rightStart: number,
+ rightEnd: number,
+ numLines: number
+ ) {
+ if (!this._renderPrefs?.use_block_expansion) {
+ return undefined;
+ }
+ let aboveBlockButton;
+ let belowBlockButton;
+ const rightSyntaxTree = this._diff.meta_b.syntax_tree;
+ if (showAbove) {
+ aboveBlockButton = this._createBlockButton(
+ section,
+ contextGroups,
+ ContextButtonType.BLOCK_ABOVE,
+ numLines,
+ rightStart - 1,
+ rightSyntaxTree
+ );
+ }
+ if (showBelow) {
+ belowBlockButton = this._createBlockButton(
+ section,
+ contextGroups,
+ ContextButtonType.BLOCK_BELOW,
+ numLines,
+ rightEnd + 1,
+ rightSyntaxTree
+ );
+ }
+ if (aboveBlockButton || belowBlockButton) {
+ const blockExpansionContainer = this._createElement(
+ 'div',
+ 'blockExpansion aboveBelowButtons'
+ );
+ aboveBlockButton && blockExpansionContainer.appendChild(aboveBlockButton);
+ belowBlockButton && blockExpansionContainer.appendChild(belowBlockButton);
+ return blockExpansionContainer;
+ }
+ return undefined;
+ }
+
+ private _createBlockButton(
+ section: HTMLElement,
+ contextGroups: GrDiffGroup[],
+ buttonType: ContextButtonType,
+ numLines: number,
+ referenceLine: number,
+ syntaxTree?: SyntaxBlock[]
+ ) {
+ const containingBlock = findMostNestedContainingBlock(
+ referenceLine,
+ syntaxTree
+ );
+ let linesToExpand = numLines;
+ if (containingBlock) {
+ const {range} = containingBlock;
+ const targetLine =
+ buttonType === ContextButtonType.BLOCK_ABOVE
+ ? range.end_line
+ : range.start_line;
+ const distanceToTargetLine = Math.abs(targetLine - referenceLine);
+ if (distanceToTargetLine < numLines) {
+ linesToExpand = distanceToTargetLine;
}
}
- return row;
+ return this._createContextButton(
+ buttonType,
+ section,
+ contextGroups,
+ numLines,
+ linesToExpand
+ );
}
/**
@@ -469,10 +601,9 @@
type: ContextButtonType,
section: HTMLElement,
contextGroups: GrDiffGroup[],
- numLines: number
+ numLines: number,
+ linesToExpand: number
) {
- const linesToExpand =
- type === ContextButtonType.ALL ? numLines : PARTIAL_CONTEXT_AMOUNT;
const button = this._createElement('gr-button', 'showContext');
button.classList.add('contextControlButton');
button.setAttribute('link', 'true');
diff --git a/polygerrit-ui/app/utils/path-list-util.ts b/polygerrit-ui/app/utils/path-list-util.ts
index dda6031..fd922fc 100644
--- a/polygerrit-ui/app/utils/path-list-util.ts
+++ b/polygerrit-ui/app/utils/path-list-util.ts
@@ -64,6 +64,8 @@
return file === SpecialFilePath.PATCHSET_LEVEL_COMMENTS;
}
+// In case there are files with comments on them but they are unchanged, then
+// we explicitly displays the file to render the comments with Unchanged status
export function addUnmodifiedFiles(
files: {[filename: string]: FileInfo},
commentedPaths: {[fileName: string]: boolean}
@@ -73,6 +75,18 @@
if (hasOwnProperty(files, commentedPath) || shouldHideFile(commentedPath)) {
return;
}
+
+ // if file is Renamed but has comments, then do not show the entry for the
+ // old file path name
+ if (
+ Object.values(files).some(
+ file =>
+ file.status === FileInfoStatus.RENAMED &&
+ file.old_path === commentedPath
+ )
+ ) {
+ return;
+ }
// TODO(TS): either change FileInfo to mark delta and size optional
// or fill in 0 here
files[commentedPath] = {
