Change expand diff context shortcut to be a toggle Also removes this shortcut from the change view because it was duplicating only the expand part of the toggleAllInlineDiffs functionality instead of expanding diff context. If the user's context preference is already the whole file, it will collapse to the default 10 line context when toggled. Change-Id: I911d5fb57edd427f00543a6eb59e8a684f8a8caf
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 ec44f92..1bdf498 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
@@ -797,6 +797,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', () => {