Skip to content

Commit

Permalink
[Select][material] Fix select menu moving on scroll when disableScrol…
Browse files Browse the repository at this point in the history 
…lLock is true (#37773)
  • Loading branch information
@VishruthR
VishruthR authored Aug 24, 2023
1 parent 9b49328 commit c54ad8f
Show file tree
Hide file tree
Showing 5 changed files with 129 additions and 71 deletions.
1 change: 1 addition & 0 deletions docs/pages/material-ui/api/popover.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"children": { "type": { "name": "node" } },
"classes": { "type": { "name": "object" }, "additionalInfo": { "cssApi": true } },
"container": { "type": { "name": "union", "description": "HTML element<br>&#124;&nbsp;func" } },
"disableScrollLock": { "type": { "name": "bool" }, "default": "false" },
"elevation": { "type": { "name": "custom", "description": "integer" }, "default": "8" },
"marginThreshold": { "type": { "name": "number" }, "default": "16" },
"onClose": { "type": { "name": "func" } },
Expand Down
3 changes: 2 additions & 1 deletion docs/translations/api-docs/popover/popover.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@
"container": {
"description": "An HTML element, component instance, or function that returns either. The <code>container</code> will passed to the Modal component.<br>By default, it uses the body of the anchorEl&#39;s top-level document object, so it&#39;s simply <code>document.body</code> most of the time."
},
"disableScrollLock": { "description": "Disable the scroll lock behavior." },
"elevation": { "description": "The elevation of the popover." },
"marginThreshold": {
"description": "Specifies how close to the edge of the window the popover can appear."
"description": "Specifies how close to the edge of the window the popover can appear. If null, the popover will not be constrained by the window."
},
"onClose": {
"description": "Callback fired when the component requests to be closed. The <code>reason</code> parameter can optionally be used to control the response to <code>onClose</code>."
Expand Down
3 changes: 2 additions & 1 deletion packages/mui-material/src/Popover/Popover.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,10 @@ export interface PopoverProps
elevation?: number;
/**
* Specifies how close to the edge of the window the popover can appear.
* If null, the popover will not be constrained by the window.
* @default 16
*/
marginThreshold?: number;
marginThreshold?: number | null;
onClose?: ModalProps['onClose'];
/**
* If `true`, the component is shown.
Expand Down
29 changes: 25 additions & 4 deletions packages/mui-material/src/Popover/Popover.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ const Popover = React.forwardRef(function Popover(inProps, ref) {
TransitionComponent = Grow,
transitionDuration: transitionDurationProp = 'auto',
TransitionProps: { onEntering, ...TransitionProps } = {},
disableScrollLock = false,
...other
} = props;

Expand Down Expand Up @@ -244,13 +245,17 @@ const Popover = React.forwardRef(function Popover(inProps, ref) {
const widthThreshold = containerWindow.innerWidth - marginThreshold;

// Check if the vertical axis needs shifting
if (top < marginThreshold) {
if (marginThreshold !== null && top < marginThreshold) {
const diff = top - marginThreshold;

top -= diff;

elemTransformOrigin.vertical += diff;
} else if (bottom > heightThreshold) {
} else if (marginThreshold !== null && bottom > heightThreshold) {
const diff = bottom - heightThreshold;

top -= diff;

elemTransformOrigin.vertical += diff;
}

Expand All @@ -269,7 +274,7 @@ const Popover = React.forwardRef(function Popover(inProps, ref) {
}

// Check if the horizontal axis needs shifting
if (left < marginThreshold) {
if (marginThreshold !== null && left < marginThreshold) {
const diff = left - marginThreshold;
left -= diff;
elemTransformOrigin.horizontal += diff;
Expand Down Expand Up @@ -309,6 +314,13 @@ const Popover = React.forwardRef(function Popover(inProps, ref) {
setIsPositioned(true);
}, [getPositioningStyle]);

React.useEffect(() => {
if (disableScrollLock) {
window.addEventListener('scroll', setPositioningStyles);
}
return () => window.removeEventListener('scroll', setPositioningStyles);
}, [anchorEl, disableScrollLock, setPositioningStyles]);

const handleEntering = (element, isAppearing) => {
if (onEntering) {
onEntering(element, isAppearing);
Expand Down Expand Up @@ -403,7 +415,10 @@ const Popover = React.forwardRef(function Popover(inProps, ref) {
});

return (
<RootSlot {...rootProps} {...(!isHostComponent(RootSlot) && { slotProps: rootSlotPropsProp })}>
<RootSlot
{...rootProps}
{...(!isHostComponent(RootSlot) && { slotProps: rootSlotPropsProp, disableScrollLock })}
>
<TransitionComponent
appear
in={open}
Expand Down Expand Up @@ -525,13 +540,19 @@ Popover.propTypes /* remove-proptypes */ = {
HTMLElementType,
PropTypes.func,
]),
/**
* Disable the scroll lock behavior.
* @default false
*/
disableScrollLock: PropTypes.bool,
/**
* The elevation of the popover.
* @default 8
*/
elevation: integerPropType,
/**
* Specifies how close to the edge of the window the popover can appear.
* If null, the popover will not be constrained by the window.
* @default 16
*/
marginThreshold: PropTypes.number,
Expand Down
164 changes: 99 additions & 65 deletions packages/mui-material/src/Popover/Popover.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -844,8 +844,8 @@ describe('<Popover />', () => {
});
});

