Refactor as an input for using isOpened correctly

[Sinewyk]

We thought about it with a colleague and we think that a small refactoring to work exactly as an input would be great.

If you do NOT use isOpened, the portal is in a non controlled state. You close it via the optional props. Nothing is different from before.

If you DO use isOpened, the Portal enter controlled state.

With the value prop of an input being equivalent to the isOpened prop of the portal and toBeDefined equivalent to onChange (or beforeClose, not sure yet).

Anything wanting to close the portal will declare its intent to close the portal, the user will catch that and be able to change its state correctly so that isOpened is false. But you do not actually edit the active state of the portal to false or do anything at all inside the portal, except declare your intent to close the portal (if using closeOnEsc and pressing escape, you will trigger the onChange equivalent, but the portal will not close unless the user re-renders with isOpened to false or unmounts the Portal).

And of course, we do not forget the warning in propTypes that isOpened MUST come with the portal equivalent onChange prop.

Thoughts ?

[tajo]

I think I don't get it. What's the use case? Pull request?

[Sinewyk]

Don't have a PR yet.

The use case is related to maybe

<A>
  <Portal isOpened={some A state}/>
</A>

and you forget onClose to change the state in A responsible for isOpened back to false. If something triggers a rerender of A, your Portal will open back up and you will be left scratching your head.

In controlled state, if you omit the onChange equivalent you have a warning that if you are using isOpened, you need to use to listen to closing events, and then it's a simple matter of opening/closing the portal related to isOpened in componentWillReceiveProps and simply trigger the listener in closeOnEscape, closeOnClickOutside, etc ...

But by default, the portal will not close up anymore if you use isOpened, user have to send isOpened to false for closing/opening logic to happen only in componentWillReceiveProps (and componentWillUnmount) via isOpened. closeOnEsc, closeOnOutsideClick and calling closePortal from the portal children, will simply trigger the onChange (or whatever you want to call it) so that user can do his stuff for the state (isOpened in this case).

Just like a controlled input.

[tajo]

I get it know. This is a good idea. Yeah, it would be helpful to introduce the onChange prop for the controlled state. It doesn't have to be mandatory (or firing some warning) though - no unnecessary BC break. People still can use the controlled state if they already implemented all closing logic in upper layers (outside of react-portal). Also, if they don't need to utilize closeOnEsc, closeOnOutsideClick or closePortal, onChange would be redundant (use-case when the portal is opened and closed programmatically, by server... etc).

Also, the README.md should be updated with terms controlled / uncontrolled. It really makes sense and it is a concept that React devs are familiar with already (from forms).