-
Notifications
You must be signed in to change notification settings - Fork 21
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.
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
SPIKE - RSC-815 Intelligent Dropdown Placement #591
base: main
Are you sure you want to change the base?
Conversation
…red-components into RSC-815-intelligent-dropdown-placement
dropdownDivRef: RefObject<HTMLDivElement>, | ||
childrenCount: number, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than computing the number of JSX children and then doing math that makes assumptions about the styling, why not just measure the div?
// childrenCount is added as a dependency because in components like NxSearchDropdown, the height of the | ||
// menu depends on the number of results/menu items shown in the component. | ||
useEffect(() => { | ||
window.addEventListener('scroll', determinePlacement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should get design input on whether it should move when scrolling occurs. Additionally this implementation is limited in that it won't handle scrolling of containers other than the viewport itself
// menu depends on the number of results/menu items shown in the component. | ||
useEffect(() => { | ||
window.addEventListener('scroll', determinePlacement); | ||
window.addEventListener('resize', determinePlacement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider useResizeObserver
to check for resizes of the element's container, not just the viewport
The more I think about this the more different cases I see that we need to determine whether or not they need special behavior. Here's a list of what I can think of so far:
Some of these are more feasible to address than others, and some are more important to address than others. The most important ones IMO, which are also some of the most doable, are 1 and 6. Of the rest, many are more about convenience than breakage (does the user need to scroll to see the menu), and some would be very difficult (trying to figure out whether a parent container is or isn't cutting off an overflowing menu) |
Co-authored-by: Ross Pokorny <[email protected]>
https://issues.sonatype.org/browse/RSC-815