From 200061e281c91073209c71b7642d803f488f29b5 Mon Sep 17 00:00:00 2001
From: Becky Siegel <beckysiegel@google.com>
Date: Fri, 6 Jan 2017 10:15:10 -0800
Subject: [PATCH] Add keyboard shortcut for star toggle

This change adds a keyboard shortcut to star a CL from the change view,
to match GWT UI features. The 's' key toggles the the CL star.

Bug: Issue 5192
Change-Id: I6f471ac2638815b78cb374326b0e37eaad0fa30d
---
 .../elements/change/gr-change-view/gr-change-view.html   | 3 ++-
 .../app/elements/change/gr-change-view/gr-change-view.js | 9 +++++++++
 .../change/gr-change-view/gr-change-view_test.html       | 6 ++++++
 .../elements/shared/gr-change-star/gr-change-star.html   | 2 +-
 .../app/elements/shared/gr-change-star/gr-change-star.js | 2 +-
 5 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 7bd8f1bf9d..26e226e86b 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -241,7 +241,8 @@ limitations under the License.
     <div class="container" hidden$="{{_loading}}">
       <div class="header">
         <span class="header-title">
-          <gr-change-star change="{{_change}}" hidden$="[[!_loggedIn]]"></gr-change-star>
+          <gr-change-star id="changeStar"
+              change="{{_change}}" hidden$="[[!_loggedIn]]"></gr-change-star>
           <a href$="[[_computeChangePermalink(_change._number)]]">[[_change._number]]</a><!--
        --><span class="changeStatus">[[_computeChangeStatus(_change, _patchRange.patchNum)]]</span><!--
        -->:
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index eb23c616b2..12b3a169c6 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -127,6 +127,7 @@
       'shift+r': '_handleCapitalRKey',
       'a': '_handleAKey',
       'd': '_handleDKey',
+      's': '_handleSKey',
       'u': '_handleUKey',
     },
 
@@ -636,6 +637,14 @@
       this._switchToMostRecentPatchNum();
     },
 
+    _handleSKey: function(e) {
+      if (this.shouldSuppressKeyboardShortcut(e) ||
+          this.modifierPressed(e)) { return; }
+
+      e.preventDefault();
+      this.$.changeStar.toggleStar();
+    },
+
     _handleUKey: function(e) {
       if (this.shouldSuppressKeyboardShortcut(e)) { return; }
 
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index c92d97a332..62598e61ed 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -51,6 +51,12 @@ limitations under the License.
     });
 
     suite('keyboard shortcuts', function() {
+      test('S should toggle the CL star', function() {
+        var starStub = sandbox.stub(element.$.changeStar, 'toggleStar');
+        MockInteractions.pressAndReleaseKeyOn(element, 83, null, 's');
+        assert(starStub.called);
+      });
+
       test('U should navigate to / if no backPage set', function() {
         var showStub = sandbox.stub(page, 'show');
         MockInteractions.pressAndReleaseKeyOn(element, 85, null, 'u');
diff --git a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.html b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.html
index 05fe10bb9b..93bf5ee796 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.html
+++ b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.html
@@ -42,7 +42,7 @@ limitations under the License.
         fill: #ffac33;
       }
     </style>
-    <button class$="[[_computeStarClass(change.starred)]]" on-tap="_handleStarTap">
+    <button class$="[[_computeStarClass(change.starred)]]" on-tap="toggleStar">
       <!-- Public Domain image from the Noun Project: https://thenounproject.com/search/?q=star&i=25969 -->
       <svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" x="0px" y="0px" viewBox="0 0 100 100" enable-background="new 0 0 100 100" xml:space="preserve"><path d="M26.439,95.601c-5.608,2.949-9.286,0.276-8.216-5.968l4.5-26.237L3.662,44.816c-4.537-4.423-3.132-8.746,3.137-9.657  l26.343-3.829L44.923,7.46c2.804-5.682,7.35-5.682,10.154,0l11.78,23.87l26.343,3.829c6.27,0.911,7.674,5.234,3.138,9.657  L77.277,63.397l4.501,26.237c1.07,6.244-2.608,8.916-8.216,5.968L50,83.215L26.439,95.601z"></path></svg>
     </button>
diff --git a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.js b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.js
index 23c56b4329..beb0ff1c9b 100644
--- a/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.js
+++ b/polygerrit-ui/app/elements/shared/gr-change-star/gr-change-star.js
@@ -34,7 +34,7 @@
       return classes.join(' ');
     },
 
-    _handleStarTap: function() {
+    toggleStar: function() {
       var newVal = !this.change.starred;
       this.set('change.starred', newVal);
       this._xhrPromise = this.$.restAPI.saveChangeStarred(this.change._number,