[chore] Update doc to inform v3 users about close transition #672
Conversation
|
Thanks, @hoodwink73. I'll have a look on your PR later. |
docs/styles/transitions.md
Outdated
|
|
||
| The above example will apply the fade transition globally, affecting all modals | ||
| whose `afterOpen` and `beforeClose` classes have not been set via the | ||
| `className` prop. To apply the transition to one modal only, you can change | ||
| the above class names and pass an object to your modal's `className` prop as | ||
| described in the [previous section](classes.md). | ||
|
|
||
| In order for the close transition to take effect, you will also need to pass | ||
| the `closeTimeoutMS={150}` prop to each of your modals. |
There was a problem hiding this comment.
Maybe this can be rephrased, something like: The close animation is controlled via timer which you can define using closeTimeoutMS={number}, where the value is the time interval in milliseconds. The value must be synchronized with the css transition you have specified.
What do you think?
There was a problem hiding this comment.
This is explicit and better. I will update this.
There was a problem hiding this comment.
Ah, also this part:
The value must be synchronized with the css transition or style you have specified (because styles can also be used in react-modal).
docs/styles/transitions.md
Outdated
|
|
||
| Also, if you are using `v3` which supports **React 16**, the close transition works [only if you use](https://github.com/reactjs/react-modal/issues/530#issuecomment-335208533) the `isOpen` prop to toggle the visibility of the modal. | ||
|
|
||
| Do not conditionally render the `<Modal />`. |
There was a problem hiding this comment.
Maybe we need to elaborate here because it works for react 15, but not in 16 (if I remember correctly).
It seems that the conditional rendering in react 16+ will destroy the rendered elements unconditionally (seems the reason why it isn't work anymore). So, we probably hold the reference for the modal DOM tree, but it's not attached to any other element (this needs to be confirmed).
There was a problem hiding this comment.
The example is really awesome. 👍
There was a problem hiding this comment.
I am also not sure of why it does not work in React 16+ but in React 15. I will try to take a look at your suggestion of holding reference to the tree and try to come up with an explanation. Thanks for the hint.
There was a problem hiding this comment.
Does this look like a legit explanation to you regarding the limitation to control the unmounting of Portal?
facebook/react#10826
And this leads to the conditional rendering of <Modal /> not working.
I will try to do some debugging and experiment here.
There was a problem hiding this comment.
Great! This can be included in the doc.
There was a problem hiding this comment.
You can also answer #530, depending on the conclusion of your investigation and eventually close the issue. I'll continue to follow this and try to help where I can.
docs/styles/transitions.md
Outdated
| > | ||
| <h2>Add modal content here</h2> | ||
| </Modal> | ||
| : null |
diasbruno
changed the title
|
@diasbruno hey I did some debugging and figured out concrete reasons why conditional rendering is not working in React 16. I will add this to the opened issue. Thanks a lot for all help. Have a great weekend. |
|
Thank you so much. Great effort on getting this, @hoodwink73. |
|
🙌 I also posted a lengthy comment on #530 regarding my debugging efforts. It might help you to close the issue. |
|
Yeah, that was great. That should clarify what is going on. Great job. |
Does not fix any issue. But informs the user about the discussion in #530
Changes proposed:
v3be aware that close transition does not work with conditional render