wix-playground / stylable-components

Fully-tested & strictly-typed component library based on React, using optional Wix styling.
MIT License
35 stars 3 forks source link

Managing Dialogs #130

Open tobich opened 7 years ago

tobich commented 7 years ago

How should dialogs (not only modal) be handled? The current spec follows other libraries, where dialogs are managed by parent components using the isOpen (or similar) prop/state.

For example:

<ConfirmFileRemoval isOpen={this.isCfrDialogOpen} onCloseRequest={() => this.isCfrDialogOpen = false} />

There's couple of problems with this approach:

  1. Such dialog is tied to another component. But dialogs are usually tied to a process/flow, not component.
  2. The dialog is always rendered, at least as "closed", regardless of whether is visible or not. So "closing a dialog" is more like "hiding a dialog"
  3. This approach doesn't scale well. All possible dialogs of some component must be part of its render function.
  4. The more potential dialogs, the slower parent render is

The alternative is having a central dialog manager. Opening a dialog is then a command/message sent to the manager, rather than changing its state.

this.dialogManager.openModal(<ConfirmFileRemoval onCloseRequest={dialog => this.dialogManager.close(dialog) } />

This approach solves all the above problems.

tyv commented 7 years ago

what is considered as this in this.dialogManager: user application, parent, root, router?

tyv commented 7 years ago

what if each time you render the <Dialog ... /> it does smth like

render() {
  Dialog.manager.whatEver()
}
idoros commented 7 years ago

I think declarative dialogs are better. It looks like any other component vdom. And The owner component shouldn't really care how / where / when the internal component is rendered.

Even in normal containment, there is no guarantee that the children you pass are going to render when you are. Their render might be prevented or delayed by some parent component or by React (>16).

In the future, I'd like to have the option when building a dialog to indicate that its a layer, which means that any other portals rendering from the same application context will not render when a layer is rendered, and any other layer from that context will queue up and wait for the layer to close. Layer opening from the layer context should open above it and cause the same behavior for any other portals in the layer.