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
This commit is contained in:
		@@ -142,6 +142,7 @@ shortcuts are.
 | 
				
			|||||||
    UP_TO_CHANGE: 'UP_TO_CHANGE',
 | 
					    UP_TO_CHANGE: 'UP_TO_CHANGE',
 | 
				
			||||||
    TOGGLE_DIFF_MODE: 'TOGGLE_DIFF_MODE',
 | 
					    TOGGLE_DIFF_MODE: 'TOGGLE_DIFF_MODE',
 | 
				
			||||||
    REFRESH_CHANGE: 'REFRESH_CHANGE',
 | 
					    REFRESH_CHANGE: 'REFRESH_CHANGE',
 | 
				
			||||||
 | 
					    EDIT_TOPIC: 'EDIT_TOPIC',
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    NEXT_LINE: 'NEXT_LINE',
 | 
					    NEXT_LINE: 'NEXT_LINE',
 | 
				
			||||||
    PREV_LINE: 'PREV_LINE',
 | 
					    PREV_LINE: 'PREV_LINE',
 | 
				
			||||||
@@ -223,6 +224,8 @@ shortcuts are.
 | 
				
			|||||||
      'Refresh list of changes');
 | 
					      'Refresh list of changes');
 | 
				
			||||||
  _describe(Shortcut.TOGGLE_CHANGE_STAR, ShortcutSection.ACTIONS,
 | 
					  _describe(Shortcut.TOGGLE_CHANGE_STAR, ShortcutSection.ACTIONS,
 | 
				
			||||||
      'Star/unstar change');
 | 
					      '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.NEXT_LINE, ShortcutSection.DIFFS, 'Go to next line');
 | 
				
			||||||
  _describe(Shortcut.PREV_LINE, ShortcutSection.DIFFS, 'Go to previous line');
 | 
					  _describe(Shortcut.PREV_LINE, ShortcutSection.DIFFS, 'Go to previous line');
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -244,6 +244,7 @@ limitations under the License.
 | 
				
			|||||||
              is="dom-if"
 | 
					              is="dom-if"
 | 
				
			||||||
              if="[[_showAddTopic(change.*, _settingTopic)]]">
 | 
					              if="[[_showAddTopic(change.*, _settingTopic)]]">
 | 
				
			||||||
            <gr-editable-label
 | 
					            <gr-editable-label
 | 
				
			||||||
 | 
					                class="topicEditableLabel"
 | 
				
			||||||
                label-text="Add a topic"
 | 
					                label-text="Add a topic"
 | 
				
			||||||
                value="[[change.topic]]"
 | 
					                value="[[change.topic]]"
 | 
				
			||||||
                max-length="1024"
 | 
					                max-length="1024"
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -343,5 +343,12 @@
 | 
				
			|||||||
    _computeIsMutable(account) {
 | 
					    _computeIsMutable(account) {
 | 
				
			||||||
      return !!Object.keys(account).length;
 | 
					      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();
 | 
				
			||||||
 | 
					    },
 | 
				
			||||||
  });
 | 
					  });
 | 
				
			||||||
})();
 | 
					})();
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -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', () => {
 | 
					    suite('plugin endpoints', () => {
 | 
				
			||||||
      test('endpoint params', done => {
 | 
					      test('endpoint params', done => {
 | 
				
			||||||
        element.change = {labels: {}};
 | 
					        element.change = {labels: {}};
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -291,6 +291,7 @@
 | 
				
			|||||||
        [this.Shortcut.EXPAND_ALL_MESSAGES]: '_handleExpandAllMessages',
 | 
					        [this.Shortcut.EXPAND_ALL_MESSAGES]: '_handleExpandAllMessages',
 | 
				
			||||||
        [this.Shortcut.COLLAPSE_ALL_MESSAGES]: '_handleCollapseAllMessages',
 | 
					        [this.Shortcut.COLLAPSE_ALL_MESSAGES]: '_handleCollapseAllMessages',
 | 
				
			||||||
        [this.Shortcut.OPEN_DIFF_PREFS]: '_handleOpenDiffPrefsShortcut',
 | 
					        [this.Shortcut.OPEN_DIFF_PREFS]: '_handleOpenDiffPrefsShortcut',
 | 
				
			||||||
 | 
					        [this.Shortcut.EDIT_TOPIC]: '_handleEditTopic',
 | 
				
			||||||
      };
 | 
					      };
 | 
				
			||||||
    },
 | 
					    },
 | 
				
			||||||
 | 
					
 | 
				
			||||||
@@ -944,6 +945,14 @@
 | 
				
			|||||||
      this.$.downloadOverlay.open();
 | 
					      this.$.downloadOverlay.open();
 | 
				
			||||||
    },
 | 
					    },
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    _handleEditTopic(e) {
 | 
				
			||||||
 | 
					      if (this.shouldSuppressKeyboardShortcut(e) ||
 | 
				
			||||||
 | 
					          this.modifierPressed(e)) { return; }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      e.preventDefault();
 | 
				
			||||||
 | 
					      this.$.metadata.editTopic();
 | 
				
			||||||
 | 
					    },
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    _handleRefreshChange(e) {
 | 
					    _handleRefreshChange(e) {
 | 
				
			||||||
      if (this.shouldSuppressKeyboardShortcut(e)) { return; }
 | 
					      if (this.shouldSuppressKeyboardShortcut(e)) { return; }
 | 
				
			||||||
      e.preventDefault();
 | 
					      e.preventDefault();
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -54,6 +54,7 @@ limitations under the License.
 | 
				
			|||||||
    kb.bindShortcut(kb.Shortcut.EXPAND_ALL_MESSAGES, 'x');
 | 
					    kb.bindShortcut(kb.Shortcut.EXPAND_ALL_MESSAGES, 'x');
 | 
				
			||||||
    kb.bindShortcut(kb.Shortcut.COLLAPSE_ALL_MESSAGES, 'z');
 | 
					    kb.bindShortcut(kb.Shortcut.COLLAPSE_ALL_MESSAGES, 'z');
 | 
				
			||||||
    kb.bindShortcut(kb.Shortcut.OPEN_DIFF_PREFS, ',');
 | 
					    kb.bindShortcut(kb.Shortcut.OPEN_DIFF_PREFS, ',');
 | 
				
			||||||
 | 
					    kb.bindShortcut(kb.Shortcut.EDIT_TOPIC, 't');
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    let element;
 | 
					    let element;
 | 
				
			||||||
    let sandbox;
 | 
					    let sandbox;
 | 
				
			||||||
@@ -109,6 +110,12 @@ limitations under the License.
 | 
				
			|||||||
    });
 | 
					    });
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    suite('keyboard shortcuts', () => {
 | 
					    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', () => {
 | 
					      test('S should toggle the CL star', () => {
 | 
				
			||||||
        const starStub = sandbox.stub(element.$.changeStar, 'toggleStar');
 | 
					        const starStub = sandbox.stub(element.$.changeStar, 'toggleStar');
 | 
				
			||||||
        MockInteractions.pressAndReleaseKeyOn(element, 83, null, 's');
 | 
					        MockInteractions.pressAndReleaseKeyOn(element, 83, null, 's');
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -203,6 +203,8 @@
 | 
				
			|||||||
          this.Shortcut.TOGGLE_CHANGE_STAR, 's');
 | 
					          this.Shortcut.TOGGLE_CHANGE_STAR, 's');
 | 
				
			||||||
      this.bindShortcut(
 | 
					      this.bindShortcut(
 | 
				
			||||||
          this.Shortcut.REFRESH_CHANGE_LIST, 'shift+r');
 | 
					          this.Shortcut.REFRESH_CHANGE_LIST, 'shift+r');
 | 
				
			||||||
 | 
					      this.bindShortcut(
 | 
				
			||||||
 | 
					          this.Shortcut.EDIT_TOPIC, 't');
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      this.bindShortcut(
 | 
					      this.bindShortcut(
 | 
				
			||||||
          this.Shortcut.OPEN_REPLY_DIALOG, 'a');
 | 
					          this.Shortcut.OPEN_REPLY_DIALOG, 'a');
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -99,6 +99,12 @@
 | 
				
			|||||||
      });
 | 
					      });
 | 
				
			||||||
    },
 | 
					    },
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    open() {
 | 
				
			||||||
 | 
					      return this._open().then(() => {
 | 
				
			||||||
 | 
					        this.$.input.$.input.focus();
 | 
				
			||||||
 | 
					      });
 | 
				
			||||||
 | 
					    },
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    _open(...args) {
 | 
					    _open(...args) {
 | 
				
			||||||
      this.$.dropdown.open();
 | 
					      this.$.dropdown.open();
 | 
				
			||||||
      this._inputText = this.value;
 | 
					      this._inputText = this.value;
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user