Please can somebody explain me the usage and the checks of this code block?

if((!isBodyOverflowing&&isModalOverflowing&&!isRTL())||(isBodyOverflowing&&!isModalOverflowing&&isRTL())){this._element.style.paddingLeft=`${scrollbarWidth}px`}if((isBodyOverflowing&&!isModalOverflowing&&!isRTL())||(!isBodyOverflowing&&isModalOverflowing&&isRTL())){this._element.style.paddingRight=`${scrollbarWidth}px`}

Please can somebody explain me the usage and the checks of this code block?

if((!isBodyOverflowing&&isModalOverflowing&&!isRTL())||(isBodyOverflowing&&!isModalOverflowing&&isRTL())){this._element.style.paddingLeft=`${scrollbarWidth}px`}if((isBodyOverflowing&&!isModalOverflowing&&!isRTL())||(!isBodyOverflowing&&isModalOverflowing&&isRTL())){this._element.style.paddingRight=`${scrollbarWidth}px`}

I can be wrong about this but I think this got something to do with issues about modal shifting/jumping or not being centered with/without scrollbars on the body.

Found it:#14933

But that's not everything.. I decided to go down the rabbit hole.

So let's begin:

isBodyOverflowing is usinggetBoundingClientRect() to check if the body's width is less than the window'sinnerWidth ("the viewport width including the size of a rendered scroll bar (if any)"). My guess, this was done to see if there is a scrollbar on the document root or not. (I was a bit confused as we compare widths but yeah - the overflow means on the y-axis.)

isModalOverflowing is checking if the element'sscrollHeight ("the height of an element's content, including content not visible on the screen due to overflow.") is bigger than the document root'sclientHeight ("the viewport height excluding the size of a rendered scroll bar (if any)."). My guess, this was done to see if the modal is higher than the document root and would cause a scrollbar.

ThepaddingLeft is set to shift the modal dialog to the right and create a spacing (the size of a scrollbar) and vice versa. My guess, this was done to compensate for a scrollbar on the opposite side.

Thoughts:

So I checked the examples in our documentation and this doesn't seem to work properly. (This is true for older Bootstrap versions too. Maybe it did at some point tho.) The result, the dialog isn't centered properly. (At least in FF and Chrome on Windows. Safari 14 and Chrome on Android is fine tho.)
This is partly caused by the fact that when we meassurescrollHeight the dialog isn't yet visible so it's 0 andisModalOverflowing is always false.

AboutisRTL(). If I adddir="rtl" to the html tag the body's scrollbar is still on the right side. The modal's scrollbar on the left. 🤯 Not sure how that is handled when the OS is set rtl.

In general without those extra paddings it seems to work fine for me.

@alpadev what a lovely approach 🛩️

129 line this._adjustDialog() is pointless for sure, cause it hidden as you told

I am not sure for the other two usages (resize 304 / handleUpdate 211)
Did you test it on resize too?

And one more cherry, that is out of this scope: lines 405|407 & 414|419 seems redundant as elementoverflow-y isauto

I am not sure for the other two usages (resize 304 / handleUpdate 211)
Did you test it on resize too?

So I made a codepen for the case when the body doesn't overflow. SinceisModalOverflowing is false there is no padding added so it looks good. If I resizeisModalOverflowing becomes true and it's adding padding-left that makes it not centered. (Case!isBodyOverflowing && isModalOverflowing && !isRTL())

I tried it on the documentation page. There is padding-right added because the body does overflow (CaseisBodyOverflowing && !isModalOverflowing && !isRTL()).

There is no case for when both are overflowing or both are not overflowing.

Also the_adjustDialog is only adding paddings but never take them away. So at some point it's adding paddings making it uncentered and they will never go away until closed.

And one more cherry, that is out of this scope: lines 405|407 & 414|419 seems redundant as elementoverflow-y isauto

My guess this was added so in case when you click on a static backdrop and it would do this little animation, it's not causing any scrollbars on the backdrop if it reaches outside but not sure.

Likely related:#31845

@alpadev REALLY HELPFUL
I will not touch it on this pr but nice to have it in mind too
Issue created#33580

Any other comments over the changes?

My guess this was added so in case when you click on a static backdrop and it would do this little animation

That animation is transform scale, and transforming any element does not affect the layout of the real DOM element 🤔

My guess this was added so in case when you click on a static backdrop and it would do this little animation

That animation is transform scale, and transforming any element does not affect the layout of the real DOM element 🤔

@rohit2sharma95 Check for yourself 🙂https://jsfiddle.net/z1pb7c9w/

@GeoSot BrowserStack tests broke. Please temporarily push any PRs that are not made from an upstream branch so that you check if everything is fine before we merge :)

EDIT: NVM, it seems it was just a fluke. That being said, let's make sure BrowserStack tests pass for PRs coming from your fork.