opendev/gerrit
Code Issues Proposed changes

Merge "Remove remaining usage of LegacyElementMixin functionality"

Browse Source
This commit is contained in:
Ben Rohlfs
2021-03-15 08:29:56 +00:00
committed by Gerrit Code Review
parent c70d70d903 ce2fda54b3
commit a35fbbcc1a
11 changed files with 140 additions and 143 deletions
Download Patch File Download Diff File Expand all files Collapse all files

30
polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog-it_test.js
View File

@@ -18,7 +18,6 @@
import '../../../test/common-test-setup-karma.js'; import '../../../test/common-test-setup-karma.js';
import {resetPlugins, stubRestApi} from '../../../test/test-utils.js'; import {resetPlugins, stubRestApi} from '../../../test/test-utils.js';
import './gr-reply-dialog.js'; import './gr-reply-dialog.js';
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js'; import {getPluginLoader} from '../../shared/gr-js-api-interface/gr-plugin-loader.js';
import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js'; import {_testOnly_initGerritPluginApi} from '../../shared/gr-js-api-interface/gr-gerrit.js';
@@ -104,7 +103,7 @@ suite('gr-reply-dialog-it tests', () => {
assert.isTrue(sendStub.called); assert.isTrue(sendStub.called);
}); });
test('lgtm plugin', done => { test('lgtm plugin', async () => {
resetPlugins(); resetPlugins();
pluginApi.install(plugin => { pluginApi.install(plugin => {
const replyApi = plugin.changeReply(); const replyApi = plugin.changeReply();
@@ -121,20 +120,17 @@ suite('gr-reply-dialog-it tests', () => {
element = basicFixture.instantiate(); element = basicFixture.instantiate();
setupElement(element); setupElement(element);
getPluginLoader().loadPlugins([]); getPluginLoader().loadPlugins([]);
getPluginLoader().awaitPluginsLoaded() await getPluginLoader().awaitPluginsLoaded();
.then(() => { await flush();
flush(() => { const textarea = element.$.textarea.getNativeTextarea();
const textarea = element.$.textarea.getNativeTextarea(); textarea.value = 'LGTM';
textarea.value = 'LGTM'; textarea.dispatchEvent(new CustomEvent(
textarea.dispatchEvent(new CustomEvent( 'input', {bubbles: true, composed: true}));
'input', {bubbles: true, composed: true})); await flush();
const labelScoreRows = dom(element.$.labelScores.root) const labelScoreRows = element.$.labelScores.shadowRoot
.querySelector('gr-label-score-row[name="Code-Review"]'); .querySelector('gr-label-score-row[name="Code-Review"]');
const selectedBtn = dom(labelScoreRows.root) const selectedBtn = labelScoreRows.shadowRoot
.querySelector('gr-button[data-value="+1"].iron-selected'); .querySelector('gr-button[data-value="+1"].iron-selected');
assert.isOk(selectedBtn); assert.isOk(selectedBtn);
done();
});
});
}); });
}); });

6
polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
View File

@@ -757,13 +757,13 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
} }
if (section === FocusTarget.BODY) { if (section === FocusTarget.BODY) {
const textarea = this.$.textarea; const textarea = this.$.textarea;
textarea.async(() => textarea.getNativeTextarea().focus()); setTimeout(() => textarea.getNativeTextarea().focus());
} else if (section === FocusTarget.REVIEWERS) { } else if (section === FocusTarget.REVIEWERS) {
const reviewerEntry = this.$.reviewers.focusStart; const reviewerEntry = this.$.reviewers.focusStart;
reviewerEntry.async(() => reviewerEntry.focus()); setTimeout(() => reviewerEntry.focus());
} else if (section === FocusTarget.CCS) { } else if (section === FocusTarget.CCS) {
const ccEntry = this.$.ccs.focusStart; const ccEntry = this.$.ccs.focusStart;
ccEntry.async(() => ccEntry.focus()); setTimeout(() => ccEntry.focus());
} }
} }

196
polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
View File

