Fix `Shift A` shortcut for `hide_left_side` diff mode
Hiding the left side of a diff could be triggered by either setting
`hide_left_side` in the render prefs, or by calling `toggleLeftDiff()`
on gr-diff directly. We prefer to let gr-diff users rather update the
preferences than calling special methods.
Also, `toggleLeftDiff()` was only toggling the `no-left` css class, not
updating the preferences, so code relying on the preferences was
actually broken.
Release-Notes: skip
Change-Id: I47d458bf38a967df2402401cf7a6beb6e42000ee
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 1f1ac5c..00f7639 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
@@ -827,7 +827,10 @@
toggleLeftDiff() {
assertIsDefined(this.diffElement);
- this.diffElement.toggleLeftDiff();
+ this.renderPrefs = {
+ ...this.renderPrefs,
+ hide_left_side: !this.renderPrefs.hide_left_side,
+ };
}
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
index 9dabe1d..eea8aaa 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.ts
@@ -560,14 +560,6 @@
assert.equal(stub.lastCall.args.length, 0);
});
- test('delegates toggleLeftDiff()', () => {
- assertIsDefined(element.diffElement);
- const stub = sinon.stub(element.diffElement, 'toggleLeftDiff');
- element.toggleLeftDiff();
- assert.isTrue(stub.calledOnce);
- assert.equal(stub.lastCall.args.length, 0);
- });
-
test('clearBlame', async () => {
element.blame = [];
await element.updateComplete;
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
index 5738cb7..a8a9ff4 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts
@@ -48,7 +48,7 @@
ContentLoadNeededEventDetail,
DiffContextExpandedExternalDetail,
} from '../../../api/diff';
-import {isSafari, toggleClass} from '../../../utils/dom-util';
+import {isSafari} from '../../../utils/dom-util';
import {assertIsDefined} from '../../../utils/common-util';
import {GrDiffSelection} from '../gr-diff-selection/gr-diff-selection';
import {property, query, state} from 'lit/decorators.js';
@@ -508,10 +508,6 @@
return !!this.highlights.selectedRange;
}
- toggleLeftDiff() {
- toggleClass(this, 'no-left');
- }
-
private blameChanged() {
this.setBlame(this.blame ?? []);
if (this.blame) {
@@ -624,18 +620,16 @@
}
private renderPrefsChanged() {
- if (this.renderPrefs.hide_left_side) {
- this.classList.add('no-left');
- }
- if (this.renderPrefs.disable_context_control_buttons) {
- this.classList.add('disable-context-control-buttons');
- }
- if (this.renderPrefs.hide_line_length_indicator) {
- this.classList.add('hide-line-length-indicator');
- }
- if (this.renderPrefs.show_sign_col) {
- this.classList.add('with-sign-col');
- }
+ this.classList.toggle('no-left', !!this.renderPrefs.hide_left_side);
+ this.classList.toggle(
+ 'disable-context-control-buttons',
+ !!this.renderPrefs.disable_context_control_buttons
+ );
+ this.classList.toggle(
+ 'hide-line-length-indicator',
+ !!this.renderPrefs.hide_line_length_indicator
+ );
+ this.classList.toggle('with-sign-col', !!this.renderPrefs.show_sign_col);
if (this.prefs) {
this.updatePreferenceStyles();
}
diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
index 917decc..19c0df8 100644
--- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
+++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff_test.ts
@@ -180,10 +180,13 @@
await element.updateComplete;
});
- test('toggleLeftDiff', () => {
- element.toggleLeftDiff();
+ test('toggleLeftDiff', async () => {
+ element.renderPrefs = {...element.renderPrefs, hide_left_side: true};
+ await element.updateComplete;
assert.isTrue(element.classList.contains('no-left'));
- element.toggleLeftDiff();
+
+ element.renderPrefs = {...element.renderPrefs, hide_left_side: false};
+ await element.updateComplete;
assert.isFalse(element.classList.contains('no-left'));
});
