Fix modal container

[arnaud-morvan]

MapStore2 Modal try to set it's container to ".ms2 > div": https://github.com/geosolutions-it/MapStore2/blob/5e4e0f5320550a648cb55994ebeea718d9915fcd/web/client/components/misc/WithContainer.jsx#L16

Note that it does not give a default but overwrite the value given to the constructor !

In MapStore2 this selector does not return any element, so container fallback on "body" as in react-bootstrap: https://react-bootstrap.github.io/react-overlays/api/Modal#container

When using some extensions (GeoNetwork, TJS), which inject tags <div class="ms2">..., modal does not appear anymore because those tags have "display: none". IMHO we should always inject modal in body, for a correct backDrop.

This fix it by using the standard modal from react-bootstrap.

[arnaud-morvan]

Hi @dsuren1

What do you think about removing withContainer from MapStore2:

https://github.com/geosolutions-it/MapStore2/blob/5e4e0f5320550a648cb55994ebeea718d9915fcd/web/client/components/misc/WithContainer.jsx#L16

Because:

• it does not match any tag in MS2 without plugins, but create an issue with plugins
• it overwrites contructor property container instead of giving a default value
• we may use an id instead of a class for a modal container (a class, by definition, can match multiple elements, so behavior is not determinist)

Because here we also remove handleDialogClick which may have some useful effect:

https://github.com/geosolutions-it/MapStore2/blob/5e4e0f5320550a648cb55994ebeea718d9915fcd/web/client/components/misc/Modal.jsx#L24

But I do not see how to fix this use case without importing directly Modal from react-bootstrap

Or maybe we coult set it as a default value instead of an overwrite to avoid breaking something else.

Cheers

Arnaud

[dsuren1]

@arnaud-morvan
Thanks for suggestion.

When using some extensions (GeoNetwork, TJS), which inject tags <div class="ms2">..., modal does not appear anymore because those tags have "display: none". IMHO we should always inject modal in body, for a correct backDrop.

Kindly correct me if I'm wrong but I don't see the issue associated with this PR. From what I infer, it seems the using multiple extension sometimes causes Modal to not appear due to the way Modal in MS overrides container.

It's really strange where extensions has <div class="ms2"> has display: none.

What do you think about removing withContainer from MapStore2:

Unfortunately, it is not as simple as you might think. There are many downstream projects dependant on this component which has MS as it's base and Mapstore itself.

The Modal and WithContainer hoc is catered to suit MS and all dependant project such that it uses the themePrefix (project specific) or ms2 (Mapstore) else fallback to body as in bootstrap

Because here we also remove handleDialogClick which may have some useful effect

I believe you can use onHide to perform any side effects. Reason for overriding is here

But I do not see how to fix this use case without importing directly Modal from react-bootstrap

I can approve this PR as the code looks fine but however I would like to understand the cause better (more detailed steps to reproduce) to see if it's reproducible and based on the outcome I can open a ticket in MS and take it forward accordingly.
Let me know what you think.