-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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: switch to onClick from onMouseDown in modal components #16847
fix: switch to onClick from onMouseDown in modal components #16847
Conversation
✅ Deploy Preview for v11-carbon-react ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for carbon-elements ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
This looks like a fix to me !!
@tay1orjones resolved 👍 |
@davidmenendez would you be up for writing a small test to ensure the consumer-provided onClick is called like we expect? |
@tay1orjones tests included 👍 |
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.
LGTM! 🚀
@tay1orjones when you have a chance 👍 |
@tay1orjones 👀 😬 |
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.
Sorry for the delay, was out on vacation last week 😅
b12ac2e
Closes #16846
switches the outside click detector event in
Modal
andComposedModal
fromonMouseDown
toonClick
in order to alleviate some strange behaviors noticed with components built onListBox
inside of modal components.Changelog
Changed
onMouseDown
event toonClick
inModal
andComposedModal
Testing / Reviewing