Windows 7
Firefox 61.0.2
Why modal script adds margin-right and padding-right for .sticky-top?
We just add padding-right
to the body to center horizontally our moral when there is no scrollbar
Ok I made this CodePen to demonstrate the issue: https://codepen.io/Johann-S/pen/KxKpOR
Fixed it with this
<script type="text/javascript">
$('body').on('show.bs.modal', function () {
$('.sticky-top').css('margin-left', '-=0px');
});
$('body').on('hidden.bs.modal', function () {
$('.sticky-top').css('margin-left', 'auto');
});
</script>
@mdo I'm a bit lost here, I understand the issue, I know how to fix it but I'm not sure what I'll do is the right fix.
My fix will not add margin-right
or padding-right
if the element have our container class (.container
), but I'm not sure if it's the right things to do 馃
My fix replace those lines :
https://github.com/twbs/bootstrap/blob/v4-dev/js/src/modal.js#L432-L433
By:
const fixedContent = [].slice.call(document.querySelectorAll(Selector.FIXED_CONTENT))
.filter((elem) => !elem.classList.contains('container'))
const stickyContent = [].slice.call(document.querySelectorAll(Selector.STICKY_CONTENT))
.filter((elem) => !elem.classList.contains('container'))
/CC @XhmikosR if you have any ideas, @zalog because you worked on that too
Now you can see the problem on the site:
http://agnevy6d.beget.tech
Click "袟袗袣袗袟袗孝鞋 袟袙袨袧袨袣" under phone numbers.
Pay attention to file "main.css", line 38.
Empirically, I realized that setting these properties solves the problem.
If uncomment line 38 (and reload the page), there will be no problem.
I too see the issue for sure. @Johann-S's fix seems OK, do we have a PR for this?
Nope because I'm not sure my fix is correct and won't create side effects
Why do you even go for margin and padding? body
gets padding-right when modal is activated so everything else should be contained within the body. With v.4.2.1 when body
gets class modal-open
and style i.e. padding-right: 17px
added, fixed elements get extra padding-right and sticky elements get extra padding-right and negative margin-right. That simply looks bad:
FIX:
Both padding and margin should be left as they are!
Sticky element will be contained by the parent's width so nothing needs changing here (only when browser doesn't do sticky).
Fixed element is contained by the viewport's width, but you can position it properly with right: 17px
.
To be precise it's right: <whatever is added to body as padding-right>
.
any update?
This is still not fixed so this is my solution:
$('body').on('show.bs.modal', function () {
$('.sticky-top').addClass("fixModal");
});
$('body').on('hidden.bs.modal', function () {
$('.sticky-top').removeClass("fixModal");
});
And the css class .fixModal
.fixModal {
padding-right: 0 !important;
margin-right: 0 !important;
}
@XhmikosR I still have problems even with home solution with this issue. It will be solved on the next release?
As you can see, the issue is still open and it even has the label help wanted
added, so I'd say no :P
Any PRs which might fix this are welcome as usual :)
I just ran into this problem when migrating from v3 to v4. Looks like it might have been fixed in #30621, but it's still waiting on a fix for a test conflict issue
Having the same issue. Steps to reproduce in my web app:
UPD. I have fixed my problem applying patch by @muhamadamin1992 https://github.com/twbs/bootstrap/pull/30621/commits/1c9f4a456ef55f0a499a45658b28a1865b297235
$(stickyContent).each(function (index, element) {
if (window.innerWidth <= element.clientWidth + _this10._scrollbarWidth) { // this is added line
var actualMargin = element.style.marginRight;
var calculatedMargin = $(element).css('margin-right');
$(element).data('margin-right', actualMargin).css('margin-right', parseFloat(calculatedMargin) - _this10._scrollbarWidth + "px");
}
}); // Adjust body padding
Most helpful comment
any update?