react-modal icon indicating copy to clipboard operation
react-modal copied to clipboard

Question: Why doesn't react-modal use imperative style?

Open Webbrother opened this issue 5 years ago • 11 comments

Summary:

Why doesn't react-modal use imperative style?

I mean, following:

class MyComponent extends Component {
  constructor() {
    super();
    ...
    this.modal = ...
    ...
    this.handleClick = this.handleClick.bind(this);
  }
  
  handleClick() {
    this.modal.open(<div>Some content of modal.</div>);
  }
  
  render() {
    return (<button onClick = {this.handleClick} >Click me to open modal</button>);
  }
}

Webbrother avatar Jun 06 '19 16:06 Webbrother

Hi @Webbrother, do you mean expose the open/close methods?

diasbruno avatar Jun 06 '19 16:06 diasbruno

@diasbruno correct.

Webbrother avatar Jun 06 '19 16:06 Webbrother

Can you provide a case where this can be helpful? (Of course there is, but just so we can see where isOpen attribute is not sufficient).

diasbruno avatar Jun 06 '19 16:06 diasbruno

@diasbruno I've provided some dummy example in the first message. Actually my question doesn't mean that current implementation bad or good. My question just "Why?". My question more about conception. IMO modal is global UI element, and it should not be a child of MyComponent.

So I just thought that

...
render() {
  <div>
      <SomeComponent />
      <Modal>...</Modal>
  </div> 
}

is a bit confusing. We declare <Modal> like child of my current component, but in fact it is not a child, because it is global UI element.

Webbrother avatar Jun 06 '19 17:06 Webbrother

@Webbrother Sure. Just trying to figure it out how this can help.

...but in fact it is not a child, because it is global UI element.

react-modal doesn't works as a singleton instance, you can instantiate as many modals as you need and even nest than.

We declare <Modal> like child of my current component,...

This gives cohesion, to instantiate where it is used.

Hope this explains how this library works.

Let me know if you have other questions.

Thanks.

diasbruno avatar Jun 06 '19 17:06 diasbruno

@diasbruno Thank you. But IMO imperative style can also process mentioned use cases (multiple instances (just call this.modal.open() twice...), nesting, etc...) What problems imperative style can cause?

Webbrother avatar Jun 06 '19 18:06 Webbrother

If you prefer an imperative API, you can always wrap this component to do as you see fit.

class Modal extends React.Component {
  state = {
    isOpen: false
  }

  render() {
    return (
      <ReactModal {...this.props} isOpen={this.state.isOpen} />
    )
  }

  open() {
    this.setState({ isOpen: true })
  }

  close() {
    this.setState({ isOpen: false })
  }
}

That said, I liked the imperative API 4 years ago, but came to personally dislike it because:

  • Rendering should be declarative.
  • I stick with state for everything unless there's a reason not to. It's intuitive and gives me full control over the modal.
  • I avoid writing APIs that rely on ref instances. If anything, it's my last option.

Disclaimer: I'm not a contributor to this project, but I hope this answers your question.

srph avatar Jun 12 '19 13:06 srph

Declaring the state of the UI, rather than setting it imperatively, is at the core of React. This library simply reflects that. You can always find a way to bridge things to an imperative system if you need to, but it's natural that a React library will be declarative-first.

hen-x avatar Oct 14 '19 08:10 hen-x

just wanted to cast another vote for an imperative API.

if the modal lives entirely in the render method, then its content has to live in the state/props.

compare:

function (props) {
  const handleClick = () => {
    fetch('a thing').then(res => Modal.open(<BrandedAlert message={res} />))
  }
  return <button onClick={handleClick}>do a thing</button>
}

to

function (props) {
  const [ fetchResponse, setFetchResponse ] = useState()
  const [ modalOpen, setModalOpen ] = useState(false)
  const handleClick = () => {
    fetch('a thing').then(res => {
      setFetchResponse(res)
      setModalOpenTrue(res)
    }))
  }
  return <>
    <button onClick={handleClick}>do a thing</button>
    <Modal isOpen={modalOpen}><BrandedAlert message={fetchResponse}/></modal>
  </>
}

Pseudo code, but you get the idea.

now imagine a single component that opens multiple modals. state vars would quickly proliferate.

now imagine we wanna pass a function to <BrandedAlert /> which modifies the parent component state with values scoped to handleClick.. those would have to go into state as well. maybe thats ideal - but it seems like a lot of leg work for not a great deal of benefit.

even cooler - is if the modal component (BrandedAlert in this example) could close with a response (resolve)

would allow stuff like:

 Modal.open(<BrandedPrompt  />).then(response => fetch( .... ))

EDIT: for anyone who comes across this later - I've started a bare-bones implementation of an imperative modal API

https://github.com/nihlton/react-imperial-modal

needs work - but its a start.

nihlton avatar Apr 29 '20 01:04 nihlton

Ability to use imperative call may be important for optimizations and clean code. Simplified use case below

Prerequisites

const Root = (props) => () => {
  return (
    <Parent>
      {children.map(({ data }) => (
        <Child data={data} />
      ))}
    </Parent>
  );
};

const Child = ({ data }) => () => {
  return (
    <button></button>
  );
};

Declarative Way

And now if we want add some modal with data controlled via children's buttons, with declarative style we should add it into each child:

const Child = ({ data }) => () => {
  const [isModalOpen, setIsModalOpen] = useState(false);

  return (
    <>
      <button onClick={() => setIsModalOpen(!isModalOpen)}></button>

      <Modal isOpen={isModalOpen}>
        {data}
      </Modal>
    </>
  );
};

It requires definition and processing Modal component a lot of times, for every child, even if it will not be opened. And also there appears some state variable, and another one level on nesting

Imperative Way

Instead, if we have imperative Modal api, this code can be simplified to this:

const Child = ({ data }) => () => {
  return (
    <button onClick={() => openModal(<div>{data}</div>)}></button>
  );
};

So now nothing is rendered before it has to, there are no excess variables, and no code-repeating

P.S.

Though React is declarative, but JS itself is not. This is why in React there are a lot of ways to get imperative direct control back. Modal dialogs are not so declarative, and let's be honest, this:

const [isOpen, setIsOpen] = useState(false);
// ...
setIsOpen(true)

is just longer version of this:

openDialog();

Modals are still imperative in most cases, just in some strange way. So additinal ability to open Modal directly could be very useful <3

agdust avatar Jun 09 '22 14:06 agdust

Ok. Here is the deal :thinking:

What if we could use this imperactive style in a way that is opt-in?

Maybe something like...we could use @srph's idea and provide it under import ModalImperactive from 'react-modal/imperactive'.

Any thoughts?

PRs are always welcome.

diasbruno avatar Jun 09 '22 14:06 diasbruno