Disable overflow menu actions when they are in progress Adds the ability to disable entries in GR-DROPDOWN via the `disabledIds` property. GR-CHANGE-ACTIONS uses this to disable overflow menu change actions in the same way that it disables the buttons for top-level actions. Tests are added for both elements. Bug: Issue 5366 Change-Id: Iaeba5f5cb9cfa4d7a89d955a65f3a4134780942a
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html index 9f31550..ab3a98a 100644 --- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html +++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -84,6 +84,7 @@ on-tap-item-cherrypick="_handleCherrypickTap" on-tap-item-delete="_handleDeleteTap" hidden$="[[_shouldHideActions(_menuActions.*, _loading)]]" + disabled-ids="[[_disabledMenuActions]]" items="[[_menuActions]]">More</gr-dropdown> <section hidden$="[[_shouldHideActions(_topLevelActions.*, _loading)]]"> <template
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js index 148b724..aa90a09 100644 --- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js +++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -170,6 +170,10 @@ type: Array, value: function() { return []; }, }, + _disabledMenuActions: { + type: Array, + value: function() { return []; }, + }, }, ActionType: ActionType, @@ -281,6 +285,7 @@ this._keyCount(revisionActionsChangeRecord) === 0 && additionalActions.length === 0; this._actionLoadingMessage = null; + this._disabledMenuActions = []; }, _getValuesFor: function(obj) { @@ -563,9 +568,16 @@ _setLoadingOnButtonWithKey: function(key) { this._actionLoadingMessage = this._computeLoadingLabel(key); - // Return a NoOp for menu keys. @see Issue 5366 - if (MENU_ACTION_KEYS.indexOf(key) !== -1) { return function() {}; } + // If the action appears in the overflow menu. + if (MENU_ACTION_KEYS.indexOf(key) !== -1) { + this.push('_disabledMenuActions', key === '/' ? 'delete' : key); + return function() { + this._actionLoadingMessage = null; + this._disabledMenuActions = []; + }.bind(this); + } + // Otherwise it's a top-level action. var buttonEl = this.$$('[data-action-key="' + key + '"]'); buttonEl.setAttribute('loading', true); buttonEl.disabled = true;
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html index c1b582e..9d27ddc 100644 --- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html +++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -399,11 +399,16 @@ }); test('_setLoadingOnButtonWithKey overflow menu', function() { - // TODO(wyatta): Should not throw error when setting loading on an - // overflow action. @see Issue 5366 var key = 'cherrypick'; var cleanup = element._setLoadingOnButtonWithKey(key); + assert.equal(element._actionLoadingMessage, 'Cherry-Picking...'); + assert.include(element._disabledMenuActions, 'cherrypick'); assert.isFunction(cleanup); + + cleanup(); + + assert.notOk(element._actionLoadingMessage); + assert.notInclude(element._disabledMenuActions, 'cherrypick'); }); suite('revert change', function() {
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html index c72e3da..db07de4 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html +++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
@@ -62,12 +62,16 @@ display: block; padding: .85em 1em; } + li .itemAction.disabled { + color: #ccc; + cursor: default; + } li .itemAction:link, li .itemAction:visited { color: #00e; text-decoration: none; } - li .itemAction:hover { + li .itemAction:not(.disabled):hover { background-color: #6B82D6; color: #fff; } @@ -127,7 +131,7 @@ initial-count="75"> <li> <span - class="itemAction" + class$="itemAction [[_computeDisabledClass(link.id, disabledIds.*)]]" data-id$="[[link.id]]" on-tap="_handleItemTap" hidden$="[[link.url]]">[[link.name]]</span>
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js index d1fae7f..6ab1301 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js +++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js
@@ -44,6 +44,15 @@ value: 40, }, + /** + * List the IDs of dropdown buttons to be disabled. (Note this only + * diisables bittons and not link entries.) + */ + disabledIds: { + type: Array, + value: function() { return []; }, + }, + _hasAvatars: String, }, @@ -76,7 +85,13 @@ _handleItemTap: function(e) { var id = e.target.getAttribute('data-id'); - if (id) { this.dispatchEvent(new CustomEvent('tap-item-' + id)); } + if (id && this.disabledIds.indexOf(id) === -1) { + this.dispatchEvent(new CustomEvent('tap-item-' + id)); + } + }, + + _computeDisabledClass: function(id, disabledIdsRecord) { + return disabledIdsRecord.base.indexOf(id) === -1 ? '' : 'disabled'; }, }); })();
diff --git a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html index 2794caf..390213c 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html +++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown_test.html
@@ -80,5 +80,16 @@ MockInteractions.tap(element.$$('.itemAction')); assert.isTrue(stub.called); }); + + test('disabled non link item', function() { + element.items = [{name: 'item one', id: 'foo'}]; + element.disabledIds = ['foo']; + + var stub = sinon.stub(); + element.addEventListener('tap-item-foo', stub); + flushAsynchronousOperations(); + MockInteractions.tap(element.$$('.itemAction')); + assert.isFalse(stub.called); + }); }); </script>