[Modal] Modal bottom margin bug on Firefox

[jjylha]

Bug Report

Steps to reproduce

1. Go to https://fomantic-ui.com/modules/modal.html#scrolling-modal
2. Open "Scrolling Modal" with Firefox and scroll all the way down
3. Modal and the bottom of the screen do not have the same space as other browsers

Expected result

Modals should have same margins on all browsers

Actual result

Firefox renders margin incorrectly

Testcase

https://fomantic-ui.com/modules/modal.html#scrolling-modal

Version

2.7.8

[TomerPacific]

@lubber-de , Can I try to solve this issue? Anything specific I need to be aware of?

[lubber-de]

@TomerPacific Sure, go ahead 👍

[TomerPacific]

@lubber-de , I might be misunderstanding the issue as I am not seeing any difference. I am trying to reproduce it using the latest Firefox version and this is what I am seeing:

Chrome

Firefox

I read the steps to reproduce several times and tried different variations to better understand the issue. The pictures above represent what I tried doing which was to go to the link and pressing the Run Code under Scrolling Modal. I also tried just scrolling all the way down on the page and didn't see any differences.

Let me know how to proceed.

[lubber-de]

🤔 I am going to reproduce using browserstack again. Maybe it was occurring on macos and Firefox only

[lubber-de]

OK, results are here:
MacOS, Firefox 69 (and also 70 beta):

• Modal does not have any bottom margin at all
  

MacOS, Chrome 77:

• Modal does have the bottom margin
  

Windows 10, Firefox 69 (and also 70 beta)

• Again, no bottom margin
  

Now it gets interesting:
Windows 10, Edge 18

• Also no bottom margin
  

After comparing all of this, i wonder, if the slight difference is really worth the whole investigation.
Anway this is now a "non-chrome" issue

[TomerPacific]

@lubber-de , so what would be your optimal solution? Do you want all browsers to have the bottom margin or not?

[lubber-de]

@TomerPacific I think, the most optimal solution would be to have the bottom margin on every browser

[TomerPacific]

@lubber-de , Ok. I will work on making it similar for all browsers. As a side note, I am operating a Windows machine, so it will be hard for me to make sure things are working correctly for Mac.

[TomerPacific]

@lubber-de , from some investigation, I have learned that this issue does not reside within the Fomantic-UI repository, but rather the Fomantic-UI-Docs repository.

Just to make sure I am on the right path, it is this file and it's associated stylesheet I should be working with. Correct?

[lubber-de]

from some investigation, I have learned that this issue does not reside within the Fomantic-UI repository, but rather the Fomantic-UI-Docs repository.

😨 Never thought about that...

Just to make sure I am on the right path, it is this file and it's associated stylesheet I should be working with. Correct?

Yes

[TomerPacific]

@lubber-de , thanks.
Do you want this issue to be moved there or should we handle things here?

[lubber-de]

@TomerPacific If it's really related to the docs repo having some breaking additional CSS there, then we should move it over to the Docs Repo, i think

[TomerPacific]

@lubber-de , do you want me to open a new issue there or are you going to move this issue over there? I have no powers here 🎩

[lubber-de]

@TomerPacific Moved issue from Fomantic-UI repo because it seems it's a bug in the docs rather than in FUI itself 😉

[TomerPacific]

@lubber-de , sorry to be a hassle, but just noticed that the stylesheet that is being reference is found inside the Fomantic-UI repository.
Why? I am sure you have a good reason for this.

If I want to simulate my fix, how would one do so?

[lubber-de]

@TomerPacific What stylesheet are you talking about? Everything should be available in the docs repo unless you really have to change something in the FUI repo again (but then it isn't a docs issue...) From what you were telling us, i guess the issue is because of some additional CSS which only exists in the docs?
The docs of course assume to have a complete dist of FUI.

To simulate your fix, you can

• follow the steps in https://github.com/fomantic/Fomantic-UI-Docs/blob/master/README.md to compile the docs
  or
• checkout the gh-pages branch of the docs-repo (where also a full dist of FUI is available), point it to the root of some local webserver and experiment with your changes there before moving over your changes to the "raw" files in the develop branch

[jjylha]

The bug should be in FUI CSS (Since SUI flex modals I would guess) and not in the Docs CSS. We have it also inside our build application (No Docs CSS included?) and it is messing up some custom positioned stuff inside the modals depending on the browser.

I do still get this bug on Windows Firefox Quantum latest version (69.0.2 64) but not with Windows latest Chrome version (77.0.3865.90). No margin on Firefox but margin on Chrome.

[lubber-de]

Ok, i'll move this issue back to the FUI repo 🙄 😉

[TomerPacific]

@lubber-de , after going over the modal.less file and inspecting the elements, it seems that if I add a style rule of margin-bottom, it should sort out the issue over all browsers.

I wanted to ask two questions:

1. Do I test this solution in the same fashion described above? (since this issue got moved back to FUI)
2. I added margin-bottom: 5% to the rule .ui.modal. Is this correct?

[lubber-de]

@TomerPacific You can use this jsfiddle as a base for testing https://jsfiddle.net/5d34mfgb/

[lubber-de]

I added margin-bottom: 5% to the rule .ui.modal. Is this correct?

You should use rem unit instead of percentage

[TomerPacific]

@lubber-de,
after tinkering around for a while using the jsfiddle you provided, it seems like FIrefox and Edge do not respect the rule I have set up.

body > div.ui.dimmer.modals.page.transition.visible.active > div { margin-bottom: 5rem; }

I have read countless SO questions and articles and some suggested using padding (which works, but does not provide the desired solution).

When trying a vanilla example (without all the CSS), the rule does work, so I am afraid there is a conflicting rule(s) preventing the margin to take place. I'll try to investigate it some more.

UPDATE
As this issue requires in depth knowledge of all the CSS rules, I propose that someone with more familiarity takes this issue. I am also in the state of mind that it may be best to eradicate the bottom margin, which will require less effort, but that is my own opinion.

[lubber-de]

@TomerPacific @jjylha
I finally found a way around this and fixed it by #1090
Please try https://jsfiddle.net/zt0cfo6d/1/

[jjylha]

Seems to work than you :)

Just one thing. :not(.fullscreen) should probably be :not(.overlay.fullscreen)? I mean fullscreen modal needs that consistent margin too?

[jjylha]

Well :not(.overlay.fullscreen) doen not work. Maybe just :not(.overlay) ?

[lubber-de]

@jjylha Good catch! Changed the PR accordingly 😉