jquense / react-bootstrap-modal

React port of jschr's better bootstrap modals
Other
89 stars 51 forks source link

fix access to ModalManager instance #29

Closed atis-- closed 7 years ago

atis-- commented 7 years ago

a way to fix https://github.com/jquense/react-bootstrap-modal/issues/28

Pisi-Deff commented 7 years ago

I'm glad someone has taken the time to work on this :)

Some thoughts:

What's the motivation behind removing the check for an existing getZIndex on line 95?

getZIndex is a module-wide variable and will be overwritten by each successive mounted Modal. The one declared by the Modal mounted last will then be used for all Modals.

If my assumption is correct that this.manager/ref.props.manager will refer to the same ModalManager instance for all Modals, then it might be better to:

atis-- commented 7 years ago

I agree on all points. Please see the updated version.

atis-- commented 7 years ago

Ok, even better. Please check if the new change looks alright to you.

jquense commented 7 years ago

thanks!