Move diff overlay to gr-diff-preferences
Previously, gr-diff-preferences was the contents that was displayed in
an overlay contained in the gr-diff-view. There were a handful of
functions that crossed between the two-- the diff preferences element
would fire events that needed to be handled by the diff view.
Because the gr-diff-preferences element will be added to the change
view as well, the overlay has been moved to be part of the diff
preferences element. This way, the element can handle all of the actions
taken by the panel, and all the parent element needs to do is call
the open function.
A separate change will come with the addition of diff preferences in the
change view.
Bug: Issue 5426
Change-Id: Id7396147e73354122ea3825bde2c324b5daa1d26
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index 1204170..53d908e 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -21,7 +21,6 @@
<link rel="import" href="../../../bower_components/iron-dropdown/iron-dropdown.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
-<link rel="import" href="../../shared/gr-overlay/gr-overlay.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../shared/gr-select/gr-select.html">
<link rel="import" href="../gr-diff/gr-diff.html">
@@ -276,14 +275,10 @@
</span>
</div>
</div>
- <gr-overlay id="prefsOverlay" with-backdrop>
- <gr-diff-preferences
- id="diffPreferences"
- prefs="{{_prefs}}"
- local-prefs="{{_localPrefs}}"
- on-save="_handlePrefsSave"
- on-cancel="_handlePrefsCancel"></gr-diff-preferences>
- </gr-overlay>
+ <gr-diff-preferences
+ id="diffPreferences"
+ prefs="{{_prefs}}"
+ local-prefs="{{_localPrefs}}"></gr-diff-preferences>
<div class="fileNav mobile">
<a class="mobileNavLink"
href$="[[_computeNavLinkURL(_path, _fileList, -1, 1)]]"><</a>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index fcbbe23..3e73d52 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -363,7 +363,7 @@
this.modifierPressed(e)) { return; }
e.preventDefault();
- this._openPrefs();
+ this.$.diffPreferences.open();
},
_navToChangeView: function() {
@@ -389,15 +389,6 @@
page.show(this._computeNavLinkURL(path, fileList, direction));
},
- _openPrefs: function() {
- this.$.prefsOverlay.open().then(function() {
- var diffPreferences = this.$.diffPreferences;
- var focusStops = diffPreferences.getFocusStops();
- this.$.prefsOverlay.setFocusStops(focusStops);
- this.$.diffPreferences.resetFocus();
- }.bind(this));
- },
-
/**
* @param {?string} path The path of the current file being shown.
* @param {Array.<string>} fileList The list of files in this change and
@@ -599,7 +590,7 @@
_handlePrefsTap: function(e) {
e.preventDefault();
- this._openPrefs();
+ this.$.diffPreferences.open();
},
_handlePrefsSave: function(e) {
@@ -617,15 +608,6 @@
}.bind(this));
},
- _saveDiffPreferences: function() {
- return this.$.restAPI.saveDiffPreferences(this._prefs);
- },
-
- _handlePrefsCancel: function(e) {
- e.stopPropagation();
- this.$.prefsOverlay.close();
- },
-
/**
* _getDiffViewMode: Get the diff view (side-by-side or unified) based on
* the current state.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index 4759086..d54f715 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -111,8 +111,8 @@
'Should navigate to /c/42/');
assert.equal(element.changeViewState.selectedFileIndex, 0);
- var showPrefsStub = sandbox.stub(element.$.prefsOverlay, 'open',
- function() { return Promise.resolve({}); });
+ var showPrefsStub = sandbox.stub(element.$.diffPreferences.$.prefsOverlay,
+ 'open', function() { return Promise.resolve({}); });
MockInteractions.pressAndReleaseKeyOn(element, 188, null, ',');
assert(showPrefsStub.calledOnce);
@@ -145,22 +145,6 @@
false, 'SIDE_BY_SIDE', false));
});
- test('saving diff preferences', function() {
- var savePrefs = sandbox.stub(element, '_handlePrefsSave');
- var cancelPrefs = sandbox.stub(element, '_handlePrefsCancel');
- element.$.diffPreferences._handleSave();
- assert(savePrefs.calledOnce);
- assert(cancelPrefs.notCalled);
- });
-
- test('cancelling diff preferences', function() {
- var savePrefs = sandbox.stub(element, '_handlePrefsSave');
- var cancelPrefs = sandbox.stub(element, '_handlePrefsCancel');
- element.$.diffPreferences._handleCancel();
- assert(cancelPrefs.calledOnce);
- assert(savePrefs.notCalled);
- });
-
test('keyboard shortcuts with patch range', function() {
element._changeNum = '42';
element._patchRange = {
