Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

fix(menu-surface): Fix absolute positioning for scrollX #3609

Merged
merged 9 commits into from
Sep 24, 2018
Merged

fix(menu-surface): Fix absolute positioning for scrollX #3609

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
27017ce
fix(menu-surface): Fix absolute positioning for scrollX
williamernest Sep 21, 2018
e15dd9f
fix(menu-surface): Update tests for comments
williamernest Sep 21, 2018
3ee84e4
WIP: Update test names
williamernest Sep 21, 2018
29b6d08
WIP: Update for comments
williamernest Sep 21, 2018
9b45140
Merge branch 'master' into fix/menu-surface/absolute-positioning
williamernest Sep 21, 2018
aa5de52
WIP: Add more tests for branch/line coverage
williamernest Sep 21, 2018
467f1b1
Merge branch 'master' into fix/menu-surface/absolute-positioning
williamernest Sep 21, 2018
659c52f
Merge branch 'master' into fix/menu-surface/absolute-positioning
williamernest Sep 24, 2018
8cb9b9e
WIP: Fix lint
williamernest Sep 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions packages/mdc-menu-surface/foundation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ class MDCMenuSurfaceFoundation extends MDCFoundation {
* @private
*/
adjustPositionForHoistedElement_(position) {
const {bodyDimensions, windowScroll, viewport, viewportDistance} = this.measures_;
const {windowScroll, viewportDistance} = this.measures_;

for (const prop in position) {
if (position.hasOwnProperty(prop)) {
Expand All @@ -468,10 +468,16 @@ class MDCMenuSurfaceFoundation extends MDCFoundation {

// Surfaces that are absolutely positioned need to have additional calculations for scroll
// and bottom positioning.
if (!this.isFixedPosition_ && prop === 'top') {
position[prop] = parseInt(position[prop], 10) + windowScroll.y;
} else if (!this.isFixedPosition_ && prop === 'bottom') {
position[prop] = bodyDimensions.height - (viewport.height + windowScroll.y) + parseInt(position[prop], 10);
if (!this.isFixedPosition_) {
if (prop === 'top') {
position[prop] = parseInt(position[prop], 10) + windowScroll.y;
} else if (prop === 'bottom') {
position[prop] = parseInt(position[prop], 10) - windowScroll.y;
} else if (prop === 'left') {
position[prop] = parseInt(position[prop], 10) + windowScroll.x;
} else if (prop === 'right') {
position[prop] = parseInt(position[prop], 10) - windowScroll.x;
}
}
}
}
Expand Down
17 changes: 15 additions & 2 deletions test/unit/mdc-menu-surface/menu-surface.foundation.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,22 @@ testFoundation('#open in absolute position at x/y=100, absolute position, hoiste
td.verify(mockAdapter.setPosition({left: '100px', top: '110px'}));
});

testFoundation('#open in absolute position at x/y=100, fixed position, hoisted menu surface, scrollY 10 px',
testFoundation('#open in absolute position at x/y=100, absolute position, hoisted menu surface, scrollX 10 px',
({foundation, mockAdapter, mockRaf}) => {
initAnchorLayout(mockAdapter, smallTopLeft, true, 200, {x: 0, y: 10});
initAnchorLayout(mockAdapter, smallTopLeft, true, 200, {x: 10, y: 0});
td.when(mockAdapter.hasAnchor()).thenReturn(false);
td.when(mockAdapter.getAnchorDimensions()).thenReturn(undefined);
foundation.setIsHoisted(true);
foundation.setAbsolutePosition(100, 100);
foundation.open();
mockRaf.flush();
td.verify(mockAdapter.setTransformOrigin('left top'));
td.verify(mockAdapter.setPosition({left: '110px', top: '100px'}));
});

testFoundation('#open in absolute position at x/y=100, fixed position, hoisted menu surface, scrollY/scrollY 10 px',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scrollX/scrollY?

Is there any value to having the x/y scroll tests separate for absolute but combined for fixed?

({foundation, mockAdapter, mockRaf}) => {
initAnchorLayout(mockAdapter, smallTopLeft, true, 200, {x: 10, y: 10});
td.when(mockAdapter.hasAnchor()).thenReturn(false);
td.when(mockAdapter.getAnchorDimensions()).thenReturn(undefined);
foundation.setIsHoisted(true);
Expand Down