-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Issue #5733: MenuPlacer useLayoutEffect depend on isLoading #5736
base: master
Are you sure you want to change the base?
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 6423178:
|
Hi @Methuselah96, I'd like to put this PR on your radar for approval. Thanks! |
Hello, is there something more I can do to get a reviewer to take a look at this PR? I see that there isn't much activity on this repository/package. Thanks. |
Hey there, pinging this PR again in the hopes that someone is able to take a moment to triage/review. I appreciate it. |
@@ -317,6 +318,7 @@ export const MenuPlacer = < | |||
) => { | |||
const { | |||
children, | |||
isLoading, |
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.
@labkey-nicka I'm not a maintainer but while fixing the same issue I found that you can get the isLoading property from props.selectProps.isLoading
. Therefore, we might not need an extra isLoading
prop here.
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.
Hey, thanks! Indeed isLoading
is available on props.selectProps
. Refactored and updated.
Should I continue to ping on this or just close it? Is react-select accepting changes? |
@labkey-nicka The dev team is slow in this aspect. I'd say keep the PR open but do some workaround on your end in the meantime. I've used patch-package or yarn 3 to patch them silently for my projects. |
Happy 1st Birthday! |
Is there any update on this? It is an important improvement I would say. |
Fixes #5733. This PR passes the
isLoading
prop down to theMenuPlacer
component and then relies onisLoading
as a dependency for running theuseLayoutEffect
to recompute the position of the menu. Subsequently,getMenuPlacement
will recalculate the menu's position.