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

Popup: Using React Portal instead of re-rendering the RootView #1196

Open
manishoo opened this issue Mar 17, 2020 · 2 comments
Open

Popup: Using React Portal instead of re-rendering the RootView #1196

manishoo opened this issue Mar 17, 2020 · 2 comments
Labels
enhancement help wanted stale This ticket may be old, or may no longer be valid, and will be closed if no further activity occurs

Comments

@manishoo
Copy link

I understand that, currently, when a modal is shown or dismissed, the changes are reflected with the updating of the RootView component using the ReactDOM.render function, which will re-render the whole tree, irrespective of any underlying shouldComponentUpdates.

In my case, I have a relatively large application and the showing and dismissing of these Popups and Modals—because it tries to render everything on each of these events—has a very poor user experience.

Now, I was wondering if it would be possible and more appropriate, for the sake of better performance, to implement this functionality on the web using React Portal?

@ngburke
Copy link

ngburke commented Apr 7, 2020

I had the same basic issue as you for my app. I am using the popup as sort of a 'combo-input' where the user types and various autocomplete options are shown. Here is what I had to do to as a workaround to avoid the bad performance hits to my app.

  1. Wrap the entire entry point of your app with something that blocks renders from the root view. This is not enough as you mentioned, but it is required.
export class MainWrapper extends RX.Component {
  shouldComponentUpdate() {
    // React XP triggers a re-render from the root when a popup is shown
    // This wrapper prevents that from trickling down
    return false
  }
  render() {
    return <Main/>
  }
}
  1. I don't know why this works 😞 (not accustomed to putting in hacky 'workarounds' when I don't know why it works), but I had to delay calls to RX.Popup.show and RX.Popup.dismiss with setImmedate(). Here are the snippets and my comments:
        // In the case where a picker box is open and a different one is clicked,
        // there is a race condition in React XP that will dismiss both boxes immediately.
        // The workaround below is to wait a tick to allow dismissal of the first box before
        // drawing the second box.
        setImmediate(() => RX.Popup.show({
          cacheable: true,
          positionPriorities: ['bottom', 'right'],
          getAnchor: () => { return this._viewAnchor },
          renderPopup: () => <RX.View style={ Style.tableCellInput.comboView }>{ this._renderComboItems(list) }</RX.View>,
        }, this._selectBoxId))
    // This is a performance workaround for React XP. If the popup dismiss
    // call does not wait a tick, then all components are force rendered.
    setImmediate(() => RX.Popup.dismiss(this._selectBoxId))

As an aside, the last time I brought this up the issue was closed as kind of 'not a problem' (#833). Any additional insight into these workarounds or if we are doing something wrong is appreciated.

@fbartho
Copy link
Contributor

fbartho commented Aug 9, 2020

This feels like an annoyance. -- We don't use RX.Popup or RX.Modal much in our applications, so I'm not entirely sure what we need to do in an ideal world.

Does anybody have a suggestion?

@fbartho fbartho added help wanted stale This ticket may be old, or may no longer be valid, and will be closed if no further activity occurs labels Aug 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement help wanted stale This ticket may be old, or may no longer be valid, and will be closed if no further activity occurs
Projects
None yet
Development

No branches or pull requests

3 participants