diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js index 3dabd63881..9f94b76cf5 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js @@ -1104,11 +1104,6 @@ class GrFileList extends mixinBehaviors( [ return expandedFilesRecord.base.some(f => f.path === path); } - _onLineSelected(e, detail) { - this.$.diffCursor.moveToLineNumber(detail.number, detail.side, - detail.path); - } - _computeExpandedFiles(expandedCount, totalCount) { if (expandedCount === 0) { return GrFileListConstants.FilesExpandedState.NONE; diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.js index b20a751cbf..93b8414a71 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.js +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_html.js @@ -385,7 +385,7 @@ export const htmlTemplate = html` diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html index 17f83c51e3..2eceeba642 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html @@ -1104,7 +1104,10 @@ suite('gr-file-list tests', () => { cancel() {}, getCursorStops() { return []; }, addEventListener(eventName, callback) { - callback(new Event(eventName)); + if (['render-start', 'render-content', 'scroll'] + .indexOf(eventName) >= 0) { + callback(new Event(eventName)); + } }, }]; sinon.stub(element, 'diffs', { diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js index 592de8392c..bd486dc70b 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js @@ -127,6 +127,14 @@ class GrDiffCursor extends GestureEventListeners( ]; } + constructor() { + super(); + this._boundHandleWindowScroll = () => this._handleWindowScroll(); + this._boundHandleDiffRenderStart = () => this._handleDiffRenderStart(); + this._boundHandleDiffRenderContent = () => this._handleDiffRenderContent(); + this._boundHandleDiffLineSelected = e => this._handleDiffLineSelected(e); + } + /** @override */ ready() { super.ready(); @@ -148,16 +156,16 @@ class GrDiffCursor extends GestureEventListeners( } /** @override */ - attached() { - super.attached(); + connectedCallback() { + super.connectedCallback(); // Catch when users are scrolling as the view loads. - this.listen(window, 'scroll', '_handleWindowScroll'); + window.addEventListener('scroll', this._boundHandleWindowScroll); } /** @override */ - detached() { - super.detached(); - this.unlisten(window, 'scroll', '_handleWindowScroll'); + disconnectedCallback() { + super.disconnectedCallback(); + window.removeEventListener('scroll', this._boundHandleWindowScroll); } moveLeft() { @@ -315,6 +323,11 @@ class GrDiffCursor extends GestureEventListeners( this._preventAutoScrollOnManualScroll = false; } + _handleDiffLineSelected(event) { + this.moveToLineNumber( + event.detail.number, event.detail.side, event.detail.path); + } + createCommentInPlace() { const diffWithRangeSelected = this.diffs .find(diff => diff.isRangeSelected()); @@ -472,21 +485,22 @@ class GrDiffCursor extends GestureEventListeners( spliceIdx++) { splice = changeRecord.indexSplices[spliceIdx]; - for (i = splice.index; - i < splice.index + splice.addedCount; - i++) { - this.listen(this.diffs[i], 'render-start', '_handleDiffRenderStart'); - this.listen( - this.diffs[i], 'render-content', '_handleDiffRenderContent'); + for (i = splice.index; i < splice.index + splice.addedCount; i++) { + this.diffs[i].addEventListener( + 'render-start', this._boundHandleDiffRenderStart); + this.diffs[i].addEventListener( + 'render-content', this._boundHandleDiffRenderContent); + this.diffs[i].addEventListener( + 'line-selected', this._boundHandleDiffLineSelected); } - for (i = 0; - i < splice.removed && splice.removed.length; - i++) { - this.unlisten(splice.removed[i], - 'render-start', '_handleDiffRenderStart'); - this.unlisten(splice.removed[i], - 'render-content', '_handleDiffRenderContent'); + for (i = 0; i < splice.removed && splice.removed.length; i++) { + splice.removed[i].removeEventListener( + 'render-start', this._boundHandleDiffRenderStart); + splice.removed[i].removeEventListener( + 'render-content', this._boundHandleDiffRenderContent); + splice.removed[i].removeEventListener( + 'line-selected', this._boundHandleDiffLineSelected); } } } diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html index 35f7afc15e..1aa9325384 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html @@ -136,6 +136,20 @@ suite('gr-diff-cursor tests', () => { assert.equal(cursorElement._scrollBehavior, 'keep-visible'); }); + test('moves to selected line', () => { + const moveToNumStub = sandbox.stub(cursorElement, 'moveToLineNumber'); + + cursorElement._handleDiffLineSelected( + new CustomEvent('line-selected', { + detail: {number: '123', side: 'right', path: 'some/file'}, + })); + + assert.isTrue(moveToNumStub.called); + assert.equal(moveToNumStub.lastCall.args[0], '123'); + assert.equal(moveToNumStub.lastCall.args[1], 'right'); + assert.equal(moveToNumStub.lastCall.args[2], 'some/file'); + }); + suite('unified diff', () => { setup(done => { // We must allow the diff to re-render after setting the viewMode. diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js index 27d39a74a6..09e845180e 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js @@ -1033,7 +1033,6 @@ class GrDiffView extends mixinBehaviors( [ } _onLineSelected(e, detail) { - this.$.cursor.moveToLineNumber(detail.number, detail.side); if (!this._change) { return; } const cursorAddress = this.$.cursor.getAddress(); const number = cursorAddress ? cursorAddress.number : undefined; diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html index 5dccd5e2b6..ebfa0134ca 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html @@ -904,7 +904,6 @@ suite('gr-diff-view tests', () => { test('_onLineSelected', () => { const getUrlStub = sandbox.stub(Gerrit.Nav, 'getUrlForDiffById'); const replaceStateStub = sandbox.stub(history, 'replaceState'); - const moveStub = sandbox.stub(element.$.cursor, 'moveToLineNumber'); sandbox.stub(element.$.cursor, 'getAddress') .returns({number: 123, isLeftSide: false}); @@ -919,10 +918,6 @@ suite('gr-diff-view tests', () => { element._onLineSelected(e, detail); - assert.isTrue(moveStub.called); - assert.equal(moveStub.lastCall.args[0], detail.number); - assert.equal(moveStub.lastCall.args[1], detail.side); - assert.isTrue(replaceStateStub.called); assert.isTrue(getUrlStub.called); });