Removal of isOpen?

[STRML]

Related to this comment, I always thought it odd that this component had a property isOpen for conditional rendering, when developers are capable of managing that themselves:

return (
  <div>
    <button onClick={() => this.setState({opened: !this.state.opened})}>Toggle</button>
    {this.state.opened ? 
      <Portal><div>Portal Contents</div></Portal>
    : null}
  </div>
);

Ideally, the user should manage the state themselves and Portal should have no state.

Existing helpers like closeOnEsc should then only call the onClose callback, and not actually close the portal unless the developer desires.

This also solves a few bugs, like the following:

• Create Portal with isOpen={true} and closeOnEsc={true}
• User presses esc, closing the portal
• Portal's containing component rerenders for any reason, causing componentWillReceiveProps to be called
• Portal unexpectedly re-opens

To ease the transition, perhaps it would make sense to ship a wrapper component that manages this state for you for common cases. This is similar to what I have done in other projects with <Resizable>/<ResizableBox> and <Draggable>/<DraggableCore>, which has worked nicely.

Said stateless component would also serve as a very low-overhead building block for other components.

[tajo]

I like this idea a lot. A wrapper component is must have.

It makes the library more readable and nicely separates controlled (stateless component) and uncontrolled (wrapper component) approaches.

The problem is, this means to do a complete rewrite.

[evandavis]

The problem is, this means to do a complete rewrite.

I think you might be able to do this by just taking all of the state and moving it to an HOC, and export both. That way it's an upgrade, not a rewrite. I'd be willing to start a PR if that's something you're interested in.

[tajo]

@evandavis Yes, please! That's exactly what I want to do. Ideally, it should still pass all (or most of) current tests.

[STRML]

See https://github.com/STRML/react-portal-minimal - I have not yet built the compatibility layer, but as you can see the project is much simpler without the state.

[tajo]

Removed: #157