-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: Dropdown disabled toggle issue #1571
fix: Dropdown disabled toggle issue #1571
Conversation
This fix addresses the issue where Dropdown will be passed a `disabled` prop, yet it is still clickable. This is due to context being passed the user-defined `toggle` prop directly, rather than our defined `this.toggle` function that includes a check for `disabled`. Fixes reactstrap#1542
hmm, I'm not exactly sure why those tests are failing, they seem unrelated to the changes I committed. Anyone have any ideas what could be causing this? |
I think you found some bad tests. It looks like it was expecting the Dropdown toggle to not be called... when it should have been getting called. Since you fixed it to call Dropdown's toggle (which in turn calls the user's toggle), those tests fail. In reality, those tests probably should have been failing this whole time and this PR would have make them pass. Can you change |
I had a feeling they were wrong 😉 will do 👍 |
In the previous implementation, Dropdown wasn't properly firing the toggle function within Dropdown, so the assertion was written down wrong to compensate for that.
This applies the CustomEvent polyfill recommended in the [MDN web docs](https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/CustomEvent#Polyfill). This also applies a simple polyfill I found in the (react-transition-group module)[https://github.com/reactjs/react-transition-group/blob/9e3b2d3c09b78e755bdc837b7b38337812ede2c9/src/TransitionGroup.js#L11]. I don't think this has much potential to break anything, since these will only impact browswers where these features are already broken.
This applies the CustomEvent polyfill recommended in the MDN web docs (https://developer.mozilla.org/en-US/docs/Web/API/CustomEvent/CustomEvent#Polyfill). This also applies a simple polyfill for Object.values I found in the react-transition-group module (https://github.com/reactjs/react-transition-group/blob/9e3b2d3c09b78e755bdc837b7b38337812ede2c9/src/TransitionGroup.js#L11). Fixes reactstrap#1571
This fix addresses the issue where Dropdown will be passed a
disabled
prop, yet it is still clickable. This is due to contextbeing passed the user-defined
toggle
prop directly, rather thanour defined
this.toggle
function that includes a check fordisabled
.Fixes #1542