From 06cf5833a378c5431fd5c3308793d5e05daaadd9 Mon Sep 17 00:00:00 2001 From: Kasper Nilsson Date: Tue, 12 Jun 2018 12:01:06 -0700 Subject: [PATCH] Fix editable-label losing focus The editable label was setting focus on the dropdown immediately due to it awaiting the wrong exit condition for _awaitOpen, resulting in a race where focus was only sometimes set on the input. The function was borrowed from gr-overlay, which has styles applied to it directly. In the case of the editable label, we must wait for the style to be set on the dropdown instead. Bug: Issue 8980 Change-Id: I31439a2065159976a9aeda52efd366914cae617f --- .../shared/gr-editable-label/gr-editable-label.js | 4 ++-- .../gr-editable-label/gr-editable-label_test.html | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.js b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.js index f943b8e4fd..b7d65d32d0 100644 --- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.js +++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label.js @@ -92,7 +92,7 @@ _showDropdown() { if (this.readOnly || this.editing) { return; } - this._open().then(() => { + return this._open().then(() => { this.$.input.$.input.focus(); if (!this.$.input.value) { return; } this.$.input.$.input.setSelectionRange(0, this.$.input.value.length); @@ -118,7 +118,7 @@ let iters = 0; const step = () => { this.async(() => { - if (this.style.display !== 'none') { + if (this.$.dropdown.style.display !== 'none') { fn.call(this); } else if (iters++ < AWAIT_MAX_ITERS) { step.call(this); diff --git a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.html b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.html index 058654b503..68151734f4 100644 --- a/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.html +++ b/polygerrit-ui/app/elements/shared/gr-editable-label/gr-editable-label_test.html @@ -77,14 +77,17 @@ limitations under the License. assert.isFalse(element.$.dropdown.opened); assert.isTrue(label.classList.contains('editable')); assert.equal(label.textContent, 'value text'); + const focusSpy = sandbox.spy(element.$.input.$.input, 'focus'); + const showSpy = sandbox.spy(element, '_showDropdown'); MockInteractions.tap(label); - Polymer.dom.flush(); - - // The dropdown is open (which covers up the label): - assert.isTrue(element.$.dropdown.opened); - assert.equal(input.value, 'value text'); + return showSpy.lastCall.returnValue.then(() => { + // The dropdown is open (which covers up the label): + assert.isTrue(element.$.dropdown.opened); + assert.isTrue(focusSpy.called); + assert.equal(input.value, 'value text'); + }); }); test('title with placeholder', done => {