Merge "Suggest push to refs/for/BRANCH if lacking push permission"
diff --git a/java/com/google/gerrit/server/account/AccountsUpdate.java b/java/com/google/gerrit/server/account/AccountsUpdate.java
index b46d10d..5a74047 100644
--- a/java/com/google/gerrit/server/account/AccountsUpdate.java
+++ b/java/com/google/gerrit/server/account/AccountsUpdate.java
@@ -428,6 +428,11 @@
* Updates multiple different accounts atomically. This will only store a single new value (aka
* set of all external IDs of the host) in the external ID cache, which is important for storage
* economy. All {@code updates} must be for different accounts.
+ *
+ * <p>NOTE on error handling: Since updates are executed in multiple stages, with some stages
+ * resulting from the union of all individual updates, we cannot point to the update that caused
+ * the error. Callers should be aware that a single "update of death" (or a set of updates that
+ * together have this property) will always prevent the entire batch from being executed.
*/
public ImmutableList<Optional<AccountState>> updateBatch(List<UpdateArguments> updates)
throws IOException, ConfigInvalidException {
diff --git a/java/com/google/gerrit/server/restapi/change/CommentJson.java b/java/com/google/gerrit/server/restapi/change/CommentJson.java
index edc8fcf..81b6fb3 100644
--- a/java/com/google/gerrit/server/restapi/change/CommentJson.java
+++ b/java/com/google/gerrit/server/restapi/change/CommentJson.java
@@ -190,7 +190,7 @@
return CommentContextKey.builder()
.project(project)
.changeId(changeId)
- .id(r.id)
+ .id(Url.decode(r.id)) // We reverse the encoding done while filling comment info
.path(r.path)
.patchset(r.patchSet)
.contextPadding(contextPadding)
diff --git a/polygerrit-ui/app/api/diff.ts b/polygerrit-ui/app/api/diff.ts
index 6e142d4..f6b3aa0 100644
--- a/polygerrit-ui/app/api/diff.ts
+++ b/polygerrit-ui/app/api/diff.ts
@@ -303,12 +303,8 @@
}
export declare type ImageDiffAction =
- | {
- type: 'overview-image-clicked';
- }
- | {
- type: 'overview-frame-dragged';
- }
+ | {type: 'overview-image-clicked'}
+ | {type: 'overview-frame-dragged'}
| {type: 'magnifier-clicked'}
| {type: 'magnifier-dragged'}
| {type: 'version-switcher-clicked'; button: 'base' | 'revision' | 'switch'}
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 807d0cd..c6d2ee0 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
@@ -1252,9 +1252,13 @@
this.$.fileList.collapseAllDiffs();
this._patchRange = patchRange;
+ const patchKnown =
+ !patchRange.patchNum ||
+ (this._allPatchSets ?? []).some(ps => ps.num === patchRange.patchNum);
+
// If the change has already been loaded and the parameter change is only
// in the patch range, then don't do a full reload.
- if (!changeChanged && patchChanged) {
+ if (!changeChanged && patchChanged && patchKnown) {
if (!patchRange.patchNum) {
patchRange.patchNum = computeLatestPatchNum(this._allPatchSets);
rightPatchNumChanged = true;
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
index 877bdd6..3d51ac6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts
@@ -1633,6 +1633,13 @@
assert.isTrue(reloadStub.calledOnce);
element._initialLoadComplete = true;
+ element._change = {
+ ...createChangeViewChange(),
+ revisions: {
+ rev1: createRevision(1),
+ rev2: createRevision(2),
+ },
+ };
value.basePatchNum = 1 as BasePatchSetNum;
value.patchNum = 2 as RevisionPatchSetNum;
@@ -1661,6 +1668,13 @@
element._paramsChanged(value);
element._initialLoadComplete = true;
+ element._change = {
+ ...createChangeViewChange(),
+ revisions: {
+ rev1: createRevision(1),
+ rev2: createRevision(2),
+ },
+ };
value.basePatchNum = 1 as BasePatchSetNum;
value.patchNum = 2 as RevisionPatchSetNum;
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 26af2a2..1711499 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.ts
@@ -184,7 +184,7 @@
type: String,
computed:
'_computeMessageContentExpanded(message.message,' +
- ' message.accountsInMessage,' +
+ ' message.accounts_in_message,' +
' message.tag)',
})
_messageContentExpanded = '';
@@ -193,7 +193,7 @@
type: String,
computed:
'_computeMessageContentCollapsed(message.message,' +
- ' message.accountsInMessage,' +
+ ' message.accounts_in_message,' +
' message.tag,' +
' message.commentThreads)',
})
diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
index 8d5bc33..5595d15 100644
--- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
+++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.ts
@@ -21,13 +21,7 @@
import '../../plugins/gr-endpoint-slot/gr-endpoint-slot';
import {classMap} from 'lit-html/directives/class-map';
import {GrLitElement} from '../../lit/gr-lit-element';
-import {
- customElement,
- property,
- css,
- internalProperty,
- TemplateResult,
-} from 'lit-element';
+import {customElement, property, css, state, TemplateResult} from 'lit-element';
import {sharedStyles} from '../../../styles/shared-styles';
import {
SubmittedTogetherInfo,
@@ -76,22 +70,22 @@
@property()
mergeable?: boolean;
- @internalProperty()
+ @state()
submittedTogether?: SubmittedTogetherInfo = {
changes: [],
non_visible_changes: 0,
};
- @internalProperty()
+ @state()
relatedChanges: RelatedChangeAndCommitInfo[] = [];
- @internalProperty()
+ @state()
conflictingChanges: ChangeInfo[] = [];
- @internalProperty()
+ @state()
cherryPickChanges: ChangeInfo[] = [];
- @internalProperty()
+ @state()
sameTopicChanges: ChangeInfo[] = [];
private readonly restApiService = appContext.restApiService;
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-results.ts b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
index 1c60161..c74f6f9 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-results.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-results.ts
@@ -20,10 +20,10 @@
import {
css,
customElement,
- internalProperty,
property,
PropertyValues,
query,
+ state,
TemplateResult,
} from 'lit-element';
import {GrLitElement} from '../lit/gr-lit-element';
@@ -471,7 +471,7 @@
@query('#filterInput')
filterInput?: HTMLInputElement;
- @internalProperty()
+ @state()
filterRegExp = new RegExp('');
/** All runs. Shown should only the selected/filtered ones. */
@@ -511,7 +511,7 @@
>();
/** Maintains the state of which result sections should show all results. */
- @internalProperty()
+ @state()
isShowAll: Map<Category, boolean> = new Map();
/**
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
index de9c5fb..8ff42ee 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-runs.ts
@@ -20,10 +20,10 @@
import {
css,
customElement,
- internalProperty,
property,
PropertyValues,
query,
+ state,
} from 'lit-element';
import {GrLitElement} from '../lit/gr-lit-element';
import {Action, Link, RunStatus} from '../../api/checks';
@@ -326,7 +326,7 @@
@query('#filterInput')
filterInput?: HTMLInputElement;
- @internalProperty()
+ @state()
filterRegExp = new RegExp('');
@property()
diff --git a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
index 418598b..b28596a 100644
--- a/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
+++ b/polygerrit-ui/app/elements/checks/gr-checks-tab.ts
@@ -15,13 +15,7 @@
* limitations under the License.
*/
import {html} from 'lit-html';
-import {
- css,
- customElement,
- internalProperty,
- property,
- PropertyValues,
-} from 'lit-element';
+import {css, customElement, property, PropertyValues, state} from 'lit-element';
import {GrLitElement} from '../lit/gr-lit-element';
import {Action} from '../../api/checks';
import {
@@ -64,11 +58,11 @@
@property()
changeNum: NumericChangeId | undefined = undefined;
- @internalProperty()
+ @state()
selectedRuns: string[] = [];
/** Maps checkName to selected attempt number. `undefined` means `latest`. */
- @internalProperty()
+ @state()
selectedAttempts: Map<string, number | undefined> = new Map<
string,
number | undefined
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'
+ );
+ });
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
index 2a4b250..9a98a21 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-image-viewer.ts
@@ -29,11 +29,11 @@
css,
customElement,
html,
- internalProperty,
LitElement,
property,
PropertyValues,
query,
+ state,
} from 'lit-element';
import {classMap} from 'lit-html/directives/class-map';
import {StyleInfo, styleMap} from 'lit-html/directives/style-map';
@@ -66,23 +66,23 @@
// URL for the image to use as revision.
@property({type: String}) revisionUrl = '';
- @internalProperty() protected baseSelected = true;
+ @state() protected baseSelected = true;
- @internalProperty() protected scaledSelected = true;
+ @state() protected scaledSelected = true;
- @internalProperty() protected followMouse = false;
+ @state() protected followMouse = false;
- @internalProperty() protected scale = 1;
+ @state() protected scale = 1;
- @internalProperty() protected checkerboardSelected = true;
+ @state() protected checkerboardSelected = true;
- @internalProperty() protected backgroundColor = '';
+ @state() protected backgroundColor = '';
- @internalProperty() protected automaticBlink = false;
+ @state() protected automaticBlink = false;
- @internalProperty() protected automaticBlinkShown = false;
+ @state() protected automaticBlinkShown = false;
- @internalProperty() protected zoomedImageStyle: StyleInfo = {};
+ @state() protected zoomedImageStyle: StyleInfo = {};
@query('.imageArea') protected imageArea!: HTMLDivElement;
@@ -94,16 +94,16 @@
private imageSize: Dimensions = {width: 0, height: 0};
- @internalProperty()
+ @state()
protected magnifierSize: Dimensions = {width: 0, height: 0};
- @internalProperty()
+ @state()
protected magnifierFrame: Rect = {
origin: {x: 0, y: 0},
dimensions: {width: 0, height: 0},
};
- @internalProperty()
+ @state()
protected overviewFrame: Rect = {
origin: {x: 0, y: 0},
dimensions: {width: 0, height: 0},
@@ -118,7 +118,7 @@
2,
];
- @internalProperty() protected grabbing = false;
+ @state() protected grabbing = false;
private ownsMouseDown = false;
@@ -745,16 +745,20 @@
}
mouseupMagnifier(event: MouseEvent) {
+ if (!this.ownsMouseDown) return;
+ this.grabbing = false;
+ this.ownsMouseDown = false;
const offsetX = event.clientX - this.pointerOnDown.x;
const offsetY = event.clientY - this.pointerOnDown.y;
const distance = Math.max(Math.abs(offsetX), Math.abs(offsetY));
// Consider very short drags as clicks. These tend to happen more often on
// external mice.
- if (this.ownsMouseDown && distance < DRAG_DEAD_ZONE_PIXELS) {
+ if (distance < DRAG_DEAD_ZONE_PIXELS) {
this.toggleImage();
+ this.dispatchEvent(createEvent({type: 'magnifier-clicked'}));
+ } else {
+ this.dispatchEvent(createEvent({type: 'magnifier-dragged'}));
}
- this.grabbing = false;
- this.ownsMouseDown = false;
}
mousemoveMagnifier(event: MouseEvent) {
@@ -793,8 +797,10 @@
}
mouseleaveMagnifier() {
+ if (!this.ownsMouseDown) return;
this.grabbing = false;
this.ownsMouseDown = false;
+ this.dispatchEvent(createEvent({type: 'magnifier-dragged'}));
}
dragstartMagnifier(event: DragEvent) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts
index d7b6916..9439dca 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-overview-image.ts
@@ -18,11 +18,11 @@
css,
customElement,
html,
- internalProperty,
LitElement,
property,
PropertyValues,
query,
+ state,
} from 'lit-element';
import {StyleInfo, styleMap} from 'lit-html/directives/style-map';
import {ImageDiffAction} from '../../../api/diff';
@@ -45,13 +45,13 @@
@property({type: Object})
frameRect: Rect = {origin: {x: 0, y: 0}, dimensions: {width: 0, height: 0}};
- @internalProperty() protected contentStyle: StyleInfo = {};
+ @state() protected contentStyle: StyleInfo = {};
- @internalProperty() protected contentTransformStyle: StyleInfo = {};
+ @state() protected contentTransformStyle: StyleInfo = {};
- @internalProperty() protected frameStyle: StyleInfo = {};
+ @state() protected frameStyle: StyleInfo = {};
- @internalProperty() protected dragging = false;
+ @state() protected dragging = false;
@query('.content-box') protected contentBox!: HTMLDivElement;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-zoomed-image.ts b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-zoomed-image.ts
index a14a9cc..4558dda 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-zoomed-image.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-image-viewer/gr-zoomed-image.ts
@@ -18,10 +18,10 @@
css,
customElement,
html,
- internalProperty,
LitElement,
property,
PropertyValues,
+ state,
} from 'lit-element';
import {StyleInfo, styleMap} from 'lit-html/directives/style-map';
import {Rect} from './util';
@@ -41,7 +41,7 @@
@property({type: Object})
frameRect: Rect = {origin: {x: 0, y: 0}, dimensions: {width: 0, height: 0}};
- @internalProperty() protected imageStyles: StyleInfo = {};
+ @state() protected imageStyles: StyleInfo = {};
static styles = css`
:host {
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
index 837a795..62589f8 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
@@ -102,7 +102,7 @@
import {dedupingMixin} from '@polymer/polymer/lib/utils/mixin';
import {property} from '@polymer/decorators';
import {PolymerElement} from '@polymer/polymer';
-import {Constructor} from '../../utils/common-util';
+import {check, Constructor} from '../../utils/common-util';
import {getKeyboardEvent, isModifierPressed} from '../../utils/dom-util';
import {
CustomKeyboardEvent,
@@ -222,11 +222,6 @@
viewMap?: Map<ShortcutSection, SectionView>
) => void;
-interface ShortcutEnabledElement extends PolymerElement {
- // TODO: should replace with Map so we can have proper type here
- keyboardShortcuts(): {[shortcut: string]: string};
-}
-
interface ShortcutHelpItem {
shortcut: Shortcut;
text: string;
@@ -558,14 +553,9 @@
return this.bindings.get(shortcut);
}
- attachHost(host: PolymerElement | ShortcutEnabledElement) {
- if (!('keyboardShortcuts' in host)) {
- return;
- }
- const shortcuts = host.keyboardShortcuts();
- this.activeHosts.set(host, new Map(Object.entries(shortcuts)));
+ attachHost(host: PolymerElement, shortcuts: Map<string, string>) {
+ this.activeHosts.set(host, shortcuts);
this.notifyListeners();
- return shortcuts;
}
detachHost(host: PolymerElement) {
@@ -788,6 +778,12 @@
private readonly restApiService = appContext.restApiService;
+ /** Used to disable shortcuts when the element is not visible. */
+ private observer?: IntersectionObserver;
+
+ /** Are shortcuts currently enabled? True only when element is visible. */
+ private bindingsEnabled = false;
+
modifierPressed(event: CustomKeyboardEvent) {
/* We are checking for g/v as modifiers pressed. There are cases such as
* pressing v and then /, where we want the handler for / to be triggered.
@@ -902,21 +898,62 @@
/** @override */
connectedCallback() {
super.connectedCallback();
-
this.restApiService.getPreferences().then(prefs => {
if (prefs?.disable_keyboard_shortcuts) {
this._disableKeyboardShortcuts = true;
}
});
+ this.createVisibilityObserver();
+ this.enableBindings();
+ }
- const shortcuts = shortcutManager.attachHost(this);
- if (!shortcuts) {
- return;
- }
+ /** @override */
+ disconnectedCallback() {
+ this.destroyVisibilityObserver();
+ this.disableBindings();
+ super.disconnectedCallback();
+ }
- for (const key of Object.keys(shortcuts)) {
- // TODO(TS): not needed if convert shortcuts to Map
- this._addOwnKeyBindings(key as Shortcut, shortcuts[key]);
+ /**
+ * Creates an intersection observer that enables bindings when the
+ * element is visible and disables them when the element is hidden.
+ */
+ private createVisibilityObserver() {
+ if (!this.hasKeyboardShortcuts()) return;
+ if (this.observer) return;
+ this.observer = new IntersectionObserver(entries => {
+ check(entries.length === 1, 'Expected one observer entry.');
+ const isVisible = entries[0].isIntersecting;
+ if (isVisible) {
+ this.enableBindings();
+ } else {
+ this.disableBindings();
+ }
+ });
+ this.observer.observe(this);
+ }
+
+ private destroyVisibilityObserver() {
+ if (this.observer) this.observer.unobserve(this);
+ }
+
+ /**
+ * Enables all the shortcuts returned by keyboardShortcuts().
+ * This is a private method being called when the element becomes
+ * connected or visible.
+ */
+ private enableBindings() {
+ if (!this.hasKeyboardShortcuts()) return;
+ if (this.bindingsEnabled) return;
+ this.bindingsEnabled = true;
+
+ const shortcuts = new Map<string, string>(
+ Object.entries(this.keyboardShortcuts())
+ );
+ shortcutManager.attachHost(this, shortcuts);
+
+ for (const [key, value] of shortcuts.entries()) {
+ this._addOwnKeyBindings(key as Shortcut, value);
}
// each component that uses this behaviour must be aware if go key is
@@ -941,12 +978,21 @@
}
}
- /** @override */
- disconnectedCallback() {
+ /**
+ * Disables all the shortcuts returned by keyboardShortcuts().
+ * This is a private method being called when the element becomes
+ * disconnected or invisible.
+ */
+ private disableBindings() {
+ if (!this.bindingsEnabled) return;
+ this.bindingsEnabled = false;
if (shortcutManager.detachHost(this)) {
this.removeOwnKeyBindings();
}
- super.disconnectedCallback();
+ }
+
+ private hasKeyboardShortcuts() {
+ return Object.entries(this.keyboardShortcuts()).length > 0;
}
keyboardShortcuts() {
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
index b22b8d8..d5e7fe7 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin_test.js
@@ -181,13 +181,7 @@
mapToObject(mgr.activeShortcutsBySection()),
{});
- mgr.attachHost({
- keyboardShortcuts() {
- return {
- [Shortcut.NEXT_FILE]: null,
- };
- },
- });
+ mgr.attachHost({}, new Map([[Shortcut.NEXT_FILE, null]]));
assert.deepEqual(
mapToObject(mgr.activeShortcutsBySection()),
{
@@ -196,13 +190,7 @@
],
});
- mgr.attachHost({
- keyboardShortcuts() {
- return {
- [Shortcut.NEXT_LINE]: null,
- };
- },
- });
+ mgr.attachHost({}, new Map([[Shortcut.NEXT_LINE, null]]));
assert.deepEqual(
mapToObject(mgr.activeShortcutsBySection()),
{
@@ -214,14 +202,10 @@
],
});
- mgr.attachHost({
- keyboardShortcuts() {
- return {
- [Shortcut.SEARCH]: null,
- [Shortcut.GO_TO_OPENED_CHANGES]: null,
- };
- },
- });
+ mgr.attachHost({}, new Map([
+ [Shortcut.SEARCH, null],
+ [Shortcut.GO_TO_OPENED_CHANGES, null],
+ ]));
assert.deepEqual(
mapToObject(mgr.activeShortcutsBySection()),
{
@@ -254,17 +238,13 @@
assert.deepEqual(mapToObject(mgr.directoryView()), {});
- mgr.attachHost({
- keyboardShortcuts() {
- return {
- [Shortcut.GO_TO_OPENED_CHANGES]: null,
- [Shortcut.NEXT_FILE]: null,
- [Shortcut.NEXT_LINE]: null,
- [Shortcut.SAVE_COMMENT]: null,
- [Shortcut.SEARCH]: null,
- };
- },
- });
+ mgr.attachHost({}, new Map([
+ [Shortcut.GO_TO_OPENED_CHANGES, null],
+ [Shortcut.NEXT_FILE, null],
+ [Shortcut.NEXT_LINE, null],
+ [Shortcut.SAVE_COMMENT, null],
+ [Shortcut.SEARCH, null],
+ ]));
assert.deepEqual(
mapToObject(mgr.directoryView()),
{
diff --git a/polygerrit-ui/app/package.json b/polygerrit-ui/app/package.json
index d250537..6d5e475 100644
--- a/polygerrit-ui/app/package.json
+++ b/polygerrit-ui/app/package.json
@@ -36,7 +36,7 @@
"@webcomponents/webcomponentsjs": "^1.3.3",
"ba-linkify": "file:../../lib/ba-linkify/src/",
"codemirror-minified": "^5.60.0",
- "lit-element": "^2.4.0",
+ "lit-element": "^2.5.1",
"page": "^1.11.6",
"polymer-bridges": "file:../../polymer-bridges/",
"polymer-resin": "^2.0.1",
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 70cddc0..30eb658 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -529,7 +529,7 @@
real_author?: AccountInfo;
date: Timestamp;
message: string;
- accountsInMessage?: AccountInfo[];
+ accounts_in_message?: AccountInfo[];
tag?: ReviewInputTag;
_revision_number?: PatchSetNum;
}
diff --git a/polygerrit-ui/app/yarn.lock b/polygerrit-ui/app/yarn.lock
index fa35f288..543017a 100644
--- a/polygerrit-ui/app/yarn.lock
+++ b/polygerrit-ui/app/yarn.lock
@@ -428,17 +428,17 @@
resolved "https://registry.yarnpkg.com/isarray/-/isarray-0.0.1.tgz#8a18acfca9a8f4177e09abfc6038939b05d1eedf"
integrity sha1-ihis/Kmo9Bd+Cav8YDiTmwXR7t8=
-lit-element@^2.4.0:
- version "2.4.0"
- resolved "https://registry.yarnpkg.com/lit-element/-/lit-element-2.4.0.tgz#b22607a037a8fc08f5a80736dddf7f3f5d401452"
- integrity sha512-pBGLglxyhq/Prk2H91nA0KByq/hx/wssJBQFiYqXhGDvEnY31PRGYf1RglVzyLeRysu0IHm2K0P196uLLWmwFg==
+lit-element@^2.5.1:
+ version "2.5.1"
+ resolved "https://registry.yarnpkg.com/lit-element/-/lit-element-2.5.1.tgz#3fa74b121a6cd22902409ae3859b7847d01aa6b6"
+ integrity sha512-ogu7PiJTA33bEK0xGu1dmaX5vhcRjBXCFexPja0e7P7jqLhTpNKYRPmE+GmiCaRVAbiQKGkUgkh/i6+bh++dPQ==
dependencies:
lit-html "^1.1.1"
lit-html@^1.1.1:
- version "1.3.0"
- resolved "https://registry.yarnpkg.com/lit-html/-/lit-html-1.3.0.tgz#c80f3cc5793a6dea6c07172be90a70ab20e56034"
- integrity sha512-0Q1bwmaFH9O14vycPHw8C/IeHMk/uSDldVLIefu/kfbTBGIc44KGH6A8p1bDfxUfHdc8q6Ct7kQklWoHgr4t1Q==
+ version "1.4.1"
+ resolved "https://registry.yarnpkg.com/lit-html/-/lit-html-1.4.1.tgz#0c6f3ee4ad4eb610a49831787f0478ad8e9ae5e0"
+ integrity sha512-B9btcSgPYb1q4oSOb/PrOT6Z/H+r6xuNzfH4lFli/AWhYwdtrgQkQWBbIc6mdnf6E2IL3gDXdkkqNktpU0OZQA==
page@^1.11.6:
version "1.11.6"
