From 44e2ccd2ab8e7cc6c6b5af8b46f41a8c002fb697 Mon Sep 17 00:00:00 2001 From: Viktar Donich Date: Mon, 17 Apr 2017 10:58:12 -0700 Subject: [PATCH] Make it possible to add or remove change actions to overflow menu ``` js Gerrit.install(function(plugin) { // Move cherry-pick button out of overflow menu. plugin.setActionOverflow('revision', 'cherrypick', false); // Move submit button out of overflow menu. plugin.setActionOverflow('revision', 'submit', true); }); ``` Revision and change actions are as returned by Gerrit's REST API. Feature: Issue 5360 Change-Id: I151894b39929bd67ef0e00802c699831ab3f72fc --- .../gr-change-actions/gr-change-actions.html | 3 +- .../gr-change-actions/gr-change-actions.js | 162 +++++++++++++----- .../gr-change-actions_test.html | 39 ++++- .../shared/gr-dropdown/gr-dropdown.js | 12 ++ .../shared/gr-dropdown/gr-dropdown_test.html | 17 +- .../gr-change-actions-js-api.js | 5 + .../gr-change-actions-js-api_test.html | 20 ++- 7 files changed, 198 insertions(+), 60 deletions(-) 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 71ccb04713..2cfa5baaf0 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 @@ -81,8 +81,7 @@ limitations under the License. down-arrow vertical-offset="32" horizontal-align="right" - on-tap-item-cherrypick="_handleCherrypickTap" - on-tap-item-delete="_handleDeleteTap" + on-tap-item="_handleOveflowItemTap" hidden$="[[_shouldHideActions(_menuActions.*, _loading)]]" disabled-ids="[[_disabledMenuActions]]" items="[[_menuActions]]">More 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 2b0916d82d..a4dd14fe13 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 @@ -87,15 +87,6 @@ method: 'POST', }; - /** - * Keys for actions to appear in the overflow menu rather than the top-level - * set of action buttons. - */ - var MENU_ACTION_KEYS = [ - 'cherrypick', - '/', // '/' is the key for the delete action. - ]; - Polymer({ is: 'gr-change-actions', @@ -163,11 +154,32 @@ _topLevelActions: { type: Array, computed: '_computeTopLevelActions(_allActionValues.*, ' + - '_hiddenActions.*)', + '_hiddenActions.*, _overflowActions.*)', }, _menuActions: { type: Array, - computed: '_computeMenuActions(_allActionValues.*, _hiddenActions.*)', + computed: '_computeMenuActions(_allActionValues.*, _hiddenActions.*, ' + + '_overflowActions.*)', + }, + _overflowActions: { + type: Array, + value: function() { + var value = [ + { + type: ActionType.CHANGE, + key: ChangeActions.DELETE, + }, + { + type: ActionType.REVISION, + key: RevisionActions.DELETE, + }, + { + type: ActionType.REVISION, + key: RevisionActions.CHERRYPICK, + } + ]; + return value; + }, }, _additionalActions: { type: Array, @@ -227,7 +239,8 @@ enabled: true, label: label, __type: type, - __key: ADDITIONAL_ACTION_KEY_PREFIX + Math.random().toString(36), + __key: ADDITIONAL_ACTION_KEY_PREFIX + + Math.random().toString(36).substr(2), }; this.push('_additionalActions', action); return action.__key; @@ -249,6 +262,23 @@ ], value); }, + setActionOverflow: function(type, key, overflow) { + if (type !== ActionType.CHANGE && type !== ActionType.REVISION) { + throw Error('Invalid action type given: ' + type); + } + var index = this._getActionOverflowIndex(type, key); + var action = { + type: type, + key: key, + overflow: overflow, + }; + if (!overflow && index !== -1) { + this.splice('_overflowActions', index, 1); + } else if (overflow) { + this.push('_overflowActions', action); + } + }, + setActionHidden: function(type, key, hidden) { if (type !== ActionType.CHANGE && type !== ActionType.REVISION) { throw Error('Invalid action type given: ' + type); @@ -459,20 +489,46 @@ return; } var type = el.getAttribute('data-action-type'); - if (type === ActionType.REVISION) { - this._handleRevisionAction(key); - } else if (key === ChangeActions.REVERT) { - this.showRevertDialog(); - } else if (key === ChangeActions.ABANDON) { - this._showActionDialog(this.$.confirmAbandonDialog); - } else if (key === QUICK_APPROVE_ACTION.key) { - var action = this._allActionValues.find(function(o) { - return o.key === key; - }); - this._fireAction( - this._prependSlash(key), action, true, action.payload); - } else { - this._fireAction(this._prependSlash(key), this.actions[key], false); + this._handleAction(type, key); + }, + + _handleOveflowItemTap: function(e) { + this._handleAction(e.detail.action.__type, e.detail.action.__key); + }, + + _handleAction: function(type, key) { + switch (type) { + case ActionType.REVISION: + this._handleRevisionAction(key); + break; + case ActionType.CHANGE: + this._handleChangeAction(key); + break; + default: + this._fireAction(this._prependSlash(key), this.actions[key], false); + } + }, + + _handleChangeAction: function(key) { + switch (key) { + case ChangeActions.REVERT: + this.showRevertDialog(); + break; + case ChangeActions.ABANDON: + this._showActionDialog(this.$.confirmAbandonDialog); + break; + case QUICK_APPROVE_ACTION.key: + var action = this._allActionValues.find(function(o) { + return o.key === key; + }); + this._fireAction( + this._prependSlash(key), action, true, action.payload); + break; + case ChangeActions.DELETE: + this._handleDeleteTap(); + break; + default: + this._fireAction(this._prependSlash(key), this.actions[key], false); } }, @@ -481,6 +537,12 @@ case RevisionActions.REBASE: this._showActionDialog(this.$.confirmRebase); break; + case RevisionActions.DELETE: + this._handleDeleteConfirm(); + break; + case RevisionActions.CHERRYPICK: + this._handleCherrypickTap(); + break; case RevisionActions.SUBMIT: if (!this._canSubmitChange()) { return; @@ -577,11 +639,17 @@ this._fireAction('/', this.actions[ChangeActions.DELETE], false); }, - _setLoadingOnButtonWithKey: function(key) { + _getActionOverflowIndex: function(type, key) { + return this._overflowActions.findIndex(function(action) { + return action.type === type && action.key === key; + }); + }, + + _setLoadingOnButtonWithKey: function(type, key) { this._actionLoadingMessage = this._computeLoadingLabel(key); // If the action appears in the overflow menu. - if (MENU_ACTION_KEYS.indexOf(key) !== -1) { + if (this._getActionOverflowIndex(type, key) !== -1) { this.push('_disabledMenuActions', key === '/' ? 'delete' : key); return function() { this._actionLoadingMessage = null; @@ -601,8 +669,8 @@ }, _fireAction: function(endpoint, action, revAction, opt_payload) { - var cleanupFn = this._setLoadingOnButtonWithKey(action.__key); - + var cleanupFn = + this._setLoadingOnButtonWithKey(action.__type, action.__key); this._send(action.method, opt_payload, endpoint, revAction, cleanupFn) .then(this._handleResponse.bind(this, action)); }, @@ -742,26 +810,30 @@ return actionA.label > actionB.label ? 1 : -1; }, - _computeTopLevelActions: function(actionRecord, hiddenActionsRecord) { + _computeTopLevelActions: function(actionRecord, hiddenActionsRecord, + overflowActionsRecord) { var hiddenActions = hiddenActionsRecord.base || []; return actionRecord.base.filter(function(a) { - return MENU_ACTION_KEYS.indexOf(a.__key) === -1 && - hiddenActions.indexOf(a.__key) === -1; - }); + var overflow = this._getActionOverflowIndex(a.__type, a.__key) !== -1; + return !overflow && hiddenActions.indexOf(a.__key) === -1; + }.bind(this)); }, - _computeMenuActions: function(actionRecord, hiddenActionsRecord) { + _computeMenuActions: function(actionRecord, hiddenActionsRecord, + overflowActionsRecord) { var hiddenActions = hiddenActionsRecord.base || []; - return actionRecord.base - .filter(function(a) { - return MENU_ACTION_KEYS.indexOf(a.__key) !== -1 && - hiddenActions.indexOf(a.__key) === -1; - }) - .map(function(action) { - var key = action.__key; - if (key === '/') { key = 'delete'; } - return {name: action.label, id: key, }; - }); + return actionRecord.base.filter(function(a) { + var overflow = this._getActionOverflowIndex(a.__type, a.__key) !== -1; + return overflow && hiddenActions.indexOf(a.__key) === -1; + }.bind(this)).map(function(action) { + var key = action.__key; + if (key === '/') { key = 'delete'; } + return { + name: action.label, + id: key + '-' + action.__type, + action: action, + }; + }); }, }); })(); 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 83a1c7e9ca..02b8c0466b 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 @@ -135,7 +135,8 @@ limitations under the License. test('hide menu action', function(done) { flush(function() { - var buttonEl = element.$.moreActions.$$('span[data-id="delete"]'); + var buttonEl = + element.$.moreActions.$$('span[data-id="delete-revision"]'); assert.isOk(buttonEl); assert.throws(element.setActionHidden.bind(element, 'invalid type')); element.setActionHidden(element.ActionType.CHANGE, @@ -145,13 +146,15 @@ limitations under the License. element.ChangeActions.DELETE, true); assert.lengthOf(element._hiddenActions, 1); flush(function() { - var buttonEl = element.$.moreActions.$$('span[data-id="delete"]'); + var buttonEl = + element.$.moreActions.$$('span[data-id="delete-revision"]'); assert.isNotOk(buttonEl); element.setActionHidden(element.ActionType.CHANGE, element.RevisionActions.DELETE, false); flush(function() { - var buttonEl = element.$.moreActions.$$('span[data-id="delete"]'); + var buttonEl = + element.$.moreActions.$$('span[data-id="delete-revision"]'); assert.isOk(buttonEl); done(); }); @@ -174,7 +177,7 @@ limitations under the License. test('delete buttons have explicit labels', function(done) { flush(function() { var deleteItems = element.$.moreActions.items.filter(function(item) { - return item.id === 'delete'; + return item.id.indexOf('delete') === 0; }); assert.equal(deleteItems.length, 2); assert.notEqual(deleteItems[0].name, deleteItems[1].name); @@ -414,7 +417,8 @@ limitations under the License. test('_setLoadingOnButtonWithKey top-level', function() { var key = 'rebase'; - var cleanup = element._setLoadingOnButtonWithKey(key); + var type = 'revision'; + var cleanup = element._setLoadingOnButtonWithKey(type, key); assert.equal(element._actionLoadingMessage, 'Rebasing...'); var button = element.$$('[data-action-key="' + key + '"]'); @@ -432,7 +436,8 @@ limitations under the License. test('_setLoadingOnButtonWithKey overflow menu', function() { var key = 'cherrypick'; - var cleanup = element._setLoadingOnButtonWithKey(key); + var type = 'revision'; + var cleanup = element._setLoadingOnButtonWithKey(type, key); assert.equal(element._actionLoadingMessage, 'Cherry-Picking...'); assert.include(element._disabledMenuActions, 'cherrypick'); assert.isFunction(cleanup); @@ -728,5 +733,27 @@ limitations under the License. assert.equal(approveButton.getAttribute('data-label'), 'bar+2'); }); }); + + suite('setActionOverflow', function() { + test('move action from overflow', function() { + assert.isNotOk(element.$$('[data-action-key="cherrypick"]')); + assert.strictEqual( + element.$.moreActions.items[0].id, 'cherrypick-revision'); + element.setActionOverflow('revision', 'cherrypick', false); + flushAsynchronousOperations(); + assert.isOk(element.$$('[data-action-key="cherrypick"]')); + assert.notEqual( + element.$.moreActions.items[0].id, 'cherrypick-revision'); + }); + + test('move action to overflow', function() { + assert.isOk(element.$$('[data-action-key="submit"]')); + element.setActionOverflow('revision', 'submit', true); + flushAsynchronousOperations(); + assert.isNotOk(element.$$('[data-action-key="submit"]')); + assert.strictEqual( + element.$.moreActions.items[3].id, 'submit-revision'); + }); + }); }); 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 e84357fa03..6e1637ca19 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js +++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.js @@ -23,6 +23,12 @@ * @event tap-item- */ + /** + * Fired when a non-link dropdown item is tapped. + * + * @event tap-item + */ + properties: { items: Array, topContent: Object, @@ -93,7 +99,13 @@ _handleItemTap: function(e) { var id = e.target.getAttribute('data-id'); + var item = this.items.find(function(item) { + return item.id === id; + }); if (id && this.disabledIds.indexOf(id) === -1) { + if (item) { + this.dispatchEvent(new CustomEvent('tap-item', {detail: item})); + } this.dispatchEvent(new CustomEvent('tap-item-' + id)); } }, 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 2db38b90d6..f8b11ba520 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 @@ -74,13 +74,17 @@ limitations under the License. }); test('non link items', function() { - element.items = [ - {name: 'item one', id: 'foo'}, {name: 'item two', id: 'bar'}]; - var stub = sinon.stub(); - element.addEventListener('tap-item-foo', stub); + var item0 = {name: 'item one', id: 'foo'}; + element.items = [item0, {name: 'item two', id: 'bar'}]; + var fooTapped = sinon.stub(); + var tapped = sinon.stub(); + element.addEventListener('tap-item-foo', fooTapped); + element.addEventListener('tap-item', tapped); flushAsynchronousOperations(); MockInteractions.tap(element.$$('.itemAction')); - assert.isTrue(stub.called); + assert.isTrue(fooTapped.called); + assert.isTrue(tapped.called); + assert.deepEqual(tapped.lastCall.args[0].detail, item0); }); test('disabled non link item', function() { @@ -88,10 +92,13 @@ limitations under the License. element.disabledIds = ['foo']; var stub = sinon.stub(); + var tapped = sinon.stub(); element.addEventListener('tap-item-foo', stub); + element.addEventListener('tap-item', tapped); flushAsynchronousOperations(); MockInteractions.tap(element.$$('.itemAction')); assert.isFalse(stub.called); + assert.isFalse(tapped.called); }); }); diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.js index 72c7f6ef93..96e9c904bd 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.js +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api.js @@ -33,6 +33,11 @@ }); }; + GrChangeActionsInterface.prototype.setActionOverflow = function(type, key, + overflow) { + return this._el.setActionOverflow(type, key, overflow); + }; + GrChangeActionsInterface.prototype.setActionHidden = function(type, key, hidden) { return this._el.setActionHidden(type, key, hidden); diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html index 20b5dcb00e..24da5f7e78 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.html @@ -130,8 +130,8 @@ breaking changes to gr-change-actions won’t be noticed. var button = element.$$('[data-action-key="' + key + '"]'); assert.isOk(button); assert.isFalse(button.hasAttribute('hidden')); - changeActions.setActionHidden(changeActions.ActionType.REVISION, key, - true); + changeActions.setActionHidden( + changeActions.ActionType.REVISION, key, true); flush(function() { var button = element.$$('[data-action-key="' + key + '"]'); assert.isNotOk(button); @@ -139,5 +139,21 @@ breaking changes to gr-change-actions won’t be noticed. }); }); }); + + test('move action button to overflow', function(done) { + var key = changeActions.add(changeActions.ActionType.REVISION, 'Bork!'); + flush(function() { + assert.isTrue(element.$.moreActions.hidden); + assert.isOk(element.$$('[data-action-key="' + key + '"]')); + changeActions.setActionOverflow( + changeActions.ActionType.REVISION, key, true); + flush(function() { + assert.isNotOk(element.$$('[data-action-key="' + key + '"]')); + assert.isFalse(element.$.moreActions.hidden); + assert.strictEqual(element.$.moreActions.items[0].name, 'Bork!'); + done(); + }); + }); + }); });