Revamp 'Save' behavior
'Save' no longer navigates away on success. Instead, an alert is shown
when saving, as well as when the save succeeds or fails. In addition,
the save button is disabled while waiting for the save request to
return.
Bug: Issue 7453
Change-Id: Ied737ffc99acbc2f8831b04a8cac5b4c795b65f4
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 e2d368f..67bb7d1 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
@@ -14,6 +14,10 @@
(function() {
'use strict';
+ const SAVING_MESSAGE = 'Saving changes...';
+ const SAVED_MESSAGE = 'All changes saved';
+ const SAVE_FAILED_MSG = 'Failed to save changes';
+
Polymer({
is: 'gr-editor-view',
@@ -23,6 +27,12 @@
* @event title-change
*/
+ /**
+ * Fired to notify the user of
+ *
+ * @event show-alert
+ */
+
properties: {
/**
* URL params passed from the router.
@@ -39,10 +49,14 @@
_type: String,
_content: String,
_newContent: String,
+ _saving: {
+ type: Boolean,
+ value: false,
+ },
_saveDisabled: {
type: Boolean,
value: true,
- computed: '_computeSaveDisabled(_content, _newContent)',
+ computed: '_computeSaveDisabled(_content, _newContent, _saving)',
},
_prefs: Object,
},
@@ -121,14 +135,24 @@
},
_saveEdit() {
+ this._saving = true;
+ this._showAlert(SAVING_MESSAGE);
return this.$.restAPI.saveChangeEdit(this._changeNum, this._path,
this._newContent).then(res => {
- if (!res.ok) { return; }
- this._viewEditInChangeView();
+ this._saving = false;
+ this._showAlert(res.ok ? SAVED_MESSAGE : SAVE_FAILED_MSG);
});
},
- _computeSaveDisabled(content, newContent) {
+ _showAlert(message) {
+ this.dispatchEvent(new CustomEvent('show-alert', {
+ detail: {message},
+ bubbles: true,
+ }));
+ },
+
+ _computeSaveDisabled(content, newContent, saving) {
+ if (saving) { return true; }
return content === newContent;
},
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 ab18494..254575a 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
@@ -142,39 +142,55 @@
assert.isTrue(element.$.save.hasAttribute('disabled'));
});
- test('file modification and save, !ok response', done => {
+ test('file modification and save, !ok response', () => {
const saveSpy = sandbox.spy(element, '_saveEdit');
+ const alertStub = sandbox.stub(element, '_showAlert');
saveFileStub.returns(Promise.resolve({ok: false}));
element._newContent = newText;
flushAsynchronousOperations();
assert.isFalse(element.$.save.hasAttribute('disabled'));
+ assert.isFalse(element._saving);
MockInteractions.tap(element.$.save);
- assert(saveSpy.called);
- saveSpy.lastCall.returnValue.then(() => {
+ assert.isTrue(saveSpy.called);
+ assert.equal(alertStub.lastCall.args[0], 'Saving changes...');
+ assert.isTrue(element._saving);
+ assert.isTrue(element.$.save.hasAttribute('disabled'));
+
+ return saveSpy.lastCall.returnValue.then(() => {
assert.isTrue(saveFileStub.called);
+ assert.isFalse(element._saving);
+ assert.equal(alertStub.lastCall.args[0], 'Failed to save changes');
assert.deepEqual(saveFileStub.lastCall.args,
[mockParams.changeNum, mockParams.path, newText]);
assert.isFalse(navigateStub.called);
- done();
+ assert.isFalse(element.$.save.hasAttribute('disabled'));
});
});
- test('file modification and save', done => {
+ test('file modification and save', () => {
const saveSpy = sandbox.spy(element, '_saveEdit');
+ const alertStub = sandbox.stub(element, '_showAlert');
saveFileStub.returns(Promise.resolve({ok: true}));
element._newContent = newText;
flushAsynchronousOperations();
+ assert.isFalse(element._saving);
assert.isFalse(element.$.save.hasAttribute('disabled'));
MockInteractions.tap(element.$.save);
assert.isTrue(saveSpy.called);
- saveSpy.lastCall.returnValue.then(() => {
+ assert.equal(alertStub.lastCall.args[0], 'Saving changes...');
+ assert.isTrue(element._saving);
+ assert.isTrue(element.$.save.hasAttribute('disabled'));
+
+ return saveSpy.lastCall.returnValue.then(() => {
assert.isTrue(saveFileStub.called);
- assert.isTrue(navigateStub.called);
- done();
+ assert.isFalse(element._saving);
+ assert.equal(alertStub.lastCall.args[0], 'All changes saved');
+ assert.isFalse(navigateStub.called);
+ assert.isFalse(element.$.save.hasAttribute('disabled'));
});
});
@@ -254,5 +270,15 @@
});
});
});
+
+ test('_showAlert', done => {
+ element.addEventListener('show-alert', e => {
+ assert.deepEqual(e.detail, {message: 'test message'});
+ assert.isTrue(e.bubbles);
+ done();
+ });
+
+ element._showAlert('test message');
+ });
});
</script>
