Fix related change show more calculation
Previously, there was a case where the scroll height of related changes
was greater than the offsetHeight, but only because of the bottom
margin, so no text was hidden.
This change takes that into account when computing whether or not to
display a show more button. It also switches from using hidden$ to a
computed class.
Bug: Issue 6566
Change-Id: I05a40c7637316ce1a27a1f9c4804844c17678ba4
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 97c1dee..7a61d56 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -227,6 +227,12 @@
.collapseToggleContainer {
display: flex;
}
+ #relatedChangesToggle {
+ display: none;
+ }
+ #relatedChangesToggle.showToggle {
+ display: flex;
+ }
.collapseToggleContainer gr-button {
display: block;
}
@@ -425,8 +431,7 @@
</gr-related-changes-list>
<div
id="relatedChangesToggle"
- class="collapseToggleContainer"
- hidden$="[[_computeRelatedChangesToggleHidden(_relatedChangesLoading)]]">
+ class$="collapseToggleContainer [[_computeRelatedChangesToggleClass(_relatedChangesLoading)]]">
<gr-button
link
id="relatedChangesToggleButton"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index b1501b6..27a5e86 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -1260,9 +1260,14 @@
this.updateStyles();
},
- _computeRelatedChangesToggleHidden() {
- return this._getScrollHeight(this.$.relatedChanges) <=
- this._getOffsetHeight(this.$.relatedChanges);
+ _computeRelatedChangesToggleClass() {
+ // Prevents showMore from showing when click on related change, since the
+ // line height would be positive, but related changes height is 0.
+ if (!this._getScrollHeight(this.$.relatedChanges)) { return ''; }
+
+ return this._getScrollHeight(this.$.relatedChanges) >
+ (this._getOffsetHeight(this.$.relatedChanges) +
+ this._getLineHeight(this.$.relatedChanges)) ? 'showToggle' : '';
},
_startUpdateCheckTimer() {
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 75adcdc..797b6b3 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -1172,23 +1172,29 @@
test('relatedChangesToggle shown height greater than changeInfo height',
() => {
- assert.isTrue(element.$.relatedChangesToggle.hasAttribute('hidden'));
+ assert.isFalse(element.$.relatedChangesToggle.classList
+ .contains('showToggle'));
sandbox.stub(element, '_getOffsetHeight', () => 50);
sandbox.stub(element, '_getScrollHeight', () => 60);
+ sandbox.stub(element, '_getLineHeight', () => 5);
sandbox.stub(window, 'matchMedia', () => ({matches: true}));
element._relatedChangesLoading = false;
- assert.isFalse(element.$.relatedChangesToggle.hasAttribute('hidden'));
+ assert.isTrue(element.$.relatedChangesToggle.classList
+ .contains('showToggle'));
assert.equal(updateHeightSpy.callCount, 1);
});
test('relatedChangesToggle hidden height less than changeInfo height',
() => {
- assert.isTrue(element.$.relatedChangesToggle.hasAttribute('hidden'));
+ assert.isFalse(element.$.relatedChangesToggle.classList
+ .contains('showToggle'));
sandbox.stub(element, '_getOffsetHeight', () => 50);
sandbox.stub(element, '_getScrollHeight', () => 40);
+ sandbox.stub(element, '_getLineHeight', () => 5);
sandbox.stub(window, 'matchMedia', () => ({matches: true}));
element._relatedChangesLoading = false;
- assert.isTrue(element.$.relatedChangesToggle.hasAttribute('hidden'));
+ assert.isFalse(element.$.relatedChangesToggle.classList
+ .contains('showToggle'));
assert.equal(updateHeightSpy.callCount, 1);
});
