opendev/gerrit
Code Issues Proposed changes

Update gr-fixed-panel

Browse Source 
Previously, when keepOnScroll was set to true, and the header was not
yet to the top, there was a miscalculation in the 'top' number, casuing
the header to scroll up higher than its gr-fixed-panel parent.

This occurred because calculations for '_topLast' became inaccurate, and
compounded on one another.

This change fixes the problem by utilizing the position of the
gr-fixed-panel element itself to get a top offset, since it is reatively
positioned, this can be computed.

gr-fixed-panel tests were not being run, and they were already failing.
The tests have been updated to relfect how they should behave.

Change-Id: If0156cc7b009498ee63dddf0cef1b7e8d786b17c
This commit is contained in:
Becky Siegel 2018-04-06 21:22:23 -07:00
parent a919cb69c2
commit 6c16e97d52
3 changed files with 24 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel.js
View File

@ -93,10 +93,6 @@
].join(' ');
},
_getScrollY() {
return window.scrollY;
},
unfloat() {
if (this.floatingDisabled) {
return;
@ -127,26 +123,29 @@
this._reposition();
},
_getElementTop() {
return this.getBoundingClientRect().top;
},
_reposition() {
if (!this._headerFloating) {
return;
}
const header = this.$.header;
const scrollY = this._topInitial - this._getScrollY();
// Since the outer element is relative positioned, can use its top
// to determine how to position the inner header element.
const elemTop = this._getElementTop();
let newTop;
if (this.keepOnScroll) {
if (scrollY > 0) {
// Reposition to imitate natural scrolling.
newTop = scrollY;
} else {
newTop = 0;
}
} else if (scrollY > -this._headerHeight ||
this._topLast < -this._headerHeight) {
// Allow to scroll away, but ignore when far behind the edge.
newTop = scrollY;
if (this.keepOnScroll && elemTop < 0) {
// Should stick to the top.
newTop = 0;
} else {
newTop = -this._headerHeight;
// Keep in line with the outer element.
newTop = elemTop;
}
// Initialize top style if it doesn't exist yet.
if (!header.style.top && this._topLast === newTop) {
header.style.top = newTop;
}
if (this._topLast !== newTop) {
if (newTop === undefined) {

11
polygerrit-ui/app/elements/shared/gr-fixed-panel/gr-fixed-panel_test.html
View File

@ -74,24 +74,27 @@ limitations under the License.
};
const emulateScrollY = function(distance) {
element._getScrollY.returns(distance);
element._getElementTop.returns(element._headerTopInitial - distance);
element._updateDebounced();
element.flushDebouncer('scroll');
};
setup(() => {
element._headerTopInitial = 10;
sandbox.stub(element, '_getScrollY').returns(0);
sandbox.stub(element, '_getElementTop')
.returns(element._headerTopInitial);
});
test('scrolls header along with document', () => {
emulateScrollY(20);
assert.equal(getHeaderTop(), '-12px');
// No top property is set when !_headerFloating.
assert.equal(getHeaderTop(), '');
});
test('does not stick to the top by default', () => {
emulateScrollY(150);
assert.equal(getHeaderTop(), '-100px');
// No top property is set when !_headerFloating.
assert.equal(getHeaderTop(), '');
});
test('sticks to the top if enabled', () => {

1
polygerrit-ui/app/test/index.html
View File

@ -162,6 +162,7 @@ limitations under the License.
'shared/gr-js-api-interface/gr-js-api-interface_test.html',
'shared/gr-js-api-interface/gr-plugin-endpoints_test.html',
'shared/gr-js-api-interface/gr-plugin-rest-api_test.html',
'shared/gr-fixed-panel/gr-fixed-panel_test.html',
'shared/gr-limited-text/gr-limited-text_test.html',
'shared/gr-linked-chip/gr-linked-chip_test.html',
'shared/gr-linked-text/gr-linked-text_test.html',