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

Modal as Form returns error, and Modal no longer centers on screen #1012

Closed
CaryLandholt opened this issue Dec 10, 2016 · 12 comments
Closed
Labels

Comments

@CaryLandholt
Copy link

Steps

  1. <Modal as={Form}... />

Expected Result

Vertically centered Modal without exceptions

Actual Result

Uncaught TypeError: _this._modalNode.getBoundingClientRect is not a function

and Modal is positioned below the center line

Can do <Form as={Modal}.../> but lose the ability to press the Enter key to submit the form

Version

0.62.0

@levithomason
Copy link
Member

levithomason commented Dec 10, 2016

This is because the modalNode is set via a ref and expects the ElementType (as value) to be a DOM component (div). Hence, the ref returns a DOM node which has a getBoundingClientRect method.

However, when rendering as={Form} the ref will return an instance of Form since it is a composite component. A Form instance, of course, has no getBoundingClientRect method.

This is actually the same problem we have in #659.

We need a way to reliably get the DOM node of both DOM components (e.g. a div) and/or composite components (e.g. a Form). The sticking point here is that findDOMNode() is being highly discouraged and has hopes of being deprecated, so we're left to clunky workarounds with ref callbacks, wrapping components, and proprietary props APIs for handling component references.

We can at least isolate the job finding DOM nodes to a transparent wrapping element. I made a very rough example here: http://www.react.run/HJa1BVYQl/19 that shows we can find the DOM node for any functional/class DOM/composite component.

Will have to sleep on this, but likely we'll need to add some utility for this.

@CaryLandholt
Copy link
Author

Thanks for the detailed explanation, @levithomason. Very helpful.
As you know, the more fantastic a library, the more we want from it. :)

Thanks again!

@bakatrouble
Copy link

I've managed to put a form into modal this way:

<Modal>
    <Modal.Content>
        <Form id="myform">
            ...
        </Form>
    </Modal.Content>
    <Modal.Actions>
        <Button type="submit" form="myform">Submit</Button>
    </Modal.Actions>
</Modal>

@CaryLandholt
Copy link
Author

Thanks for the hack, @bakatrouble. But won't work in our case.

@levithomason
Copy link
Member

@CaryLandholt we still want to fix this, though, does the basic Modal layout solve your use case as a workaround for now?

image

@levithomason
Copy link
Member

You can also remove the dimmer in combination with a basic Modal to make it appear as if only the Form is in the Modal:

<Modal basic dimmer={false} />

http://g.recordit.co/o9fdY5KKr2.gif

@CaryLandholt
Copy link
Author

Our need is to use a Form within a Modal. That is, to take advantage of Modal.Content, Modal.Actions, and form submit via Enter key.
The form fields are to be within Modal.Content and submit Button within Modal.Actions to achieve the desired UI.

In order to support submitting via Enter key, the submit Button must be within the Form and the onSubmit must be on the Form element.

@bakatrouble's approach has the submit button outside of the Form but used the form attribute on the submit Button.

We also tried the reverse - to surround the Modal by the Form which resulted in a messed up UI. For example:

<Form onSubmit={submit}>
  <Modal>
    <Modal.Content>...</Modal.Content>
    <Modal.Actions>
      <Button type="submit">Submit</Button>
    </Modal.Actions>
  </Modal>
</Form>

@levithomason
Copy link
Member

In order to support submitting via Enter key, the submit Button must be within the Form and the onSubmit must be on the Form element.

As of HTML5, you do not need to have the button inside the form. You can do this with the form button attribute.

@CaryLandholt
Copy link
Author

Right, this is what @bakatrouble suggested.

However, we are constructing the actions separate from the content and wanted to avoid needing to pass the form name down. We also have other forms on the same page so the form names would need to be unique.

@levithomason
Copy link
Member

I see. The other option here is to hoist your form state and use a callback to submit the data.

@CaryLandholt
Copy link
Author

Yeah - was hoping to avoid workarounds and hacks to just use a form within a modal
I appreciate the effort and suggestions, however.
The library is absolutely fantastic and has sped up our velocity considerably.

@levithomason
Copy link
Member

Closing for housekeeping.

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

No branches or pull requests

3 participants