Add throttle to mark-reviewed shortcut
When reloading the diff page using cmd + r, and releasing cmd first, we process the r:keyup event and wrongly toggle the reviewed state of the file. Add throttle so that the modifier pressed is checked for the first call and the subsequent call within throttle interval is ignored. Change-Id: Ida63ba753bbf793217c7c6094729218b2503b5f3
This commit is contained in:
@@ -280,7 +280,7 @@ class GrDiffView extends KeyboardShortcutMixin(
|
||||
[Shortcut.UP_TO_CHANGE]: '_handleUpToChange',
|
||||
[Shortcut.OPEN_DIFF_PREFS]: '_handleCommaKey',
|
||||
[Shortcut.TOGGLE_DIFF_MODE]: '_handleToggleDiffMode',
|
||||
[Shortcut.TOGGLE_FILE_REVIEWED]: '_handleToggleFileReviewed',
|
||||
[Shortcut.TOGGLE_FILE_REVIEWED]: '_throttledToggleFileReviewed',
|
||||
[Shortcut.EXPAND_ALL_DIFF_CONTEXT]: '_handleExpandAllDiffContext',
|
||||
[Shortcut.NEXT_UNREVIEWED_FILE]: '_handleNextUnreviewedFile',
|
||||
[Shortcut.TOGGLE_BLAME]: '_handleToggleBlame',
|
||||
@@ -306,6 +306,12 @@ class GrDiffView extends KeyboardShortcutMixin(
|
||||
this.flagsService = appContext.flagsService;
|
||||
}
|
||||
|
||||
connectedCallback() {
|
||||
super.connectedCallback();
|
||||
this._throttledToggleFileReviewed = this._throttleWrap(e =>
|
||||
this._handleToggleFileReviewed(e));
|
||||
}
|
||||
|
||||
/** @override */
|
||||
attached() {
|
||||
super.attached();
|
||||
|
||||
@@ -32,6 +32,7 @@ const blankFixture = fixtureFromElement('div');
|
||||
suite('gr-diff-view tests', () => {
|
||||
suite('basic tests', () => {
|
||||
let element;
|
||||
let clock;
|
||||
|
||||
suiteSetup(() => {
|
||||
const kb = TestKeyboardShortcutBinder.push();
|
||||
@@ -82,6 +83,7 @@ suite('gr-diff-view tests', () => {
|
||||
}
|
||||
|
||||
setup(() => {
|
||||
clock = sinon.useFakeTimers();
|
||||
sinon.stub(appContext.flagsService, 'isEnabled').returns(true);
|
||||
stub('gr-rest-api-interface', {
|
||||
getConfig() {
|
||||
@@ -128,6 +130,11 @@ suite('gr-diff-view tests', () => {
|
||||
return element._loadComments();
|
||||
});
|
||||
|
||||
teardown(() => {
|
||||
clock.restore();
|
||||
sinon.restore();
|
||||
});
|
||||
|
||||
test('params change triggers diffViewDisplayed()', () => {
|
||||
sinon.stub(element.reporting, 'diffViewDisplayed');
|
||||
sinon.stub(element.$.diffHost, 'reload').returns(Promise.resolve());
|
||||
@@ -388,6 +395,11 @@ suite('gr-diff-view tests', () => {
|
||||
assert.isFalse(element._setReviewed.called);
|
||||
assert.isTrue(element._handleToggleFileReviewed.calledOnce);
|
||||
|
||||
MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
|
||||
assert.isTrue(element._handleToggleFileReviewed.calledOnce);
|
||||
|
||||
clock.tick(1000);
|
||||
|
||||
MockInteractions.pressAndReleaseKeyOn(element, 82, null, 'r');
|
||||
assert.isTrue(element._handleToggleFileReviewed.calledTwice);
|
||||
assert.isTrue(element._setReviewed.called);
|
||||
|
||||
Reference in New Issue
Block a user