Change `Cancel` to `Close` in gr-editor-view

Previously, tapping `Cancel` would discard any unsaved changes and
navigate directly to the edit patch set in the change view. This was
wrong for a couple of reasons:
- If the user came from a non-edit patch set and cancelled immediately,
  they would be taken to an unexpected patch set
- If the user started modifying a file and never saved, `Cancel` would
  navigate to a patch set that may not exist at all

This change restyles the buttons on the editor view to flat link
buttons, re-orders them, renames `Cancel` to `Close`, and modifies the
`Close` behavior to only navigate to the edit patch set if a file was
successfully modified.

Bug: Issue 4437
Change-Id: I130c417e8cda961f4d7cd9e384137f8bc2cba181

polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.html

diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.html b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.html
index 23f1824..56c2ca8 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.html
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.html
@@ -70,11 +70,15 @@
             on-changed="_handlePathChanged"></gr-editable-label>
         <span class="rightControls">
           <gr-button
+              id="close"
+              link
+              on-tap="_handleCloseTap">Close</gr-button>
+          <gr-button
               id="save"
               disabled$="[[_saveDisabled]]"
               primary
+              link
               on-tap="_saveEdit">Save</gr-button>
-          <gr-button id="cancel" on-tap="_handleCancelTap">Cancel</gr-button>
         </span>
       </header>
     </gr-fixed-panel>

polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js

diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
index 5bbb9e9..23f76db 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js
@@ -54,6 +54,10 @@
         type: Boolean,
         value: false,
       },
+      _successfulSave: {
+        type: Boolean,
+        value: false,
+      },
       _saveDisabled: {
         type: Boolean,
         value: true,
@@ -119,12 +123,15 @@
       return this.$.restAPI.renameFileInChangeEdit(this._changeNum,
           this._path, path).then(res => {
             if (!res.ok) { return; }
+
+            this._successfulSave = true;
             this._viewEditInChangeView();
           });
     },
 
     _viewEditInChangeView() {
-      Gerrit.Nav.navigateToChange(this._change, this.EDIT_NAME);
+      const patch = this._successfulSave ? this.EDIT_NAME : this._patchNum;
+      Gerrit.Nav.navigateToChange(this._change, patch);
     },
 
     _getFileData(changeNum, path, patchNum) {
@@ -145,7 +152,10 @@
           this._newContent).then(res => {
             this._saving = false;
             this._showAlert(res.ok ? SAVED_MESSAGE : SAVE_FAILED_MSG);
-            if (res.ok) { this._content = this._newContent; }
+            if (!res.ok) { return; }
+
+            this._content = this._newContent;
+            this._successfulSave = true;
           });
     },
 
@@ -161,7 +171,7 @@
       return content === newContent;
     },
 
-    _handleCancelTap() {
+    _handleCloseTap() {
       // TODO(kaspern): Add a confirm dialog if there are unsaved changes.
       this._viewEditInChangeView();
     },

polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html

diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html
index 74315f3..5f76a17 100644
--- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html
+++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html
@@ -94,14 +94,14 @@
     });
   });
 
-  test('edit file path', done => {
+  test('edit file path', () => {
     element._changeNum = mockParams.changeNum;
     element._path = mockParams.path;
     savePathStub.onFirstCall().returns(Promise.resolve({}));
     savePathStub.onSecondCall().returns(Promise.resolve({ok: true}));
 
     // Calling with the same path should not navigate.
-    element._handlePathChanged({detail: mockParams.path}).then(() => {
+    return element._handlePathChanged({detail: mockParams.path}).then(() => {
       assert.isFalse(savePathStub.called);
         // !ok response
       element._handlePathChanged({detail: 'newPath'}).then(() => {
@@ -110,7 +110,7 @@
         // ok response
         element._handlePathChanged({detail: 'newPath'}).then(() => {
           assert.isTrue(navigateStub.called);
-          done();
+          assert.isTrue(element._successfulSave);
         });
       });
     });
@@ -195,18 +195,19 @@
         assert.isFalse(navigateStub.called);
         assert.isTrue(element.$.save.hasAttribute('disabled'));
         assert.equal(element._content, element._newContent);
+        assert.isTrue(element._successfulSave);
       });
     });
 
-    test('file modification and cancel', () => {
-      const cancelSpy = sandbox.spy(element, '_handleCancelTap');
+    test('file modification and close', () => {
+      const closeSpy = sandbox.spy(element, '_handleCloseTap');
       element._newContent = newText;
       flushAsynchronousOperations();
 
       assert.isFalse(element.$.save.hasAttribute('disabled'));
 
-      MockInteractions.tap(element.$.cancel);
-      assert.isTrue(cancelSpy.called);
+      MockInteractions.tap(element.$.close);
+      assert.isTrue(closeSpy.called);
       assert.isFalse(saveFileStub.called);
       assert.isTrue(navigateStub.called);
     });
@@ -284,5 +285,19 @@
 
     element._showAlert('test message');
   });
+
+  test('_viewEditInChangeView respects _patchNum', () => {
+    navigateStub.restore();
+    const navStub = sandbox.stub(Gerrit.Nav, 'navigateToChange');
+    element._patchNum = element.EDIT_NAME;
+    element._viewEditInChangeView();
+    assert.equal(navStub.lastCall.args[1], element.EDIT_NAME);
+    element._patchNum = '1';
+    element._viewEditInChangeView();
+    assert.equal(navStub.lastCall.args[1], '1');
+    element._successfulSave = true;
+    element._viewEditInChangeView();
+    assert.equal(navStub.lastCall.args[1], element.EDIT_NAME);
+  });
 });
 </script>