opendev/gerrit
Code Issues Proposed changes

Hide related change list when it is longer than other content

Browse Source 
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
This commit is contained in:
Becky Siegel
2017-03-08 17:29:40 -08:00
parent 8e1ecbef14
commit a7a04f7f93
5 changed files with 262 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
View File

@@ -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. -->
<div id="change_plugins"></div>
</div>
<div class="changeInfo-column mainChangeInfo">
<div id="mainChangeInfo" class="changeInfo-column mainChangeInfo">
<div class="commitActions" hidden$="[[!_loggedIn]]">
<gr-button
class="reply"
@@ -362,16 +374,30 @@ limitations under the License.
id="commitCollapseToggleButton"
class="collapseToggleButton"
on-tap="_toggleCommitCollapsed">
[[_computeCollapseCommitText(_commitCollapsed)]]
[[_computeCollapseText(_commitCollapsed)]]
</gr-button>
</div>
</div>
<div class="relatedChanges">
<gr-related-changes-list id="relatedChanges"
class$="[[_computeRelatedChangesClass(_relatedChangesCollapsed, _relatedChangesLoading)]]"
change="[[_change]]"
has-parent="{{hasParent}}"
loading="{{_relatedChangesLoading}}"
patch-num="[[_computeLatestPatchNum(_allPatchSets)]]">
</gr-related-changes-list>
<div
id="relatedChangesToggle"
class="collapseToggleContainer"
hidden$="[[_computeRelatedChangesToggleHidden(_relatedChangesLoading)]]">
<gr-button
link
id="relatedChangesToggleButton"
class="collapseToggleButton"
on-tap="_toggleRelatedChangesCollapsed">
[[_computeCollapseText(_relatedChangesCollapsed)]]
</gr-button>
</div>
</div>
</div>
</div>

91
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
View File

@@ -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);
},
});
})();

96
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
View File

@@ -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');
});
});
});
</script>

60
polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.html
View File

@@ -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.
}
}
</style>
<div hidden$="[[!_loading]]">Loading...</div>
<div hidden$="[[_loading]]">
<div hidden$="[[!loading]]">Loading...</div>
<div hidden$="[[loading]]">
<hr class="mobile">
<section class="relatedChanges" hidden$="[[!_relatedResponse.changes.length]]" hidden>
<h4>Relation chain</h4>
@@ -94,7 +102,7 @@ limitations under the License.
is="dom-repeat"
items="[[_relatedResponse.changes]]"
as="related">
<div class$="[[_computeChangeContainerClass(change, related)]]">
<div class$="rightIndent [[_computeChangeContainerClass(change, related)]]">
<a href$="[[_computeChangeURL(related._change_number, related._revision_number)]]"
class$="[[_computeLinkClass(related)]]">
[[related.commit.subject]]
@@ -108,37 +116,45 @@ limitations under the License.
<section hidden$="[[!_submittedTogether.length]]" hidden>
<h4>Submitted together</h4>
<template is="dom-repeat" items="[[_submittedTogether]]" as="change">
<a href$="[[_computeChangeURL(change._number)]]"
class$="[[_computeLinkClass(change)]]">
[[change.project]]: [[change.branch]]: [[change.subject]]
</a>
<div>
<a href$="[[_computeChangeURL(change._number)]]"
class$="[[_computeLinkClass(change)]]">
[[change.project]]: [[change.branch]]: [[change.subject]]
</a>
</div>
</template>
</section>
<section hidden$="[[!_sameTopic.length]]" hidden>
<h4>Same topic</h4>
<template is="dom-repeat" items="[[_sameTopic]]" as="change">
<a href$="[[_computeChangeURL(change._number)]]"
class$="[[_computeLinkClass(change)]]">
[[change.project]]: [[change.branch]]: [[change.subject]]
</a>
<div>
<a href$="[[_computeChangeURL(change._number)]]"
class$="[[_computeLinkClass(change)]]">
[[change.project]]: [[change.branch]]: [[change.subject]]
</a>
</div>
</template>
</section>
<section hidden$="[[!_conflicts.length]]" hidden>
<h4>Merge conflicts</h4>
<template is="dom-repeat" items="[[_conflicts]]" as="change">
<a href$="[[_computeChangeURL(change._number)]]"
class$="[[_computeLinkClass(change)]]">
[[change.subject]]
</a>
<div>
<a href$="[[_computeChangeURL(change._number)]]"
class$="[[_computeLinkClass(change)]]">
[[change.subject]]
</a>
</div>
</template>
</section>
<section hidden$="[[!_cherryPicks.length]]" hidden>
<h4>Cherry picks</h4>
<template is="dom-repeat" items="[[_cherryPicks]]" as="change">
<a href$="[[_computeChangeURL(change._number)]]"
class$="[[_computeLinkClass(change)]]">
[[change.branch]]: [[change.subject]]
</a>
<div>
<a href$="[[_computeChangeURL(change._number)]]"
class$="[[_computeLinkClass(change)]]">
[[change.branch]]: [[change.subject]]
</a>
</div>
</template>
</section>
</div>

12
polygerrit-ui/app/elements/change/gr-related-changes-list/gr-related-changes-list.js
View File

@@ -30,8 +30,10 @@
value: false,
reflectToAttribute: true,
},
_loading: Boolean,
loading: {
type: Boolean,
notify: true,
},
_connectedRevisions: {
type: Array,
computed: '_computeConnectedRevisions(change, patchNum, ' +
@@ -54,14 +56,14 @@
],
clear: function() {
this._loading = true;
this.loading = true;
},
reload: function() {
if (!this.change || !this.patchNum) {
return Promise.resolve();
}
this._loading = true;
this.loading = true;
var promises = [
this._getRelatedChanges().then(function(response) {
this._relatedResponse = response;
@@ -97,7 +99,7 @@
}.bind(this)));
return Promise.all(promises).then(function() {
this._loading = false;
this.loading = false;
}.bind(this));
},