(dropdown): Update dropdownMenuContent to expose portal container prop #5189
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the content includes the portal component, however we do not have access to the portal props, including `container`. if we want to portal our main menu to another element, we can not. i think if the container includes the portal component, the portal props should be exposed, at least the container. as it stands currently, shad exports the menuContent that includes the portal, as well as the menuportal component itself. it also points users to the radix documentation for proper use of the api. radix says to wrap your element in a portal component and provide the container prop. the confusing situation that i found myself in, was i tried wraping the menucontent with the menuportal that was provided. this did not work and it took me a very long time to figure out that content was already wrapped in a portal. my specific use case for this change is on our doc site, all of our code exists within an element with a specific id and our tailwind classes are prefixed with that ID, so we need to portal the dropdown to exist within that element with that specific id.
|
@kmccrory-grx is attempting to deploy a commit to the shadcn-pro Team on Vercel. A member of the Team first needs to authorize it. |
kmccrory-grx
changed the title
kmccrory-grx
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
the dropdownMenuContent includes the portal component, however we do not have access to the portal props, including
container. if we want to portal our main menu to another element, we can not.i think if the container includes the portal component, the portal props should be exposed, at least the container.
as it stands currently, shad exports the menuContent that includes the portal, as well as the menuportal component itself. it also points users to the radix documentation for proper use of the api. this is confusing because radix says to wrap your element in a portal component and provide the container prop.
the situation that i found myself in, was i tried wraping the menucontent with the menuportal that was provided. this did not work and it took me a very long time to figure out that content was already wrapped in a portal.
my specific use case for this change is on our doc site, all of our code exists within an element with a specific id and our tailwind classes are prefixed with that ID, so we need to portal the dropdown to exist within that element with that specific id.
i think the options here are,
• decouple menuportal from menucontent
• give access to portal props from menucontent
i would lean towards decoupling since that is how the api in the radix docs has it laid out