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>
