Modal + Position breaking autoFocus in v1

[maazadeeb]

trafficstars

We use a combination of Modal and Position components from react-overlays for creating our popovers. The autoFocus behavior is broken in version 1.0.0-beta.2, when using this combination.

Our component hierarchy is like this

<Modal>
  <Position>
    <Content />
  </Position>
</Modal>

This is a codesandbox using v0.8.3: https://codesandbox.io/s/jvyvmq4009. Here, the focus works. And this is using v1.0.0-beta.2: https://codesandbox.io/s/6l46wy8jnr, where it doesn't.

I believe the issue lies in the way Position is initially rendering its child, before actually positioning it. It uses visibility: hidden, which doesn't let the child receive focus. Changing it to opacity: 0 appears to solve the issue. In my v1 sandbox, I've just overridden Position.prototype.getNullState to reflect this change and it works. You can uncomment it in the sandbox and try it out.

Let me know if this is a valid bug and if so, whether the suggested fix is valid. I'd be happy to make a PR. 😄

[taion]

It doesn't immediately sound unreasonable. @jquense?

[jquense]

I that the idea for visibility was that you could still measure it? I'm happy to take a PR tho with RB using popper more directly we might drop the position component and point folks to popper or react-popper. Not sure tho yet

[maazadeeb]

Yup. I think it wants the DOM node to be rendered so that popper can do it's positioning magic.

I'll send a PR. For the test, should I write it in PositionSpec or ModalSpec or a new IntegrationSpec file?