Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-introduce Portals support #4849

Closed
cschleiden opened this issue May 11, 2018 · 2 comments · Fixed by #6211
Closed

Re-introduce Portals support #4849

cschleiden opened this issue May 11, 2018 · 2 comments · Fixed by #6211
Assignees
Milestone

Comments

@cschleiden
Copy link
Contributor

The API used by the current Layer implementation: unstable_renderSubtreeIntoContainer has a number of problems.

  1. It will be deprecated in a future React minor release ([Bug] New Context API does not work with ReactDOM.unstable_renderSubtreeIntoContainer facebook/react#12493 (comment))
  2. It does not support the new context
  3. It looks like a normal React component to consumers, but lifecycle methods behave in a very unexpected way

Just replacing that call with Portals works, but the great thing about portals - events bubble, and everything behaves like it's a single React tree - leads to problems for users who upgrade. Key events in a contextual menu, for example, now bubble up if they are not caught by the focus zone in the menu.

Maybe we can create something like an <EventBoundary> component that prevents that, and that could be used by components that use Layer but don't want the event bubbling behavior.

@cschleiden
Copy link
Contributor Author

Some more discussion about the problem in general is here: facebook/react#11387

Recommendation from the team so far seems to be don't nest the portal content, like this:

class Foo extends React.Component {
  state = {
    highlight: false,
    showFlyout: false,
  };

  mouseEnter() {
    this.setState({ highlight: true });
  }

  mouseLeave() {
    this.setState({ highlight: false });
  }

  showFlyout() {
    this.setState({ showFlyout: true });
  }

  hideFlyout() {
    this.setState({ showFlyout: false });
  }

  render() {
    return <>
      <div onMouseEnter={this.mouseEnter} onMouseLeave={this.mouseLeave} className={this.state.highlight ? 'highlight' : null}>
        Hello
        <Button onClick={this.showFlyout} />
      </div>
      {this.state.showFlyout ? <Flyout onHide={this.hideFlyout} /> : null}
    </>;
  }
}

@cschleiden
Copy link
Contributor Author

Something like this, EventBoundary might work: https://codesandbox.io/s/5zlnyxjrrn

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants