Add 't to add topic' to change view

Currently, this shortcut only works if no topic is already set. Fixing
that case will require a moderate refactor of gr-editable-label and
gr-change-metadata to depend less on the actual change data and more on
component local state, but this is good enough to unblock 2.16.

Bug: Issue 9778
Change-Id: I85d27179268cde6559b78d9f3d4fd34a881a8ca2
(cherry picked from commit 42eda5e7d2)
This commit is contained in:
Kasper Nilsson
2018-11-12 16:19:35 -08:00
committed by Paladox none
parent 679c6d9790
commit da5aa87cbc
8 changed files with 49 additions and 0 deletions

View File

@@ -142,6 +142,7 @@ shortcuts are.
UP_TO_CHANGE: 'UP_TO_CHANGE',
TOGGLE_DIFF_MODE: 'TOGGLE_DIFF_MODE',
REFRESH_CHANGE: 'REFRESH_CHANGE',
EDIT_TOPIC: 'EDIT_TOPIC',
NEXT_LINE: 'NEXT_LINE',
PREV_LINE: 'PREV_LINE',
@@ -223,6 +224,8 @@ shortcuts are.
'Refresh list of changes');
_describe(Shortcut.TOGGLE_CHANGE_STAR, ShortcutSection.ACTIONS,
'Star/unstar change');
_describe(Shortcut.EDIT_TOPIC, ShortcutSection.ACTIONS,
'Add a change topic');
_describe(Shortcut.NEXT_LINE, ShortcutSection.DIFFS, 'Go to next line');
_describe(Shortcut.PREV_LINE, ShortcutSection.DIFFS, 'Go to previous line');

View File

@@ -244,6 +244,7 @@ limitations under the License.
is="dom-if"
if="[[_showAddTopic(change.*, _settingTopic)]]">
<gr-editable-label
class="topicEditableLabel"
label-text="Add a topic"
value="[[change.topic]]"
max-length="1024"

View File

@@ -343,5 +343,12 @@
_computeIsMutable(account) {
return !!Object.keys(account).length;
},
editTopic() {
if (this._topicReadOnly || this.change.topic) { return; }
// Cannot use `this.$.ID` syntax because the element exists inside of a
// dom-if.
this.$$('.topicEditableLabel').open();
},
});
})();

View File

@@ -587,6 +587,20 @@ limitations under the License.
});
});
test('editTopic', () => {
element.account = {test: true};
element.change = {actions: {topic: {enabled: true}}};
flushAsynchronousOperations();
const label = element.$$('.topicEditableLabel');
assert.ok(label);
sandbox.stub(label, 'open');
element.editTopic();
flushAsynchronousOperations();
assert.isTrue(label.open.called);
});
suite('plugin endpoints', () => {
test('endpoint params', done => {
element.change = {labels: {}};

View File

@@ -281,6 +281,7 @@
[this.Shortcut.EXPAND_ALL_MESSAGES]: '_handleExpandAllMessages',
[this.Shortcut.COLLAPSE_ALL_MESSAGES]: '_handleCollapseAllMessages',
[this.Shortcut.OPEN_DIFF_PREFS]: '_handleOpenDiffPrefsShortcut',
[this.Shortcut.EDIT_TOPIC]: '_handleEditTopic',
};
},
@@ -927,6 +928,14 @@
this.$.downloadOverlay.open();
},
_handleEditTopic(e) {
if (this.shouldSuppressKeyboardShortcut(e) ||
this.modifierPressed(e)) { return; }
e.preventDefault();
this.$.metadata.editTopic();
},
_handleRefreshChange(e) {
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
e.preventDefault();

View File

@@ -54,6 +54,7 @@ limitations under the License.
kb.bindShortcut(kb.Shortcut.EXPAND_ALL_MESSAGES, 'x');
kb.bindShortcut(kb.Shortcut.COLLAPSE_ALL_MESSAGES, 'z');
kb.bindShortcut(kb.Shortcut.OPEN_DIFF_PREFS, ',');
kb.bindShortcut(kb.Shortcut.EDIT_TOPIC, 't');
let element;
let sandbox;
@@ -95,6 +96,12 @@ limitations under the License.
};
suite('keyboard shortcuts', () => {
test('t to add topic', () => {
const editStub = sandbox.stub(element.$.metadata, 'editTopic');
MockInteractions.pressAndReleaseKeyOn(element, 83, null, 't');
assert(editStub.called);
});
test('S should toggle the CL star', () => {
const starStub = sandbox.stub(element.$.changeStar, 'toggleStar');
MockInteractions.pressAndReleaseKeyOn(element, 83, null, 's');

View File

@@ -203,6 +203,8 @@
this.Shortcut.TOGGLE_CHANGE_STAR, 's');
this.bindShortcut(
this.Shortcut.REFRESH_CHANGE_LIST, 'shift+r');
this.bindShortcut(
this.Shortcut.EDIT_TOPIC, 't');
this.bindShortcut(
this.Shortcut.OPEN_REPLY_DIALOG, 'a');

View File

@@ -99,6 +99,12 @@
});
},
open() {
return this._open().then(() => {
this.$.input.$.input.focus();
});
},
_open(...args) {
this.$.dropdown.open();
this._inputText = this.value;