Fixes #5959 - jquery-bootstrap example uses handleHidden which does not exist #5961
Conversation
| }, | ||
| componentWillUnmount: function() { | ||
| $(this.refs.root).off('hidden', this.handleHidden); | ||
| $(this.refs.root).off('hidden.bs.modal', this.handleHidden); |
There was a problem hiding this comment.
I'm not super familiar with this code, why did this line change? That is to say: what is the difference between hidden and hidden.bs.modal, and how do we know which one is correct?
There was a problem hiding this comment.
hidden wasn't working for me. I checked the Bootstrap docs which shows to use hidden.bs.model and that did work for me.
There was a problem hiding this comment.
Also, it may not have been working because there was no listener in place for either event ie.
$(this.refs.root).on('hidden.bs.modal', this.handleHidden);
// or
$(this.refs.root).on('hidden', this.handleHidden);I added that listener on line 25
There was a problem hiding this comment.
Actually, both event names work. The syntax is how jQuery namespaces events. So it wasn't working for me initially because there wasn't an event listener setup when the component was mounted.
|
@MarkMurphy updated the pull request. |
1 similar comment
|
@MarkMurphy updated the pull request. |
MarkMurphy
changed the title
|
Another kind of dumb question: what do I need to do to hide the modal? I tried clicking the X and tried clicking outside the modal box, neither seemed to trigger this code path. |
|
I'm starting to think that we don't need the hide logic at all. We could probably solve this entire issue by just deleting the line I'm not convinced it really adds anything to the example (beyond what's already there; we already handle a couple of different callbacks). |
|
@jimfb I was wrong to remove the event namespaces...we do need them for it to work properly.
I actually like having the example in there. There are other events people might want to hook into so showing them how they can do that is worth making this work. I'll push another update. |
|
@MarkMurphy updated the pull request. |
|
@jimfb If the modal doesn't disappear when you hit ok that's very strange... There's a native confirmation prompt that's supposed to interrupt the dismissal on the modal if you click on either the |
|
Ok, this appears to work. Can you squash into a single commit ( |
1. Add a handleHidden method to the BootstrapModal component. 2. Add an event listener for 'hidden.bs.modal' on the modal root 3. Add a new onHidden prop to the BootstrapModal component. 4. Add a new 'handleModalDidClose' method to the Example component to be used as the onHidden prop of it's modal component.
MarkMurphy
force-pushed
the
issue-5959
branch
from
e6d2c27 to
0bd65aa
Compare
|
@jimfb done. |
Fixes #5959 - jquery-bootstrap example uses handleHidden which does not exist
|
Thanks @MarkMurphy! |
Fixes #5959. I've fixed this by adding a hook into the modal's
hidden.bs.modalevent.