inveniosoftware / react-invenio-app-ils

Single Page App built with React for InvenioILS.
https://react-invenio-app-ils.readthedocs.io
MIT License
5 stars 19 forks source link

modals: avoid mounting the component when not visible #31

Open ntarocco opened 4 years ago

ntarocco commented 4 years ago

Currently the modal with semantic-ui is in this form (a button inside the modal):

<Modal
        trigger={
          <Button
              ...

The issue happens when rendering a page with 2 modals, e.g this one: localhost_3000_backoffice_documents_cgdgp-q3821

The 2 modals are actually mounted and rendered, even if invisible, as the button to open them. When we open 1 of the 2 and we interact with it, we change props and therefore also the other invisible modal silently re-render and re-evaluates new props. This is not ideal.

How to reproduce To reproduce it, try to put a console.log inside the render method of RelationModal component and render the page. In the example above, both the first and the second print in the console.

Possible solution A solution would be to somehow render only the button without attaching the modal component: on click, the modal should be attached and become visible. On close, it should be detached.

  handleOpen() => {
    this.setState({ modalOpen: true });
  }

  handleClose() => {
    this.setState({ modalOpen: false });
  }

  render() => {
    <Button onClick={this.handleOpen}>Show Modal</Button>
    {this.state.modalOpen && (<ModalComponent isOpen={} />)
  }

There might be another better solution.

TODO

Ping @zzacharo @kprzerwa @topless if you have better ideas.

zzacharo commented 4 years ago

@ntarocco Some questions/observations just to understand the problem: 1) The 2 modals are rendered even invisible: Here you refer to the semantic UI modals or our wrapper components that encapsulate some logic? I am asking because I dug a bit into semantic code and I see here that if the state of the modal is not open only the button is rendered. 2) I see that now because we wrap the Semantic Modals with our logic e.g RelationsModal we have always a component there mounted that even if it shows the underlying modal on demand, still is being re-rendered whenever a depending prop is updated. One alternative solution that I would propose is to use the shouldcomponentupdate method and allow updates only when the internal state is visible :)