Merge "Change expand diff context shortcut to be a toggle"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 7b80977..c33eb26 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -576,7 +576,6 @@
[Shortcut.UP_TO_DASHBOARD]: '_handleUpToDashboard',
[Shortcut.EXPAND_ALL_MESSAGES]: '_handleExpandAllMessages',
[Shortcut.COLLAPSE_ALL_MESSAGES]: '_handleCollapseAllMessages',
- [Shortcut.EXPAND_ALL_DIFF_CONTEXT]: '_expandAllDiffs',
[Shortcut.OPEN_DIFF_PREFS]: '_handleOpenDiffPrefsShortcut',
[Shortcut.EDIT_TOPIC]: '_handleEditTopic',
[Shortcut.DIFF_AGAINST_BASE]: '_handleDiffAgainstBase',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
index ae1e438..ff60531 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.ts
@@ -564,8 +564,8 @@
this.$.diff.clearDiffContent();
}
- expandAllContext() {
- this.$.diff.expandAllContext();
+ toggleAllContext() {
+ this.$.diff.toggleAllContext();
}
_getLoggedIn() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index 99486ad..01c78f0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -796,9 +796,9 @@
assert.equal(stub.lastCall.args.length, 0);
});
- test('delegates expandAllContext()', () => {
- const stub = sinon.stub(element.$.diff, 'expandAllContext');
- element.expandAllContext();
+ test('delegates toggleAllContext()', () => {
+ const stub = sinon.stub(element.$.diff, 'toggleAllContext');
+ element.toggleAllContext();
assert.isTrue(stub.calledOnce);
assert.equal(stub.lastCall.args.length, 0);
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 1550108..b31333c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -291,7 +291,7 @@
[Shortcut.OPEN_DIFF_PREFS]: '_handleCommaKey',
[Shortcut.TOGGLE_DIFF_MODE]: '_handleToggleDiffMode',
[Shortcut.TOGGLE_FILE_REVIEWED]: '_throttledToggleFileReviewed',
- [Shortcut.EXPAND_ALL_DIFF_CONTEXT]: '_handleExpandAllDiffContext',
+ [Shortcut.TOGGLE_ALL_DIFF_CONTEXT]: '_handleToggleAllDiffContext',
[Shortcut.NEXT_UNREVIEWED_FILE]: '_handleNextUnreviewedFile',
[Shortcut.TOGGLE_BLAME]: '_handleToggleBlame',
[Shortcut.TOGGLE_HIDE_ALL_COMMENT_THREADS]:
@@ -1744,10 +1744,10 @@
return '';
}
- _handleExpandAllDiffContext(e: CustomKeyboardEvent) {
+ _handleToggleAllDiffContext(e: CustomKeyboardEvent) {
if (this.shouldSuppressKeyboardShortcut(e)) return;
- this.$.diffHost.expandAllContext();
+ this.$.diffHost.toggleAllContext();
}
_computeDiffPrefsDisabled(disableDiffPrefs?: boolean, loggedIn?: boolean) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 46a670d..b5473a3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -63,7 +63,7 @@
kb.bindShortcut(Shortcut.OPEN_DIFF_PREFS, ',');
kb.bindShortcut(Shortcut.TOGGLE_DIFF_MODE, 'm');
kb.bindShortcut(Shortcut.TOGGLE_FILE_REVIEWED, 'r');
- kb.bindShortcut(Shortcut.EXPAND_ALL_DIFF_CONTEXT, 'shift+x');
+ kb.bindShortcut(Shortcut.TOGGLE_ALL_DIFF_CONTEXT, 'shift+x');
kb.bindShortcut(Shortcut.EXPAND_ALL_COMMENT_THREADS, 'e');
kb.bindShortcut(Shortcut.TOGGLE_HIDE_ALL_COMMENT_THREADS, 'h');
kb.bindShortcut(Shortcut.COLLAPSE_ALL_COMMENT_THREADS, 'shift+e');
@@ -508,11 +508,11 @@
assert.isTrue(diffChangeStub.called);
});
- test('shift+x shortcut expands all diff context', () => {
- const expandStub = sinon.stub(element.$.diffHost, 'expandAllContext');
+ test('shift+x shortcut toggles all diff context', () => {
+ const toggleStub = sinon.stub(element.$.diffHost, 'toggleAllContext');
MockInteractions.pressAndReleaseKeyOn(element, 88, 'shift', 'x');
flush();
- assert.isTrue(expandStub.called);
+ assert.isTrue(toggleStub.called);
});
test('diff against base', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 3fb1a50..189bac7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -55,7 +55,11 @@
PolymerDomWrapper,
} from '../../../types/types';
import {CommentRangeLayer} from '../gr-ranged-comment-layer/gr-ranged-comment-layer';
-import {DiffViewMode, Side} from '../../../constants/constants';
+import {
+ createDefaultDiffPrefs,
+ DiffViewMode,
+ Side,
+} from '../../../constants/constants';
import {KeyLocations} from '../gr-diff-processor/gr-diff-processor';
import {FlattenedNodesObserver} from '@polymer/polymer/lib/utils/flattened-nodes-observer';
import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
@@ -77,7 +81,6 @@
const LARGE_DIFF_THRESHOLD_LINES = 10000;
const FULL_CONTEXT = -1;
-const LIMITED_CONTEXT = 10;
const COMMIT_MSG_PATH = '/COMMIT_MSG';
/**
@@ -968,8 +971,12 @@
this._debounceRenderDiffTable();
}
- _handleLimitedBypass() {
- this._safetyBypass = LIMITED_CONTEXT;
+ _collapseContext() {
+ // Uses the default context amount if the preference is for the entire file.
+ this._safetyBypass =
+ this.prefs?.context && this.prefs.context >= 0
+ ? null
+ : createDefaultDiffPrefs().context;
this._debounceRenderDiffTable();
}
@@ -981,8 +988,15 @@
return errorMessage ? 'showError' : '';
}
- expandAllContext() {
- this._handleFullBypass();
+ toggleAllContext() {
+ if (!this.prefs) {
+ return;
+ }
+ if (this._getBypassPrefs(this.prefs).context < 0) {
+ this._collapseContext();
+ } else {
+ this._handleFullBypass();
+ }
}
_computeNewlineWarning(warnLeft: boolean, warnRight: boolean) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
index 4b477cd..4d0e566 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_html.ts
@@ -623,7 +623,7 @@
Prevented render because "Whole file" is enabled and this diff is very
large (about [[_diffLength]] lines).
</p>
- <gr-button on-click="_handleLimitedBypass">
+ <gr-button on-click="_collapseContext">
Render with limited context
</gr-button>
<gr-button on-click="_handleFullBypass">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
index 2215d5b..6fef223 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
@@ -794,6 +794,43 @@
element.addEventListener('render', rendered);
element._renderDiffTable();
});
+
+ test('toggles expand context using bypass', async () => {
+ element.prefs = {...MINIMAL_PREFS, context: 3};
+
+ element.toggleAllContext();
+ element._renderDiffTable();
+ await flush();
+
+ assert.equal(element.prefs.context, 3);
+ assert.equal(element._safetyBypass, -1);
+ assert.equal(renderStub.firstCall.lastArg.context, -1);
+ });
+
+ test('toggles collapse context from bypass', async () => {
+ element.prefs = {...MINIMAL_PREFS, context: 3};
+ element._safetyBypass = -1;
+
+ element.toggleAllContext();
+ element._renderDiffTable();
+ await flush();
+
+ assert.equal(element.prefs.context, 3);
+ assert.isNull(element._safetyBypass);
+ assert.equal(renderStub.firstCall.lastArg.context, 3);
+ });
+
+ test('toggles collapse context from pref using default', async () => {
+ element.prefs = {...MINIMAL_PREFS, context: -1};
+
+ element.toggleAllContext();
+ element._renderDiffTable();
+ await flush();
+
+ assert.equal(element.prefs.context, -1);
+ assert.equal(element._safetyBypass, 10);
+ assert.equal(renderStub.firstCall.lastArg.context, 10);
+ });
});
suite('blame', () => {
diff --git a/polygerrit-ui/app/elements/gr-app-element.ts b/polygerrit-ui/app/elements/gr-app-element.ts
index cdc9f98..e4d72e1 100644
--- a/polygerrit-ui/app/elements/gr-app-element.ts
+++ b/polygerrit-ui/app/elements/gr-app-element.ts
@@ -392,7 +392,7 @@
}
this.bindShortcut(Shortcut.NEXT_CHUNK, 'n');
this.bindShortcut(Shortcut.PREV_CHUNK, 'p');
- this.bindShortcut(Shortcut.EXPAND_ALL_DIFF_CONTEXT, 'shift+x');
+ this.bindShortcut(Shortcut.TOGGLE_ALL_DIFF_CONTEXT, 'shift+x');
this.bindShortcut(Shortcut.NEXT_COMMENT_THREAD, 'shift+n');
this.bindShortcut(Shortcut.PREV_COMMENT_THREAD, 'shift+p');
this.bindShortcut(
diff --git a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
index 9bd2f5a..7050cbe 100644
--- a/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
+++ b/polygerrit-ui/app/mixins/keyboard-shortcut-mixin/keyboard-shortcut-mixin.ts
@@ -176,7 +176,7 @@
VISIBLE_LINE = 'VISIBLE_LINE',
NEXT_CHUNK = 'NEXT_CHUNK',
PREV_CHUNK = 'PREV_CHUNK',
- EXPAND_ALL_DIFF_CONTEXT = 'EXPAND_ALL_DIFF_CONTEXT',
+ TOGGLE_ALL_DIFF_CONTEXT = 'TOGGLE_ALL_DIFF_CONTEXT',
NEXT_COMMENT_THREAD = 'NEXT_COMMENT_THREAD',
PREV_COMMENT_THREAD = 'PREV_COMMENT_THREAD',
EXPAND_ALL_COMMENT_THREADS = 'EXPAND_ALL_COMMENT_THREADS',
@@ -404,9 +404,9 @@
'Go to previous diff chunk'
);
_describe(
- Shortcut.EXPAND_ALL_DIFF_CONTEXT,
+ Shortcut.TOGGLE_ALL_DIFF_CONTEXT,
ShortcutSection.DIFFS,
- 'Expand all diff context'
+ 'Toggle all diff context'
);
_describe(
Shortcut.NEXT_COMMENT_THREAD,
