Modal: resizing/transition issue on every current browser #2221

ghost · 2017-10-20T06:36:00Z

Hi,

we found a bug while using the semantic react dialog component.

--> Animiated Gif to show the effect <--

We can repoduce this error on
https://react.semantic-ui.com/modules/modal#modal-example-dimmer
but not with the jquery version.
So it seems a css problem base on adding / removing the scrolling class by react.

The main problem is, thats a edge case failure, that can only be repoduced if the modal dialog / the body tag will add / remove the scrolling css class and also the height and width are so chosen that the dialog doesnt fit the browser windows by missing some pixels in height and width.

For own dialogs with larger content this happens on normal sized pages, so many users of our site get this problem, when they have a special resolution.

Steps

Open the default modal dialog dimmer example with

chrome with a html / body size of width: 345px / height: 583px
firefox with a html / body size of width: 337px / height: 574px
EDGE with a html / body size of width: 332px / height: 576px

We cant repoduce the behavior with the device emulation mode on the chrome dev tools.

Expected Result

Stable Layout

Actual Result

The modal dialog is switching between scolling and not scrolling.
We think the recalculation by react adds the scrolling class to the body and also to the modal dialog and remove this again and add this again and so on.

Its toggling between (on the chrome example)

<body class="dimmable dimmed">
...
<div class="ui modal transition visible active" style="margin-top: -289px;">

and

<body class="dimmable dimmed scrolling">
...
<div class="ui scrolling modal transition visible active" style="margin-top: -294px;">

Also seen on the react component

<div class="ui modal transition visible active" style="margin-top: -289px;">

<div class="ui scrolling modal transition visible active" style="margin-top: -294px;">

The text was updated successfully, but these errors were encountered:

layershifter · 2017-10-20T17:54:17Z

Looks like duplicate of #1867.

ghost · 2017-10-25T15:01:12Z

Hi,
we spend some time to analyse the problem and compare the behaviour with the semantic ui (jquery).

We don't understand, why the scrolling css class was calculated based on component and window inner height.

const scrolling = height >= window.innerHeight

In semantic ui (jquery) its a setable property and was not calculated by the component itself. With a property scrolling:true/false the flickering will be gone. Its verfied by modifing react semantic ui.

Alternatively it would be an option to controll the scrolling from outside additionally.

Thx

levithomason · 2017-10-29T07:26:56Z

Thanks for the investigation. SUIR calculates a few properties for developer convenience. In this case, a Modal that extends taller than the screen should become scrollable. Modals that fit within the screen height do not need to be scrollable. This is always true and not applying the class in these cases would produce a "broken" UI for the user.

That said, the issue is that the Modal margin was not taken into account in the calculation. I've fixed this locally, but it needs to be cleaned up for a PR.

A fix would be to measure the height + border + margin of the modal without the scrolling className and if that fits the viewport, then the Modal is not scrolling.

Currently, the measurement doesn't differentiate between scrolling or non-scrolling modals. It also doesn't consider the border and margin, it only considers the height. The height changes when going from non-scrolling to scrolling, due to the inclusion of outer margin when scrolling.

zurez · 2017-11-03T13:12:47Z

@levithomason Any ETA on fix?

toolaugh · 2017-11-15T02:34:46Z

im too ,help me please

brianespinosa · 2017-11-16T19:30:08Z

@zurez @toolaugh PRs are welcome for any issues tagged as bug and help wanted.

levithomason · 2017-12-10T19:25:35Z

No ETA. Please use SO and Gitter for help requests. GitHub is reserved for performing work.

stale · 2018-03-10T19:46:15Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 30 days if no further activity occurs. Thank you for your contributions.

Glathrop · 2018-03-21T03:05:00Z

className="scrolling" is a workaround for this.

Tydax · 2018-03-23T17:06:20Z

@Glathrop Just discovered that, thanks for this workaround.
You can also change the margin-bottom property to auto of the Modal if you want to prevent the modal from taking the whole height if it doesn't need to.

layershifter · 2018-06-06T07:03:59Z

Can you check please the latest release with SUI 2.3?

bavicj · 2018-07-02T15:15:04Z

I have the same problem since I updated to versions 0.81.x. With 0.80.x everything was OK.
GIF example

layershifter · 2018-07-02T15:21:34Z

Please add a minimal repro case on codesandbox.

stale · 2019-01-06T15:11:49Z

