Modal with passed trigger element, buttons can't close modal- usage question/feature request

[josie11]

Steps

Pass the Modal component a trigger element instead of using it as a controlled component.

Expected/Desired Result

Clicking the trigger element will open the Modal, and you have the option of passing a button in the <Modal.Action /> section a prop that can trigger closing the Modal. Maybe have a <Modal.Button /> or another prop that can trigger a close?

Actual Result

You can no longer use buttons in <Modal.Action /> or any other child elements to close the modal. You can only click outside the Modal to close it and/or use a closeIcon.

Version

latest

Testcase

Basic Modal example in the docs (the second example)

[levithomason]

This is the intended usage. In React, there are no implicit parent-child relationships. So, the buttons cannot automatically close the Modal unless they are explicitly wired to do so (e.g. onClick -> set state).

The key is here:

instead of using it as a controlled component

In order to do this, the Modal must be controlled.

In the future, we plan to support defining Modals with shorthand props. This would allow you do accomplish what you are asking for, but, you wouldn't do it with children, you would do it with props that we provide. This is possible because we can take the prop values and internally instantiate all the children and, while doing so, add the appropriate event listeners and state mutations.

It might look something like this (note, this is not a planned API, just an example):

<Modal
  trigger={<Button>Basic Modal</Button>}
  basic
  size='small'
  header={{ icon: 'archive', content: 'Archive Old Messages' }}
  content='Your inbox is getting full, would you like us to enable automatic archiving of old messages?'
  actions={[
    { basic: true, color: 'red', inverted: true, icon: 'remove', content: 'No', },
    { color: 'green', inverted: true, icon: 'checkmark', content: 'Yes' },
  ]}
/>

As opposed to the current:

<Modal trigger={<Button>Basic Modal</Button>} basic size='small'>
  <Header icon='archive' content='Archive Old Messages' />
  <Modal.Content>
    <p>Your inbox is getting full, would you like us to enable automatic archiving of old messages?</p>
  </Modal.Content>
  <Modal.Actions>
    <Button basic color='red' inverted>
      <Icon name='remove' /> No
    </Button>
    <Button color='green' inverted>
      <Icon name='checkmark' /> Yes
    </Button>
  </Modal.Actions>
</Modal>

[levithomason]

PRs welcome!

[josie11]

Thank you for the quick response and explanation.

[josie11]

I've been playing around and trying to implement something like what you described above. I don't think the way i've written this is ideal, but I think its a start? I would love to work on implementing this feature. Here are the changes I made.

const modalJSX = (
      <ElementType {...rest} className={classes} style={{ marginTop }} ref={this.handleRef}>
        {Icon.create(closeIconName, { onClick: this.handleClose })}
        {children}
        {_.isNil(children) && header && ModalHeader.create(header)}
        {_.isNil(children) && content && ModalContent.create(content)}
        {_.isNil(children) && actions && ModalActions.create(actions.map((action, i) => {
          if (action.triggerClose) {
            const onClick = (callback) => (e) => {
              if (callback) callback(e, this.props)
              this.handleClose()
            }
            action.onClick = onClick(action.onClick)
          }
          return Button.create(action, { key: i })
        }))}
      </ElementType>
    )

The above would implement the below modal with shorthand props. A triggerClose can be passed to the shorthand action items that would allow clicking the buttons to close the modal (I would love to not have to use a open boolean in state all the time).

<Modal
  trigger={<Button>Basic Modal</Button>}
  basic
  size='small'
  header={{ icon: 'archive', content: 'Archive Old Messages' }}
  content='Your inbox is getting full, would you like us to enable automatic archiving of old messages?'
  actions={[
    { basic: true, color: 'red', inverted: true, icon: 'remove', content: 'No', triggerClose: true },
    { color: 'green', inverted: true, icon: 'checkmark', content: 'Yes', triggerClose: true },
  ]}
/>

[levithomason]

If you'd like to open a PR for this it'd be great. We can help out there with more visibility and comments.