Focus and tab between diff preferences inputs

Previously, the diff preferences form did not automatically focus to the
first textfield and did not allow tabbing between input fields. This
change adds autofocus when the overlay is opened and allows for tabbing
between the other input fields in the modal.

Bug: Issue 4140
Change-Id: If15812bb4404ca4061597755eeaf68d4cae23b3f
This commit is contained in:
beckysiegel
2016-09-16 10:44:37 -07:00
parent 34ee410b8a
commit c47410c816
6 changed files with 57 additions and 6 deletions

View File

@@ -112,8 +112,8 @@ limitations under the License.
</div> </div>
</div> </div>
<div class="actions"> <div class="actions">
<gr-button primary on-tap="_handleSave">Save</gr-button> <gr-button id="saveButton" primary on-tap="_handleSave">Save</gr-button>
<gr-button on-tap="_handleCancel">Cancel</gr-button> <gr-button id="cancelButton" on-tap="_handleCancel">Cancel</gr-button>
</div> </div>
</template> </template>
<script src="gr-diff-preferences.js"></script> <script src="gr-diff-preferences.js"></script>

View File

@@ -53,6 +53,17 @@
'_localPrefsChanged(localPrefs.*)', '_localPrefsChanged(localPrefs.*)',
], ],
getFocusStops: function() {
return {
start: this.$.contextSelect,
end: this.$.cancelButton,
};
},
resetFocus: function() {
this.$.contextSelect.focus();
},
_prefsChanged: function(changeRecord) { _prefsChanged: function(changeRecord) {
var prefs = changeRecord.base; var prefs = changeRecord.base;
// TODO(andybons): This is not supported in IE. Implement a polyfill. // TODO(andybons): This is not supported in IE. Implement a polyfill.

View File

@@ -62,6 +62,14 @@ limitations under the License.
assert.isFalse(element._newPrefs.syntax_highlighting); assert.isFalse(element._newPrefs.syntax_highlighting);
}); });
test('clicking save button calls _handleSave function', function() {
var savePrefs = sinon.stub(element, '_handleSave');
MockInteractions.tap(element.$.saveButton);
flushAsynchronousOperations();
assert(savePrefs.calledOnce);
savePrefs.restore();
});
test('events', function(done) { test('events', function(done) {
var savePromise = new Promise(function(resolve) { var savePromise = new Promise(function(resolve) {
element.addEventListener('save', function() { resolve(); }); element.addEventListener('save', function() { resolve(); });

View File

@@ -209,6 +209,7 @@ limitations under the License.
</div> </div>
<gr-overlay id="prefsOverlay" with-backdrop> <gr-overlay id="prefsOverlay" with-backdrop>
<gr-diff-preferences <gr-diff-preferences
id="diffPreferences"
prefs="{{_prefs}}" prefs="{{_prefs}}"
local-prefs="{{_localPrefs}}" local-prefs="{{_localPrefs}}"
on-save="_handlePrefsSave" on-save="_handlePrefsSave"

View File

@@ -255,7 +255,7 @@
break; break;
case 188: // ',' case 188: // ','
e.preventDefault(); e.preventDefault();
this.$.prefsOverlay.open(); this._openPrefs();
break; break;
} }
}, },
@@ -267,6 +267,15 @@
page.show(this._computeNavLinkURL(path, fileList, direction)); 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 {?string} path The path of the current file being shown.
* @param {Array.<string>} fileList The list of files in this change and * @param {Array.<string>} fileList The list of files in this change and
@@ -451,7 +460,7 @@
_handlePrefsTap: function(e) { _handlePrefsTap: function(e) {
e.preventDefault(); e.preventDefault();
this.$.prefsOverlay.open(); this._openPrefs();
}, },
_handlePrefsSave: function(e) { _handlePrefsSave: function(e) {

View File

@@ -36,7 +36,7 @@ limitations under the License.
<template> <template>
<div></div> <div></div>
</template> </template>
</text-fixture> </test-fixture>
<script> <script>
suite('gr-diff-view tests', function() { suite('gr-diff-view tests', function() {
@@ -103,7 +103,9 @@ limitations under the License.
'Should navigate to /c/42/'); 'Should navigate to /c/42/');
assert.equal(element.changeViewState.selectedFileIndex, 0); assert.equal(element.changeViewState.selectedFileIndex, 0);
var showPrefsStub = sinon.stub(element.$.prefsOverlay, 'open'); var showPrefsStub = sinon.stub(element.$.prefsOverlay, 'open',
function() { return Promise.resolve({}); });
MockInteractions.pressAndReleaseKeyOn(element, 188); // ',' MockInteractions.pressAndReleaseKeyOn(element, 188); // ','
assert(showPrefsStub.calledOnce); assert(showPrefsStub.calledOnce);
@@ -131,6 +133,26 @@ limitations under the License.
showStub.restore(); showStub.restore();
}); });
test('saving diff preferences', function() {
var savePrefs = sinon.stub(element, '_handlePrefsSave');
var cancelPrefs = sinon.stub(element, '_handlePrefsCancel');
element.$.diffPreferences._handleSave();
assert(savePrefs.calledOnce);
assert(cancelPrefs.notCalled);
savePrefs.restore();
cancelPrefs.restore();
});
test('cancelling diff preferences', function() {
var savePrefs = sinon.stub(element, '_handlePrefsSave');
var cancelPrefs = sinon.stub(element, '_handlePrefsCancel');
element.$.diffPreferences._handleCancel();
assert(cancelPrefs.calledOnce);
assert(savePrefs.notCalled);
savePrefs.restore();
cancelPrefs.restore();
});
test('keyboard shortcuts with patch range', function() { test('keyboard shortcuts with patch range', function() {
element._changeNum = '42'; element._changeNum = '42';
element._patchRange = { element._patchRange = {