@@ -609,7 +609,7 @@ suite('gr-reply-dialog tests', () => {
return false; return false;
} }
function testConfirmationDialog(done, cc) { async function testConfirmationDialog(cc) {
const yesButton = element const yesButton = element
.shadowRoot .shadowRoot
.querySelector('.reviewerConfirmationButtons gr-button:first-child'); .querySelector('.reviewerConfirmationButtons gr-button:first-child');
@@ -651,95 +651,90 @@ suite('gr-reply-dialog tests', () => {
element._pendingConfirmationDetails); element._pendingConfirmationDetails);
} }
observer await observer;
.then(() => { assert.isTrue(isVisible(element.$.reviewerConfirmationOverlay));
assert.isTrue(isVisible(element.$.reviewerConfirmationOverlay)); observer = overlayObserver('closed');
observer = overlayObserver('closed'); const expected = 'Group name has 10 members';
const expected = 'Group name has 10 members'; assert.notEqual(
assert.notEqual( element.$.reviewerConfirmationOverlay.innerText
element.$.reviewerConfirmationOverlay.innerText .indexOf(expected),
.indexOf(expected), -1);
-1); MockInteractions.tap(noButton); // close the overlay
MockInteractions.tap(noButton); // close the overlay
return observer;
}).then(() => {
assert.isFalse(isVisible(element.$.reviewerConfirmationOverlay));
// We should be focused on account entry input. await observer;
assert.isTrue( assert.isFalse(isVisible(element.$.reviewerConfirmationOverlay));
isFocusInsideElement(
element.$.reviewers.$.entry.$.input.$.input
)
);
// No reviewer/CC should have been added. // We should be focused on account entry input.
assert.equal(element.$.ccs.additions().length, 0); assert.isTrue(
assert.equal(element.$.reviewers.additions().length, 0); isFocusInsideElement(
element.$.reviewers.$.entry.$.input.$.input
)
);
// Reopen confirmation dialog. // No reviewer/CC should have been added.
observer = overlayObserver('opened'); assert.equal(element.$.ccs.additions().length, 0);
if (cc) { assert.equal(element.$.reviewers.additions().length, 0);
element._ccPendingConfirmation = {
group,
count: 10,
};
} else {
element._reviewerPendingConfirmation = {
group,
count: 10,
};
}
return observer;
})
.then(() => {
assert.isTrue(isVisible(element.$.reviewerConfirmationOverlay));
observer = overlayObserver('closed');
MockInteractions.tap(yesButton); // Confirm the group.
return observer;
})
.then(() => {
assert.isFalse(isVisible(element.$.reviewerConfirmationOverlay));
const additions = cc ?
element.$.ccs.additions() :
element.$.reviewers.additions();
assert.deepEqual(
additions,
[
{
group: {
id: 'id',
name: 'name',
confirmed: true,
_group: true,
_pendingAdd: true,
},
},
]);
// We should be focused on account entry input. // Reopen confirmation dialog.
if (cc) { observer = overlayObserver('opened');
assert.isTrue( if (cc) {
isFocusInsideElement( element._ccPendingConfirmation = {
element.$.ccs.$.entry.$.input.$.input group,
) count: 10,
); };
} else { } else {
assert.isTrue( element._reviewerPendingConfirmation = {
isFocusInsideElement( group,
element.$.reviewers.$.entry.$.input.$.input count: 10,
) };
); }
}
}) await observer;
.then(done); assert.isTrue(isVisible(element.$.reviewerConfirmationOverlay));
observer = overlayObserver('closed');
MockInteractions.tap(yesButton); // Confirm the group.
await observer;
assert.isFalse(isVisible(element.$.reviewerConfirmationOverlay));
const additions = cc ?
element.$.ccs.additions() :
element.$.reviewers.additions();
assert.deepEqual(
additions,
[
{
group: {
id: 'id',
name: 'name',
confirmed: true,
_group: true,
_pendingAdd: true,
},
},
]);
// We should be focused on account entry input.
if (cc) {
assert.isTrue(
isFocusInsideElement(
element.$.ccs.$.entry.$.input.$.input
)
);
} else {
assert.isTrue(
isFocusInsideElement(
element.$.reviewers.$.entry.$.input.$.input
)
);
}
} }
test('cc confirmation', done => { test('cc confirmation', async () => {
testConfirmationDialog(done, true); testConfirmationDialog(true);
}); });
test('reviewer confirmation', done => { test('reviewer confirmation', async () => {
testConfirmationDialog(done, false); testConfirmationDialog(false);
}); });
test('_getStorageLocation', () => { test('_getStorageLocation', () => {
@@ -887,42 +882,37 @@ suite('gr-reply-dialog tests', () => {
assert.isFalse(filter({group: cc2})); assert.isFalse(filter({group: cc2}));
}); });
test('_focusOn', () => { test('_focusOn', async () => {
sinon.spy(element, '_chooseFocusTarget'); sinon.spy(element, '_chooseFocusTarget');
flush();
const textareaStub = sinon.stub(element.$.textarea, 'async');
const reviewerEntryStub = sinon.stub(element.$.reviewers.focusStart,
'async');
const ccStub = sinon.stub(element.$.ccs.focusStart, 'async');
element._focusOn(); element._focusOn();
await flush();
assert.equal(element._chooseFocusTarget.callCount, 1); assert.equal(element._chooseFocusTarget.callCount, 1);
assert.deepEqual(textareaStub.callCount, 1); assert.equal(element.shadowRoot.activeElement.tagName, 'GR-TEXTAREA');
assert.deepEqual(reviewerEntryStub.callCount, 0); assert.equal(element.shadowRoot.activeElement.id, 'textarea');
assert.deepEqual(ccStub.callCount, 0);
element._focusOn(element.FocusTarget.ANY); element._focusOn(element.FocusTarget.ANY);
await flush();
assert.equal(element._chooseFocusTarget.callCount, 2); assert.equal(element._chooseFocusTarget.callCount, 2);
assert.deepEqual(textareaStub.callCount, 2); assert.equal(element.shadowRoot.activeElement.tagName, 'GR-TEXTAREA');
assert.deepEqual(reviewerEntryStub.callCount, 0); assert.equal(element.shadowRoot.activeElement.id, 'textarea');
assert.deepEqual(ccStub.callCount, 0);
element._focusOn(element.FocusTarget.BODY); element._focusOn(element.FocusTarget.BODY);
await flush();
assert.equal(element._chooseFocusTarget.callCount, 2); assert.equal(element._chooseFocusTarget.callCount, 2);
assert.deepEqual(textareaStub.callCount, 3); assert.equal(element.shadowRoot.activeElement.tagName, 'GR-TEXTAREA');
assert.deepEqual(reviewerEntryStub.callCount, 0); assert.equal(element.shadowRoot.activeElement.id, 'textarea');
assert.deepEqual(ccStub.callCount, 0);
element._focusOn(element.FocusTarget.REVIEWERS); element._focusOn(element.FocusTarget.REVIEWERS);
await flush();
assert.equal(element._chooseFocusTarget.callCount, 2); assert.equal(element._chooseFocusTarget.callCount, 2);
assert.deepEqual(textareaStub.callCount, 3); assert.equal(element.shadowRoot.activeElement.tagName, 'GR-ACCOUNT-LIST');
assert.deepEqual(reviewerEntryStub.callCount, 1); assert.equal(element.shadowRoot.activeElement.id, 'reviewers');
assert.deepEqual(ccStub.callCount, 0);
element._focusOn(element.FocusTarget.CCS); element._focusOn(element.FocusTarget.CCS);
await flush();
assert.equal(element._chooseFocusTarget.callCount, 2); assert.equal(element._chooseFocusTarget.callCount, 2);
assert.deepEqual(textareaStub.callCount, 3); assert.equal(element.shadowRoot.activeElement.tagName, 'GR-ACCOUNT-LIST');
assert.deepEqual(reviewerEntryStub.callCount, 1); assert.equal(element.shadowRoot.activeElement.id, 'ccs');
assert.deepEqual(ccStub.callCount, 1);
}); });
test('_chooseFocusTarget', () => { test('_chooseFocusTarget', () => {

5
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element.ts
View File

@@ -168,8 +168,9 @@ export class GrDiffBuilderElement extends LegacyElementMixin(PolymerElement) {
super.disconnectedCallback(); super.disconnectedCallback();
} }
get diffElement() { get diffElement(): HTMLTableElement {
return this.queryEffectiveChildren('#diffTable') as HTMLTableElement; // Not searching in shadowRoot, because the diff table is slotted!
return this.querySelector('#diffTable') as HTMLTableElement;
} }
_computeLeftCoverageRanges(coverageRanges: CoverageRange[]) { _computeLeftCoverageRanges(coverageRanges: CoverageRange[]) {

2
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-element_test.js
View File

@@ -843,7 +843,7 @@ suite('gr-diff-builder tests', () => {
}, },
]; ];
element = basicFixture.instantiate(); element = basicFixture.instantiate();
outputEl = element.queryEffectiveChildren('#diffTable'); outputEl = element.querySelector('#diffTable');
keyLocations = {left: {}, right: {}}; keyLocations = {left: {}, right: {}};
sinon.stub(element, '_getDiffBuilder').callsFake(() => { sinon.stub(element, '_getDiffBuilder').callsFake(() => {
const builder = new GrDiffBuilderSideBySide({content}, prefs, outputEl); const builder = new GrDiffBuilderSideBySide({content}, prefs, outputEl);

3
polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
View File

@@ -102,8 +102,7 @@ suite('gr-diff-host tests', () => {
threadEls[0].dispatchEvent( threadEls[0].dispatchEvent(
new CustomEvent('thread-discard', {detail: {rootId: 4711}})); new CustomEvent('thread-discard', {detail: {rootId: 4711}}));
const attachedThreads = element.queryAllEffectiveChildren( const attachedThreads = element.querySelectorAll('gr-comment-thread');
'gr-comment-thread');
assert.equal(attachedThreads.length, 1); assert.equal(attachedThreads.length, 1);
assert.equal(attachedThreads[0].comments[0].id, 42); assert.equal(attachedThreads[0].comments[0].id, 42);
}); });

2
polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor_test.js
View File

@@ -750,7 +750,6 @@ suite('gr-diff-processor tests', () => {
], ],
}; };
const content = _.times(200, _.constant(contentRow)); const content = _.times(200, _.constant(contentRow));
sinon.stub(element, 'async');
element._isScrolling = true; element._isScrolling = true;
element.process(content); element.process(content);
// Just the files group - no more processing during scrolling. // Just the files group - no more processing during scrolling.
@@ -770,7 +769,6 @@ suite('gr-diff-processor tests', () => {
], ],
}; };
const content = _.times(200, _.constant(contentRow)); const content = _.times(200, _.constant(contentRow));
sinon.stub(element, 'async');
element.process(content, true); element.process(content, true);
assert.equal(element.groups.length, 2); assert.equal(element.groups.length, 2);

12
polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
View File

@@ -282,6 +282,9 @@ export class GrDiff extends LegacyElementMixin(PolymerElement) {
@property({type: Array}) @property({type: Array})
layers?: DiffLayer[]; layers?: DiffLayer[];
@property({type: Boolean})
isAttached = false;
private renderDiffTableTask?: DelayedTask; private renderDiffTableTask?: DelayedTask;
constructor() { constructor() {
@@ -298,10 +301,12 @@ export class GrDiff extends LegacyElementMixin(PolymerElement) {
connectedCallback() { connectedCallback() {
super.connectedCallback(); super.connectedCallback();
this._observeNodes(); this._observeNodes();
this.isAttached = true;
} }
/** @override */ /** @override */
disconnectedCallback() { disconnectedCallback() {
this.isAttached = false;
this.renderDiffTableTask?.cancel(); this.renderDiffTableTask?.cancel();
this._unobserveIncrementalNodes(); this._unobserveIncrementalNodes();
this._unobserveNodes(); this._unobserveNodes();
@@ -325,12 +330,7 @@ export class GrDiff extends LegacyElementMixin(PolymerElement) {
} }
@observe('loggedIn', 'isAttached') @observe('loggedIn', 'isAttached')
_enableSelectionObserver(loggedIn: boolean, isAttached?: boolean) { _enableSelectionObserver(loggedIn: boolean, isAttached: boolean) {
// Polymer 2: check for undefined
if ([loggedIn, isAttached].includes(undefined)) {
return;
}
if (loggedIn && isAttached) { if (loggedIn && isAttached) {
document.addEventListener( document.addEventListener(
'-shadow-selectionchange', '-shadow-selectionchange',

6
polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
View File

@@ -52,15 +52,17 @@ suite('gr-diff tests', () => {
sinon.stub(element.$.highlights, 'handleSelectionChange'); sinon.stub(element.$.highlights, 'handleSelectionChange');
}); });
test('enabled if logged in', () => { test('enabled if logged in', async () => {
element.loggedIn = true; element.loggedIn = true;
emulateSelection(); emulateSelection();
await flush();
assert.isTrue(element.$.highlights.handleSelectionChange.called); assert.isTrue(element.$.highlights.handleSelectionChange.called);
}); });
test('ignored if logged out', () => { test('ignored if logged out', async () => {
element.loggedIn = false; element.loggedIn = false;
emulateSelection(); emulateSelection();
await flush();
assert.isFalse(element.$.highlights.handleSelectionChange.called); assert.isFalse(element.$.highlights.handleSelectionChange.called);
}); });
}); });

20
polygerrit-ui/app/elements/plugins/gr-endpoint-decorator/gr-endpoint-decorator.ts
View File

@@ -68,7 +68,9 @@ export class GrEndpointDecorator extends LegacyElementMixin(PolymerElement) {
return this._initProperties( return this._initProperties(
el, el,
plugin, plugin,
this.getContentChildren().find(el => el.nodeName !== 'GR-ENDPOINT-PARAM') // The direct children are slotted into <slot>, so this is identical to
// this.shadowRoot.querySelector('slot').assignedElements()[0].
this.firstElementChild
).then(el => { ).then(el => {
const slotEl = slot const slotEl = slot
? this.querySelector(`gr-endpoint-slot[name=${slot}]`) ? this.querySelector(`gr-endpoint-slot[name=${slot}]`)
@@ -82,9 +84,16 @@ export class GrEndpointDecorator extends LegacyElementMixin(PolymerElement) {
}); });
} }
// As of March 2021 the only known plugin that replaces an endpoint instead
// of decorating it is codemirror_editor.
_initReplacement(name: string, plugin: PluginApi): Promise<HTMLElement> { _initReplacement(name: string, plugin: PluginApi): Promise<HTMLElement> {
this.getContentChildNodes() // The direct children are slotted into <slot>, so they are identical to
// this.shadowRoot.querySelector('slot').assignedElements().
const directChildren = [...this.childNodes];
const shadowChildren = [...(this.shadowRoot?.childNodes ?? [])];
[...directChildren, ...shadowChildren]
.filter(node => node.nodeName !== 'GR-ENDPOINT-PARAM') .filter(node => node.nodeName !== 'GR-ENDPOINT-PARAM')
.filter(node => node.nodeName !== 'SLOT')
.forEach(node => (node as ChildNode).remove()); .forEach(node => (node as ChildNode).remove());
const el = document.createElement(name); const el = document.createElement(name);
return this._initProperties(el, plugin).then((el: HTMLElement) => return this._initProperties(el, plugin).then((el: HTMLElement) =>
@@ -99,13 +108,16 @@ export class GrEndpointDecorator extends LegacyElementMixin(PolymerElement) {
_initProperties( _initProperties(
htmlEl: HTMLElement, htmlEl: HTMLElement,
plugin: PluginApi, plugin: PluginApi,
content?: HTMLElement content?: Element | null
) { ) {
const el = htmlEl as HTMLElement & { const el = htmlEl as HTMLElement & {
plugin?: PluginApi; plugin?: PluginApi;
content?: HTMLElement; content?: Element;
}; };
el.plugin = plugin; el.plugin = plugin;
// The content is (only?) used in ChangeReplyPluginApi.
// Maybe it would be better for the consumer side to figure out the content
// with something like el.getRootNode().host, etc.
if (content) { if (content) {
el.content = content; el.content = content;
} }

1
polygerrit-ui/app/elements/shared/gr-hovercard/gr-hovercard-behavior.ts
View File

@@ -478,7 +478,6 @@ export const hovercardBehaviorMixin = dedupingMixin(
); );
export interface GrHovercardBehaviorInterface { export interface GrHovercardBehaviorInterface {
attached(): void;
ready(): void; ready(): void;
removeListeners(): void; removeListeners(): void;
debounceHide(): void; debounceHide(): void;