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.

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

fix: Add ChakraMenu to provide context for MenuIcon dependency #269

Merged

Conversation

malinskibeniamin
Copy link
Contributor

This is required because of the issue in the latest Chakra UI release where useMenuStyles will break the app.

See example from Storybook
Screenshot 2023-07-24 at 14 24 37

malinskibeniamin and others added 2 commits July 24, 2023 14:43
This is required because of the issue in the latest Chakra UI release where useMenuStyles will break the app.
@codesandbox-ci
Copy link

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 852b139:

Sandbox Source
chakra-react-select demo Configuration
chakra-react-select TS demo Configuration
chakra-react-select + next.js Configuration
chakra-react-select + next.js + typescript Configuration

@csandman
Copy link
Owner

Thanks for pushing this, and bringing it to my attention! There were a couple issues with your original push though. The comment you included wasn't in a valid spot. Also, you were including a Menu for every option, where as there's no need to include more than one. I ended up moving it to the react-select Menu component to reduce duplication.

@csandman csandman self-requested a review July 24, 2023 16:11
Copy link
Owner

@csandman csandman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@csandman csandman merged commit baed0c1 into csandman:main Jul 24, 2023
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants