From a7a04f7f93721bb7da69ec6174a35b2538ab5937 Mon Sep 17 00:00:00 2001 From: Becky Siegel Date: Wed, 8 Mar 2017 17:29:40 -0800 Subject: [PATCH] Hide related change list when it is longer than other content Related content loads after everything else. Find the height of the existing content, and don't let related changes take up any more space. Conditionally add an expand/collapse button if the content exceeds the allotted space. Bug: Issue 5431 Change-Id: I9ac798d8dd4125e59df51e46f4a18099a87319cb --- .../change/gr-change-view/gr-change-view.html | 38 ++++++-- .../change/gr-change-view/gr-change-view.js | 91 +++++++++++++++++- .../gr-change-view/gr-change-view_test.html | 96 +++++++++++++++++++ .../gr-related-changes-list.html | 60 +++++++----- .../gr-related-changes-list.js | 12 ++- 5 files changed, 262 insertions(+), 35 deletions(-) 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 78754500f3..48ea91ce7b 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 @@ -107,7 +107,7 @@ limitations under the License. .commitMessage { font-family: var(--monospace-font-family); max-width: 100ch; - margin-right: 2em; + margin-right: 1em; margin-bottom: 1em; } .commitMessage gr-linked-text { @@ -139,9 +139,11 @@ limitations under the License. flex: 1; overflow-x: hidden; } + .collapseToggleButton { + text-decoration: none; + } .relatedChanges { flex: 1 1 auto; - font-size: .9em; overflow: hidden; } .patchInfo { @@ -192,17 +194,27 @@ limitations under the License. max-height: 36em; overflow: hidden; } + #relatedChanges { + font-size: .9em; + } + #relatedChanges.collapsed { + margin-bottom: 1.1em; + max-height: var(--relation-chain-max-height, 2em); + overflow: hidden; + } .commitContainer { display: flex; flex-direction: column; } .collapseToggleContainer { display: flex; - justify-content: center; } .collapseToggleContainer gr-button { display: block; - width: 8em; + } + #relatedChangesToggle { + margin-left: 1em; + padding-top: var(--related-change-btn-top-padding, 0); } @media screen and (max-width: 50em) { .mobile { @@ -305,7 +317,7 @@ limitations under the License. This will not work with Shadow DOM. -->
-
+
- [[_computeCollapseCommitText(_commitCollapsed)]] + [[_computeCollapseText(_commitCollapsed)]]
+
+ + [[_computeCollapseText(_relatedChangesCollapsed)]] + +
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 8770f88565..00e39d9f6a 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 @@ -92,6 +92,7 @@ type: String, value: '', }, + _lineHeight: Number, _changeIdCommitMessageError: { type: String, computed: @@ -101,6 +102,10 @@ type: Object, observer: '_updateSelected', }, + _relatedChangesLoading: { + type: Boolean, + value: true, + }, _currentRevisionActions: Object, _allPatchSets: { type: Array, @@ -140,6 +145,10 @@ type: Boolean, value: true, }, + _relatedChangesCollapsed: { + type: Boolean, + value: true, + }, }, behaviors: [ @@ -874,6 +883,8 @@ } else { this._latestCommitMessage = null; } + var lineHeight = getComputedStyle(this).lineHeight; + this._lineHeight = lineHeight.slice(0, lineHeight.length - 2); this._change = change; if (!this._patchRange || !this._patchRange.patchNum || @@ -944,6 +955,7 @@ _reload: function() { this._loading = true; + this._relatedChangesCollapsed = true; this._getLoggedIn().then(function(loggedIn) { if (!loggedIn) { return; } @@ -1044,17 +1056,92 @@ return collapsed ? 'collapsed' : ''; }, - _computeCollapseCommitText: function(collapsed) { - return collapsed ? 'Show more' : 'Show less'; + _computeRelatedChangesClass: function(collapsed, loading) { + if (!loading && !this.customStyle['--relation-chain-max-height']) { + this._updateRelatedChangeMaxHeight(); + } + return collapsed ? 'collapsed' : ''; + }, + + _computeCollapseText: function(collapsed) { + // Symbols are up and down triangles. + return collapsed ? '\u25bc Show more' : '\u25b2 Show less'; }, _toggleCommitCollapsed: function() { this._commitCollapsed = !this._commitCollapsed; + if (this._commitCollapsed) { + window.scrollTo(0, 0); + } + }, + + _toggleRelatedChangesCollapsed: function() { + this._relatedChangesCollapsed = !this._relatedChangesCollapsed; + if (this._relatedChangesCollapsed) { + window.scrollTo(0, 0); + } }, _computeCommitToggleHidden: function(commitMessage) { if (!commitMessage) { return true; } return commitMessage.split('\n').length < MIN_LINES_FOR_COMMIT_COLLAPSE; }, + + _getOffsetHeight: function(element) { + return element.offsetHeight; + }, + + _getScrollHeight: function(element) { + return element.scrollHeight; + }, + + /** + * @desc get the line height of an element to the nearest integer. + * */ + _getLineHeight: function(element) { + var lineHeightStr = getComputedStyle(element).lineHeight; + return Math.round(lineHeightStr.slice(0, lineHeightStr.length - 2)); + }, + + /** + * @desc new max height for the related changes section, shorter than + * the existing change info height. + */ + _updateRelatedChangeMaxHeight: function() { + // Takes into account approximate height for the expand button and + // bottom margin + var extraHeight = 24; + var maxExistingHeight; + var hasCommitToggle = + !this._computeCommitToggleHidden(this._latestCommitMessage); + if (hasCommitToggle) { + // Make sure the content is lined up if both areas have buttons. If the + // commit message is not collapsed, instead use the change info hight. + maxExistingHeight = this._getOffsetHeight(this.$.commitMessage); + } else { + maxExistingHeight = this._getOffsetHeight(this.$.mainChangeInfo) - + extraHeight; + } + + // Get the line height of related changes, and convert it to the nearest + // integer. + var lineHeight = this._getLineHeight(this.$.relatedChanges); + + // Figure out a new height that is divisible by the rounded line height. + var remainder = maxExistingHeight % lineHeight; + var newHeight = maxExistingHeight - remainder; + + // Update the max-height of the relation chain to this new height; + this.customStyle['--relation-chain-max-height'] = newHeight + 'px'; + if (hasCommitToggle) { + this.customStyle['--related-change-btn-top-padding'] = remainder + 'px'; + } + this.updateStyles(); + }, + + _computeRelatedChangesToggleHidden: function() { + return this._getScrollHeight(this.$.relatedChanges) <= + this._getOffsetHeight(this.$.relatedChanges); + }, }); })(); 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 cf714dfef1..884142c6ad 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 @@ -988,5 +988,101 @@ limitations under the License. element.$.commitMessage.classList.contains('collapsed')); }); }); + + suite('related changes expand/collapse', function() { + var updateHeightSpy; + setup(function() { + updateHeightSpy = sandbox.spy(element, '_updateRelatedChangeMaxHeight'); + }); + + test('relatedChangesToggle shown height greater than changeInfo height', + function() { + assert.isTrue(element.$.relatedChangesToggle.hasAttribute('hidden')); + + sandbox.stub(element, '_getOffsetHeight', function() { + return 50; + }); + + sandbox.stub(element, '_getScrollHeight', function() { + return 60; + }); + element._relatedChangesLoading = false; + assert.isFalse(element.$.relatedChangesToggle.hasAttribute('hidden')); + assert.equal(updateHeightSpy.callCount, 1); + }); + + test('relatedChangesToggle hidden height less than changeInfo height', + function() { + assert.isTrue(element.$.relatedChangesToggle.hasAttribute('hidden')); + sandbox.stub(element, '_getOffsetHeight', function() { + return 50; + }); + + sandbox.stub(element, '_getScrollHeight', function() { + return 40; + }); + element._relatedChangesLoading = false; + assert.isTrue(element.$.relatedChangesToggle.hasAttribute('hidden')); + assert.equal(updateHeightSpy.callCount, 1); + }); + + test('relatedChangesToggle functions', function() { + sandbox.stub(element, '_getOffsetHeight', function() { + return 50; + }); + + sandbox.stub(element, '_getScrollHeight', function() { + return 60; + }); + element._relatedChangesLoading = false; + assert.isTrue(element._relatedChangesCollapsed); + assert.isTrue( + element.$.relatedChanges.classList.contains('collapsed')); + MockInteractions.tap(element.$.relatedChangesToggleButton); + assert.isFalse(element._relatedChangesCollapsed); + assert.isFalse( + element.$.relatedChanges.classList.contains('collapsed')); + }); + + test('_updateRelatedChangeMaxHeight without commit toggle', function() { + sandbox.stub(element, '_getOffsetHeight', function() { + return 50; + }); + + sandbox.stub(element, '_getLineHeight', function() { + return 12; + }); + + // 50 (existing height) - 24 (extra height) = 26 (adjusted height). + // 50 (existing height) % 12 (line height) = 2 (remainder). + // 26 (adjusted height) - 2 (remainder) = 24 (max height to set). + + element._updateRelatedChangeMaxHeight(); + assert.equal(element.customStyle['--relation-chain-max-height'], + '24px'); + assert.equal(element.customStyle['--related-change-btn-top-padding'], + undefined); + }); + + test('_updateRelatedChangeMaxHeight with commit toggle', function() { + element._latestCommitMessage = _.times(31, String).join('\n'); + sandbox.stub(element, '_getOffsetHeight', function() { + return 50; + }); + + sandbox.stub(element, '_getLineHeight', function() { + return 12; + }); + + // 50 (existing height) % 12 (line height) = 2 (remainder). + // 50 (existing height) - 2 (remainder) = 48 (max height to set). + + element._updateRelatedChangeMaxHeight(); + assert.equal(element.customStyle['--relation-chain-max-height'], + '48px'); + assert.equal(element.customStyle['--related-change-btn-top-padding'], + '2px'); + }); + }); }); diff --git a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html index 69c8cc0f0f..20f9f7701a 100644 --- a/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html +++ b/polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html @@ -28,7 +28,7 @@ limitations under the License. margin: .5em 0 0; } section { - margin-bottom: 1em; + margin-bottom: 1.4em; /* Same as line height for collapse purposes */ } a { display: block; @@ -45,8 +45,16 @@ limitations under the License. } .changeContainer.thisChange:before { content: '➔'; - position: absolute; - transform: translateX(-1.2em); + width: 1.2em + } + h4, + section div { + display: flex + } + h4:before, + section div:before { + content: ' '; + width: 1.2em } .relatedChanges a { display: inline-block; @@ -85,8 +93,8 @@ limitations under the License. } } -
Loading...
-
+
Loading...
+