Semantic-ui-react: V/H Centering of Modal Broken (After Upgrade to SUI 2.3)

[mcrawshaw]

Steps

View working modals after update to SUI 2.3

Expected Result

No change

Actual Result

Modal are now position in top-left (with some of the modal clipped)

Version

SUI 2.3 / RSUI 0.78.2

[brianespinosa]

Valid issue. I created a codesandbox example to complete the bug report form.

https://codesandbox.io/s/r08o8v4z3n

Community PRs are welcome.

[brianespinosa]

@mcrawshaw you may want to stick to 2.2.* for now since 2.3.0 just came out today. There are likely going to be some other issues that pop up too.

[brianespinosa]

Two things will need to happen to get this to work the way SUI does it in 2.3.

• [ ] Add a style to the Dimmer of {{display: 'fixed !important'}}
• [ ] Disable the negative top margin added inline to the .ui.modal

The question here is timing. This is actually a fairly significant change. I think some people will potentially have install bases of CSS that they will not be able to update/recompile soon. I am proposing what we do is add the ability to keep the existing functionality for modals for the time being if people want by adding a boolean flex prop to Modal. If flex is true, the above two changes would be enabled. If flex is false, then the old way would still be in place.

My thought here is that even though it would be a breaking change, I think the defaultProp for flex should be true so that you have to opt out. This is the way SUI is going with the modal display so in the long term, making people opt in to flex display is probably going to be bad. This at least gives people time to update.

@levithomason @layershifter thoughts?

[levithomason]

Thanks for the report @mcrawshaw and for the investigation @brianespinosa.

I would shy away from temporary public API updates. I'd support declaring that SUIR 0.78.x requires SUI 2.2, update the SUIR Modal for SUI 2.3 and ship it as SUIR 0.79.x.

Thoughts?

[levithomason]

Per #2550, let's go with the breaking change upgrade path.

[brianespinosa]

I would shy away from temporary public API updates. I'd support declaring that SUIR 0.78.x requires SUI 2.2, update the SUIR Modal for SUI 2.3 and ship it as SUIR 0.79.x.

Sounds good. Any PR on this will need to also update the docs to make not of this.

[jlukic]

Sorry about the incompatibilities, I wasn't aware at the time SUIR css wasn't pinned to a specific SUI release.

[levithomason]

Closing in favor of #2550

[chachaxw]

This bug is really bad.

[brianespinosa]

@chachaxw This is an open source project. Feel free to jump in and submit a PR to start solving some of the issues in #2550

I am going to lock this issue to reduce noise since everything is being handled over in #2550