Move some change actions into an overflow menu Uncommonly-used (and destructive) change actions are moved into an overflow menu implemented with a GR-DROPDOWN. Feature: Issue 5315 Change-Id: I2731fd1baadf5ef598fd7c1ab0ba6d021df1f326
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 a6c32b0..f54cfc9 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
@@ -19,6 +19,7 @@ <link rel="import" href="../../../behaviors/rest-client-behavior.html"> <link rel="import" href="../../shared/gr-button/gr-button.html"> +<link rel="import" href="../../shared/gr-dropdown/gr-dropdown.html"> <link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html"> <link rel="import" href="../../shared/gr-overlay/gr-overlay.html"> <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html"> @@ -38,7 +39,8 @@ section { display: inline-block; } - gr-button { + gr-button, + gr-dropdown { margin-left: .5em; } gr-button:before { @@ -63,10 +65,20 @@ } </style> <div> - <section hidden$="[[_shouldHideActions(_allActionValues.*, _loading)]]"> + <gr-dropdown + id="moreActions" + down-arrow + vertical-offset="32" + horizontal-align="right" + on-tap-item-abandon="_handleAbandonTap" + on-tap-item-cherrypick="_handleCherrypickTap" + on-tap-item-delete="_handleDeleteTap" + hidden$="[[_shouldHideActions(_menuActions.*, _loading)]]" + items="[[_menuActions]]">More</gr-dropdown> + <section hidden$="[[_shouldHideActions(_topLevelActions.*, _loading)]]"> <template is="dom-repeat" - items="[[_allActionValues]]" + items="[[_topLevelActions]]" as="action"> <gr-button title$="[[action.title]]" hidden$="[[_computeActionHidden(action.__key, _hiddenActions.*)]]"
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 0f7d5be..331c3c5 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,6 +87,16 @@ method: 'POST', }; + /** + * Keys for actions to appear in the overflow menu rather than the top-level + * set of action buttons. + */ + var MENU_ACTION_KEYS = [ + 'abandon', + 'cherrypick', + '/', // '/' is the key for the delete action. + ]; + Polymer({ is: 'gr-change-actions', @@ -141,6 +151,15 @@ computed: '_computeAllActions(actions.*, revisionActions.*,' + 'primaryActionKeys.*, _additionalActions.*, change)', }, + _topLevelActions: { + type: Array, + computed: '_computeTopLevelActions(_allActionValues.*)', + }, + _menuActions: { + type: Array, + computed: '_computeMenuActions(_allActionValues.*, _hiddenActions.*)', + }, + _additionalActions: { type: Array, value: function() { return []; }, @@ -433,12 +452,8 @@ var type = el.getAttribute('data-action-type'); if (type === ActionType.REVISION) { this._handleRevisionAction(key); - } else if (key === ChangeActions.DELETE) { - this._showActionDialog(this.$.confirmDeleteDialog); } 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; @@ -455,10 +470,6 @@ case RevisionActions.REBASE: this._showActionDialog(this.$.confirmRebase); break; - case RevisionActions.CHERRYPICK: - this.$.confirmCherrypick.branch = ''; - this._showActionDialog(this.$.confirmCherrypick); - break; case RevisionActions.SUBMIT: if (!this._canSubmitChange()) { return; @@ -632,6 +643,19 @@ }.bind(this)).then(this._handleResponseError.bind(this)); }, + _handleAbandonTap: function() { + this._showActionDialog(this.$.confirmAbandonDialog); + }, + + _handleCherrypickTap: function() { + this.$.confirmCherrypick.branch = ''; + this._showActionDialog(this.$.confirmCherrypick); + }, + + _handleDeleteTap: function() { + this._showActionDialog(this.$.confirmDeleteDialog); + }, + /** * Merge sources of change actions into a single ordered array of action * values. @@ -685,5 +709,25 @@ // Otherwise, sort by the button label. return actionA.label > actionB.label ? 1 : -1; }, + + _computeTopLevelActions: function(actionRecord) { + return actionRecord.base.filter(function(a) { + return MENU_ACTION_KEYS.indexOf(a.__key) === -1; + }); + }, + + _computeMenuActions: function(actionRecord, hiddenActionsRecord) { + 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, }; + }); + }, }); })();
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 27a4c12..dbdf00f 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
@@ -133,11 +133,10 @@ }); }); - test('hide change action', function(done) { + test('hide menu action', function(done) { flush(function() { - var buttonEl = element.$$('[data-action-key="/"]'); + var buttonEl = element.$.moreActions.$$('span[data-id="delete"]'); assert.isOk(buttonEl); - assert.isFalse(buttonEl.hasAttribute('hidden')); assert.throws(element.setActionHidden.bind(element, 'invalid type')); element.setActionHidden(element.ActionType.CHANGE, element.ChangeActions.DELETE, true); @@ -146,16 +145,14 @@ element.ChangeActions.DELETE, true); assert.lengthOf(element._hiddenActions, 1); flush(function() { - var buttonEl = element.$$('[data-action-key="/"]'); - assert.isOk(buttonEl); - assert.isTrue(buttonEl.hasAttribute('hidden')); + var buttonEl = element.$.moreActions.$$('span[data-id="delete"]'); + assert.isNotOk(buttonEl); element.setActionHidden(element.ActionType.CHANGE, element.RevisionActions.DELETE, false); flush(function() { - var buttonEl = element.$$('[data-action-key="/"]'); + var buttonEl = element.$.moreActions.$$('span[data-id="delete"]'); assert.isOk(buttonEl); - assert.isFalse(buttonEl.hasAttribute('hidden')); done(); }); }); @@ -167,7 +164,8 @@ flush(function() { var buttonEls = Polymer.dom(element.root) .querySelectorAll('gr-button'); - assert.equal(buttonEls.length, 6); + var menuItems = element.$.moreActions.items; + assert.equal(buttonEls.length + menuItems.length, 6); assert.isFalse(element.hidden); done(); }); @@ -175,18 +173,18 @@ test('delete buttons have explicit labels', function(done) { flush(function() { - var buttonEls = - Polymer.dom(element.root).querySelectorAll('[data-action-key="/"]'); - assert.equal(buttonEls.length, 2); - assert.notEqual(buttonEls[0].getAttribute('data-label'), - buttonEls[1].getAttribute['data-label']); + var deleteItems = element.$.moreActions.items.filter(function(item) { + return item.id === 'delete'; + }); + assert.equal(deleteItems.length, 2); + assert.notEqual(deleteItems[0].name, deleteItems[1].name) assert.isTrue( - buttonEls[0].getAttribute('data-label') === 'Delete Revision' || - buttonEls[0].getAttribute('data-label') === 'Delete Change' + deleteItems[0].name === 'Delete Revision' || + deleteItems[0].name === 'Delete Change' ); assert.isTrue( - buttonEls[1].getAttribute('data-label') === 'Delete Revision' || - buttonEls[1].getAttribute('data-label') === 'Delete Change' + deleteItems[1].name === 'Delete Revision' || + deleteItems[1].name === 'Delete Change' ); done(); }); @@ -295,10 +293,7 @@ flushAsynchronousOperations(); assert.isFalse(element.$.confirmRebase.hidden); - var cherryPickButton = - element.$$('gr-button[data-action-key="cherrypick"]'); - assert.ok(cherryPickButton); - MockInteractions.tap(cherryPickButton); + element._handleCherrypickTap(); flushAsynchronousOperations(); assert.isTrue(element.$.confirmRebase.hidden); assert.isFalse(element.$.confirmCherrypick.hidden); @@ -321,9 +316,7 @@ }); test('works', function() { - var cherryPickButton = - element.$$('gr-button[data-action-key="cherrypick"]'); - MockInteractions.tap(cherryPickButton); + element._handleCherrypickTap(); var action = { __key: 'cherrypick', __type: 'revision', @@ -360,12 +353,10 @@ }); test('branch name cleared when re-open cherrypick', function() { - var cherryPickButton = - element.$$('gr-button[data-action-key="cherrypick"]'); var emptyBranchName = ''; element.$.confirmCherrypick.branch = 'master'; - MockInteractions.tap(cherryPickButton); + element._handleCherrypickTap(); assert.equal(element.$.confirmCherrypick.branch, emptyBranchName); }); }); @@ -467,12 +458,6 @@ var fireActionStub; var deleteAction; - var tapDeleteAction = function() { - var deleteButton = element.$$('gr-button[data-action-key=\'/\']'); - MockInteractions.tap(deleteButton); - flushAsynchronousOperations(); - }; - setup(function() { fireActionStub = sinon.stub(element, '_fireAction'); element.change = { @@ -494,12 +479,12 @@ }); test('does not delete on action', function() { - tapDeleteAction(); + element._handleDeleteTap(); assert.isFalse(fireActionStub.called); }); test('shows confirm dialog', function() { - tapDeleteAction(); + element._handleDeleteTap(); assert.isFalse(element.$$('#confirmDeleteDialog').hidden); MockInteractions.tap( element.$$('#confirmDeleteDialog').$$('gr-button[primary]')); @@ -508,7 +493,7 @@ }); test('hides delete confirm on cancel', function() { - tapDeleteAction(); + element._handleDeleteTap(); MockInteractions.tap( element.$$('#confirmDeleteDialog').$$('gr-button:not([primary])')); flushAsynchronousOperations();
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 1882500..f640dd6 100644 --- a/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html +++ b/polygerrit-ui/app/elements/shared/gr-dropdown/gr-dropdown.html
@@ -49,11 +49,6 @@ gr-button[link] { padding: 1em 0; } - gr-button:focus:not([primary]):not([secondary]), - gr-button:hover:not([primary]):not([secondary]) { - background-color: transparent; - border-color: transparent; - } ul { list-style: none; }