Fomantic-ui: [Modal] Modal bottom margin bug on Firefox
Bug Report
Steps to reproduce
- Go to https://fomantic-ui.com/modules/modal.html#scrolling-modal
- Open "Scrolling Modal" with Firefox and scroll all the way down
- 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
jjylha
All 26 comments
@lubber-de , Can I try to solve this issue? Anything specific I need to be aware of?
TomerPacific
on 5 Oct 2019
@TomerPacific Sure, go ahead 👍
lubber-de
on 5 Oct 2019
@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.
TomerPacific
on 5 Oct 2019
:thinking: I am going to reproduce using browserstack again. Maybe it was occurring on macos and Firefox only
lubber-de
on 5 Oct 2019
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
lubber-de
on 5 Oct 2019
@lubber-de , so what would be your optimal solution? Do you want all browsers to have the bottom margin or not?
TomerPacific
on 5 Oct 2019
@TomerPacific I think, the most optimal solution would be to have the bottom margin on every browser
lubber-de
on 5 Oct 2019
@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
on 5 Oct 2019
@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?
TomerPacific
on 6 Oct 2019
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
lubber-de
on 6 Oct 2019
@lubber-de , thanks.
Do you want this issue to be moved there or should we handle things here?
TomerPacific
on 6 Oct 2019
@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
lubber-de
on 6 Oct 2019
@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 🎩
TomerPacific
on 6 Oct 2019
@TomerPacific Moved issue from Fomantic-UI repo because it seems it's a bug in the docs rather than in FUI itself 😉
lubber-de
on 6 Oct 2019
@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?
TomerPacific
on 6 Oct 2019
@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
lubber-de
on 6 Oct 2019
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.
jjylha
on 8 Oct 2019
Ok, i'll move this issue back to the FUI repo 🙄 😉
lubber-de
on 8 Oct 2019
@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:
- Do I test this solution in the same fashion described above? (since this issue got moved back to FUI)
- I added
margin-bottom: 5%to the rule.ui.modal.Is this correct?
TomerPacific
on 8 Oct 2019
@TomerPacific You can use this jsfiddle as a base for testing https://jsfiddle.net/5d34mfgb/
lubber-de
on 8 Oct 2019
I added margin-bottom: 5% to the rule .ui.modal. Is this correct?
You should use rem unit instead of percentage
lubber-de
on 8 Oct 2019
@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.
TomerPacific
on 8 Oct 2019
@TomerPacific @jjylha
I finally found a way around this and fixed it by #1090
Please try https://jsfiddle.net/zt0cfo6d/1/
lubber-de
on 12 Oct 2019
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
on 14 Oct 2019
Well :not(.overlay.fullscreen) doen not work. Maybe just :not(.overlay) ?
jjylha
on 14 Oct 2019
@jjylha Good catch! Changed the PR accordingly :wink:
lubber-de
on 14 Oct 2019
Related issues
ocharles
·
4Comments
GammaGames
·
4Comments
PhilippGrashoff
·
3Comments
davekc
·
4Comments
loweoj
·
3Comments