Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

recalculate menu top position on scroll when using menuPosition='fixed' #3646

Closed
xmile1 opened this issue Jun 23, 2019 · 7 comments
Closed

recalculate menu top position on scroll when using menuPosition='fixed' #3646

xmile1 opened this issue Jun 23, 2019 · 7 comments
Labels
issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet

Comments

@xmile1
Copy link

xmile1 commented Jun 23, 2019

Hi, thanks for having this awesome library, it has been very helpful.

There seems to be a bug when using menuPosition fixed with react select except of course I am missing something.

The menu position should be with the select box when scrolling the page vertically but its fixed and isn't recalculated on scroll.

I have reproduced it here
https://codesandbox.io/s/react-codesandboxer-example-xy47y
@kalbert312
Copy link

kalbert312 commented Aug 15, 2019 •
edited
Loading

Bumped into while searching for menu. I came up with a hacky solution for an older version of react-select. Wasn't perfect, but it even handled nested scrolling by hiding the menu if the inner scroll container hid the select.

Might be of use:

	updateMenuPosition = () => {
		this.setState((prevState) => {
			const wrapper = $(this.wrapperRef);
			const menuElement = $('.Select-menu-outer');
			const isMenuOpen = menuElement.length > 0;

			let menuPositionStyling = null;
			let maxMenuHeightOffset = null;
			if (isMenuOpen) {
				const menuBoundingBox = menuElement[0].getBoundingClientRect();
				const realMenuHeight = menuBoundingBox.height;
				const unconstrainedMenuHeight = realMenuHeight - prevState.maxMenuHeightOffset;
				const scrollableContainer = wrapper.parents('.scrollable-container')[0];
				const scrollableContainerBoundingBox = scrollableContainer != null ? scrollableContainer.getBoundingClientRect() : null;
				const controlBoundingBox = wrapper.find('.Select-control')[0].getBoundingClientRect();
				const borderOffset = 2;

				menuPositionStyling = {
					position: 'fixed',
					top: controlBoundingBox.top + controlBoundingBox.height + borderOffset, // + 2 to go below border
					left: controlBoundingBox.left,
				};

				// hack: safari scrolling element is <body>, other browsers is <html>. Getting/setting scrollTop on this element will make the below functionality work on both browsers while still using html element for boundaries.
				// document.scrollingElement is not defined for IE 11, so fallback to htmlElement.
				const htmlElement = document.documentElement;
				const pageScrollingElement = document.scrollingElement || htmlElement;

				const pageScrollable = htmlElement.clientHeight < htmlElement.scrollHeight;
				let heightBoundary = window.innerHeight;
				let hide = controlBoundingBox.top < 0 || controlBoundingBox.bottom > htmlElement.clientHeight;

				if (pageScrollable) {
					const remainingScrollDistance = htmlElement.scrollHeight - pageScrollingElement.scrollTop - htmlElement.clientHeight;
					heightBoundary += remainingScrollDistance;
				}

				if (menuPositionStyling.top + unconstrainedMenuHeight > heightBoundary) { // place dropdown on top
					if (scrollableContainerBoundingBox != null &&
						(controlBoundingBox.top < scrollableContainerBoundingBox.top || controlBoundingBox.top > scrollableContainerBoundingBox.bottom)) {
						hide = true;
					}
					menuPositionStyling.top = controlBoundingBox.top - borderOffset - unconstrainedMenuHeight;
					if (menuPositionStyling.top < 0) {
						maxMenuHeightOffset = menuPositionStyling.top;
						menuPositionStyling.top = 0;
					}
				} else { // dropdown on bottom
					if (scrollableContainerBoundingBox != null &&
						(controlBoundingBox.bottom < scrollableContainerBoundingBox.top || controlBoundingBox.bottom > scrollableContainerBoundingBox.bottom)) {
						hide = true;
					}
					if (!hide) {
						const calculatedMenuBottom = menuPositionStyling.top + menuBoundingBox.height + pageScrollingElement.scrollTop;
						if (pageScrollable && calculatedMenuBottom > htmlElement.clientHeight) {
							pageScrollingElement.scrollTop = Math.max(calculatedMenuBottom - htmlElement.clientHeight, pageScrollingElement.scrollTop);
						}
					}
				}

				if (hide) {
					menuPositionStyling.display = 'none';
				}
			}

			return {
				menuPositionStyling,
				maxMenuHeightOffset,
			};
		}, () => this.forceUpdate());
}

Had a window scroll handler call that.

@m-andrew-albright
Copy link

Is there any update on this issue?

@xmile1
Copy link
Author

xmile1 commented Sep 14, 2019 •
edited
Loading

FWIW, I currently use this hack

  handleCloseMenuOnScroll = () => {
    // forcing select menu re-rendering on scroll
    this.setState({ top: 0 });
   // as expected do not close the menu on scroll
    return false;
  }

<Select
     ...
     styles={{
         menuPortal: base => ({
            // this just updates the menuPortal style to trigger a rerender
            top: this.state.top,
            ...base,
         }),
     }}
     closeMenuOnScroll={this.handleCloseMenuOnScroll}
/>

@HodayaGruz
Copy link

HodayaGruz commented Jan 26, 2020 •
edited
Loading

hi, did anyone find a way to fix it?

@xmile1
Copy link
Author

xmile1 commented Jan 27, 2020

hi, did anyone find a way to fix it?

@HodayaGruz have you tried #3646 (comment) above ?

@HodayaGruz
Copy link

@xmile1 i tried, the menu is stick to the top of the page

@bladey bladey added the issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet label Jun 1, 2020
@bladey bladey mentioned this issue Jun 15, 2020
Closed
@bladey
Copy link
Contributor

bladey commented Jun 15, 2020

Hello -

Thanks for reporting this issue.

This bug has been reported multiple times as per issue #4088.

A new master issue #4088 has since been created to keep track this this bug.

This new issue will exist as the source of truth going forward to investigate the issue, report findings, and implement a bug fix.

We'll take into account all the details above while investigating.

If you feel this issue has been wrongly closed as it isn't related to the new master issue #4088, please let us know and we'll take another look.

Thank you!

@bladey bladey closed this as completed Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@bladey @m-andrew-albright @kalbert312 @xmile1 @HodayaGruz