[0, 18, 16].forEach((marginThreshold) => {
describe(`positioning when \`marginThreshold=${marginThreshold}\``, () => {
describe('prop: marginThreshold', () => {
[0, 18, 16].forEach((marginThreshold) => {
function getElementStyleOfOpenPopover(anchorEl = document.createElement('svg')) {
let style;
render(
Expand All @@ -858,106 +858,140 @@ describe('<Popover />', () => {
},
}}
marginThreshold={marginThreshold}
PaperProps={{ component: FakePaper }}
slotProps={{ paper: { component: FakePaper } }}
>
<div />
</Popover>,
);
return style;
}

specify('when no movement is needed', () => {
const negative = marginThreshold === 0 ? '' : '-';
const positioningStyle = getElementStyleOfOpenPopover();
describe(`positioning when \`marginThreshold=${marginThreshold}\``, () => {
specify('when no movement is needed', () => {
const negative = marginThreshold === 0 ? '' : '-';
const positioningStyle = getElementStyleOfOpenPopover();

expect(positioningStyle.top).to.equal(`${marginThreshold}px`);
expect(positioningStyle.left).to.equal(`${marginThreshold}px`);
expect(positioningStyle.transformOrigin).to.match(
new RegExp(`${negative}${marginThreshold}px ${negative}${marginThreshold}px( 0px)?`),
);
});

specify('top < marginThreshold', () => {
const mockedAnchor = document.createElement('div');
stub(mockedAnchor, 'getBoundingClientRect').callsFake(() => ({
left: marginThreshold,
top: marginThreshold - 1,
}));
const positioningStyle = getElementStyleOfOpenPopover(mockedAnchor);

expect(positioningStyle.top).to.equal(`${marginThreshold}px`);
expect(positioningStyle.left).to.equal(`${marginThreshold}px`);
expect(positioningStyle.transformOrigin).to.match(/0px -1px( 0ms)?/);
});

describe('bottom > heightThreshold', () => {
let windowInnerHeight;

before(() => {
windowInnerHeight = window.innerHeight;
window.innerHeight = marginThreshold * 2;
});

after(() => {
window.innerHeight = windowInnerHeight;
expect(positioningStyle.top).to.equal(`${marginThreshold}px`);
expect(positioningStyle.left).to.equal(`${marginThreshold}px`);
expect(positioningStyle.transformOrigin).to.match(
new RegExp(`${negative}${marginThreshold}px ${negative}${marginThreshold}px( 0px)?`),
);
});

specify('test', () => {
specify('top < marginThreshold', () => {
const mockedAnchor = document.createElement('div');
stub(mockedAnchor, 'getBoundingClientRect').callsFake(() => ({
left: marginThreshold,
top: marginThreshold + 1,
top: marginThreshold - 1,
}));

const positioningStyle = getElementStyleOfOpenPopover(mockedAnchor);

expect(positioningStyle.top).to.equal(`${marginThreshold}px`);
expect(positioningStyle.left).to.equal(`${marginThreshold}px`);
expect(positioningStyle.transformOrigin).to.match(/0px 1px( 0px)?/);
expect(positioningStyle.transformOrigin).to.match(/0px -1px( 0px)?/);
});
});

specify('left < marginThreshold', () => {
const mockedAnchor = document.createElement('div');
stub(mockedAnchor, 'getBoundingClientRect').callsFake(() => ({
left: marginThreshold - 1,
top: marginThreshold,
}));

const positioningStyle = getElementStyleOfOpenPopover(mockedAnchor);
describe('bottom > heightThreshold', () => {
let windowInnerHeight;

expect(positioningStyle.top).to.equal(`${marginThreshold}px`);
before(() => {
windowInnerHeight = window.innerHeight;
window.innerHeight = marginThreshold * 2;
});

expect(positioningStyle.left).to.equal(`${marginThreshold}px`);
after(() => {
window.innerHeight = windowInnerHeight;
});

expect(positioningStyle.transformOrigin).to.match(/-1px 0px( 0px)?/);
});
specify('test', () => {
const mockedAnchor = document.createElement('div');
stub(mockedAnchor, 'getBoundingClientRect').callsFake(() => ({
left: marginThreshold,
top: marginThreshold + 1,
}));

describe('right > widthThreshold', () => {
let innerWidthContainer;
const positioningStyle = getElementStyleOfOpenPopover(mockedAnchor);

before(() => {
innerWidthContainer = window.innerWidth;
window.innerWidth = marginThreshold * 2;
expect(positioningStyle.top).to.equal(`${marginThreshold}px`);
expect(positioningStyle.left).to.equal(`${marginThreshold}px`);
expect(positioningStyle.transformOrigin).to.match(/0px 1px( 0px)?/);
});
});

after(() => {
window.innerWidth = innerWidthContainer;
});

specify('test', () => {
specify('left < marginThreshold', () => {
const mockedAnchor = document.createElement('div');
stub(mockedAnchor, 'getBoundingClientRect').callsFake(() => ({
left: marginThreshold + 1,
left: marginThreshold - 1,
top: marginThreshold,
}));

const positioningStyle = getElementStyleOfOpenPopover(mockedAnchor);

expect(positioningStyle.top).to.equal(`${marginThreshold}px`);

expect(positioningStyle.left).to.equal(`${marginThreshold}px`);
expect(positioningStyle.transformOrigin).to.match(/1px 0px( 0px)?/);

expect(positioningStyle.transformOrigin).to.match(/-1px 0px( 0px)?/);
});

describe('right > widthThreshold', () => {
let innerWidthContainer;

before(() => {
innerWidthContainer = window.innerWidth;
window.innerWidth = marginThreshold * 2;
});

after(() => {
window.innerWidth = innerWidthContainer;
});

specify('test', () => {
const mockedAnchor = document.createElement('div');
stub(mockedAnchor, 'getBoundingClientRect').callsFake(() => ({
left: marginThreshold + 1,
top: marginThreshold,
}));

const positioningStyle = getElementStyleOfOpenPopover(mockedAnchor);

expect(positioningStyle.top).to.equal(`${marginThreshold}px`);
expect(positioningStyle.left).to.equal(`${marginThreshold}px`);
expect(positioningStyle.transformOrigin).to.match(/1px 0px( 0px)?/);
});
});
});
});

describe('positioning when `marginThreshold=null`', () => {
it('should not apply the marginThreshold when marginThreshold is null', () => {
const mockedAnchor = document.createElement('div');
const valueOutsideWindow = -100;
stub(mockedAnchor, 'getBoundingClientRect').callsFake(() => ({
top: valueOutsideWindow,
left: valueOutsideWindow,
}));

let style;
render(
<Popover
anchorEl={mockedAnchor}
open
TransitionProps={{
onEntering: (node) => {
style = node.style;
},
}}
marginThreshold={null}
slotProps={{ paper: { component: FakePaper } }}
>
<div />
</Popover>,
);

expect(style.top).to.equal(`${valueOutsideWindow}px`);
expect(style.left).to.equal(`${valueOutsideWindow}px`);
expect(style.transformOrigin).to.match(/0px 0px( 0px)?/);
});
});
});
Expand Down

0 comments on commit c54ad8f

Please sign in to comment.