There has been no activity in this thread for 180 days. While we care about every issue and we’d love to see this fixed, the core team’s time is limited so we have to focus our attention on the issues that are most pressing. Therefore, we will likely not be able to get to this one.

However, PRs for this issue will of course be accepted and welcome!

If there is no more activity in the next 180 days, this issue will be closed automatically for housekeeping. To prevent this, simply leave a reply here. Thanks!

grean · 2019-01-10T17:00:09Z

I personnaly fix this by adding those two lines on my modal component. The height value depending of your needs, for me it was 300..

<Modal
   ...
   className="scrolling"
   style={{maxHeight:'300px'}}
>...</Modal>

ktaras · 2019-03-13T16:55:39Z

Just adding class scrolling doesn't help.
It fixes flickering, but when the content is large there is no scrollbar and the content just overflows through the modal borders. Any other suggestions? Maybe scrolling should be applied in complex with some other adjustments?

It happens after upgrading to 0.81 and later versions. 0.80.2 is ok.

mgrivera · 2019-03-25T18:31:11Z

Hi, I must say I'm very new to react & semantic-ui as well.
So, version 0.80.2 is Ok, I assume for semantic-ui-react; I currently have version 0.86.0 and experiencing the issues mentioned here.
My question is: what version should we use for semantic-ui-css; I currently am using version 2.4.0. But what version should I use for Modal to behave correctly?
Thanks and bye ...

layershifter · 2019-06-23T10:06:47Z

I provided a fix in #3679, please try it, feedback there is much appreciated 🙌

levithomason added bug help wanted labels Oct 29, 2017

stale bot added the stale label Mar 10, 2018

stale bot removed the stale label Mar 21, 2018

layershifter changed the title ~~[Modal][Dimmer?] resizing/transition issue on every current browser~~ Modal: resizing/transition issue on every current browser Jul 10, 2018

layershifter mentioned this issue Jul 10, 2018

Modal: scrolling issue with top aligned #2891

Closed

layershifter mentioned this issue Oct 7, 2018

Modal flickers resizing when modal content is larger than page #3197

Closed

Blapi mentioned this issue Nov 20, 2018

Modal is not displayed properly #3289

Closed

stale bot added the stale label Jan 6, 2019

layershifter removed the stale label Jan 10, 2019

layershifter mentioned this issue Jun 23, 2019

fix(Modal): support IE11, fix scrolling glitches #3679

Merged

layershifter closed this as completed in #3679 Jul 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modal: resizing/transition issue on every current browser #2221

Modal: resizing/transition issue on every current browser #2221

ghost commented Oct 20, 2017

layershifter commented Oct 20, 2017

ghost commented Oct 25, 2017

levithomason commented Oct 29, 2017

zurez commented Nov 3, 2017

toolaugh commented Nov 15, 2017 •

edited

Loading

brianespinosa commented Nov 16, 2017

levithomason commented Dec 10, 2017

stale bot commented Mar 10, 2018

Glathrop commented Mar 21, 2018

Tydax commented Mar 23, 2018

layershifter commented Jun 6, 2018

bavicj commented Jul 2, 2018

layershifter commented Jul 2, 2018

stale bot commented Jan 6, 2019

grean commented Jan 10, 2019

ktaras commented Mar 13, 2019 •

edited

Loading

mgrivera commented Mar 25, 2019

layershifter commented Jun 23, 2019

Modal: resizing/transition issue on every current browser #2221

Modal: resizing/transition issue on every current browser #2221

Comments

ghost commented Oct 20, 2017

Steps

Expected Result

Actual Result

layershifter commented Oct 20, 2017

ghost commented Oct 25, 2017

levithomason commented Oct 29, 2017

zurez commented Nov 3, 2017

toolaugh commented Nov 15, 2017 • edited Loading

brianespinosa commented Nov 16, 2017

levithomason commented Dec 10, 2017

stale bot commented Mar 10, 2018

Glathrop commented Mar 21, 2018

Tydax commented Mar 23, 2018

layershifter commented Jun 6, 2018

bavicj commented Jul 2, 2018

layershifter commented Jul 2, 2018

stale bot commented Jan 6, 2019

grean commented Jan 10, 2019

ktaras commented Mar 13, 2019 • edited Loading

mgrivera commented Mar 25, 2019

layershifter commented Jun 23, 2019

toolaugh commented Nov 15, 2017 •

edited

Loading

ktaras commented Mar 13, 2019 •

edited

Loading