Modal + Navbar + Styling causes body padding to be added where there was not previously a scrollbar #6037

nazrhyn · 2016-06-20T21:18:12Z

Hello, again.

Bug Description:

When there's a navbar, followed by a set of containers configured to control scrolling in a specific way and a modal opens, the body is shifted left by one scrollbar's width even though there was not previously a scrollbar in place for which to compensate. Here's a short GIF of this effect in action:

If the <nav> element is removed, the issue no longer occurs. If the classes of page-content-wrapper and wrapper are combined, the issue no longer occurs (and the inner scrollbar is no longer present). If the overflow: hidden on the body is removed, the outer scrollbar appears, and the compensation hides it without shifting anything.

I don't believe this is the same thing as #5749. This is the actual issue I alluded to in #6035. My intuition says that something about this combination of elements and styles is not being detected, and the calculation to determine the rendered width of the scrollbar is happening even though it is not needed, as the scrollbar was already hidden.

Hopefully I didn't miss another issue already tracking this!

Thanks, again, for your time, and this library.

Link to minimally-working plunker that reproduces the issue:

http://plnkr.co/edit/07fH45MYlEfTFobTkCvA

Version of Angular, UIBS, and Bootstrap:

Angular: 1.5.3
UIBS: 1.3.1
Bootstrap: 3.3.6

The URL for the plunker in the GIF isn't the same as the one in the link as I kept hitting CTRL+S out of habit, while I was writing the plunker, and it created like 920378402395 versions. I created a new one with a clean version history.

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

icfantv · 2016-06-20T21:35:02Z

Pretty sure this is a duplicate of (and explained by) #6035 and #5749. But please feel free to convince me otherwise.

nazrhyn · 2016-06-20T21:46:45Z

Not a duplicate of #6035 as I opened that to point out the issue in your site, and only alluded to the potential for this issue to be submitted in the future.

Reading through #5749, again, with comments:

The navbar shift a little to right when you open a modal.

That's not the issue I'm seeing. I'm seeing the absence of a scrollbar being replaced with the space that scrollbar would have taken up if it were there.

Several posts about padding.

In the box layout, the place at which padding is added is the <body> tag. This is more visible, in my case, because of (1) the fact that the inner container is scrolling and will always display that scrollbar and (2) because I've colored all of the containers differently. Adding padding to the navbar or those containers is just going to shift their contents over and not prevent those containers from, themselves, being shifted over by the padding of their parent.

Also, even if that were the problem, I want the freedom to set those containers to have whatever padding they should have based on our design/layout.

Plunker by @RobJacobs (http://plnkr.co/edit/yuRFho)

In this plunker, the body is scrolling, not a container within it. In my example, the body is explicitly told to hide all scrolling by overflow: hidden. In the "broken" plunker (http://plnkr.co/edit/ZBWY2E), the effect is on the contents of the nav, and not containers within the body.

icfantv · 2016-06-20T21:48:42Z

Sorry, I neglected to see who the reporter of #6035 was; sorry about that. I'll reopen and flag as needing some attention. Please note that this does not mean that we can/will do anything about it.

icfantv · 2016-06-20T21:49:16Z

@RobJacobs, can you tell if this is a related issue to #5749? Thanks.

nazrhyn · 2016-06-20T21:58:55Z

When we were tracking this down, I found the blocks of code that're doing width calculation and applying the body style. The scrollbarWidth function on $uibPosition is being triggered, into this block:

if (isBody) {
  if (angular.isUndefined(BODY_SCROLLBAR_WIDTH)) {
    var bodyElem = $document.find('body');
    bodyElem.addClass('uib-position-body-scrollbar-measure');
    BODY_SCROLLBAR_WIDTH = $window.innerWidth - bodyElem[0].clientWidth;
    BODY_SCROLLBAR_WIDTH = isFinite(BODY_SCROLLBAR_WIDTH) ? BODY_SCROLLBAR_WIDTH : 0;
    bodyElem.removeClass('uib-position-body-scrollbar-measure');
  }
  return BODY_SCROLLBAR_WIDTH;
}

That figures out the measurement, and then this block applies it, getting the result from $uibPosition.scrollbarPadding(appendToElement), inside $uibModalStack:

scrollbarPadding = $uibPosition.scrollbarPadding(appendToElement);
if (scrollbarPadding.heightOverflow && scrollbarPadding.scrollbarWidth) {
    appendToElement.css({paddingRight: scrollbarPadding.right + 'px'});
}

Without thinking about it too much, it just seems like it needs to somehow check to see whether the <body> element is already set to hide the scrollbar. In that case, the scrollbar width should be reported as zero, right? I can make a PR for this, if you'd like, and we could talk about the solution there.

RobJacobs · 2016-06-21T13:30:41Z

We could add a check in the scrollbarPadding calc to make sure the scrollParent does not have overflow & overflowX/Y set to hidden. If overflow or overflowX/Y is set to hidden, return false for the widthOverflow or heightOveflow values. The problem is the scrollParent method will never return an element with overflow 'hidden' unless the document element has overflow hidden.

The scrollParent() method traverses the parent elements until it finds one that has an overflow set to 'auto' or 'scroll' and returns the document element as a last resort. This mimics how the jQueryUI scrollParent method works.

The way this example is structured (body overflow: hidden) the scrollParent() method ends up returning the document element, so we would need to stop the scrollParent traverse at the body element. Not sure what sort of impact that will have in other implementations, but I'm inclined to follow how jQueryUI works.

The example is kind of odd with how the css is structured. Setting the wrapper class to 100vh then the page-content-wrapper to 100%-52px is redundant. Why not just eliminate the element with the wrapper class?

Here is an updated plunk that works as you expect with the following changes:

Set html & body styles to height: 100%; padding: 0; margin: 0; as that is the recommended approach when using the css calc
Removed 'wrapper' element
Set navbar margin-bottom: 0 rather than using margin-top: -20px margin on the wrapper class.

nazrhyn · 2016-06-21T13:32:42Z

Thanks @RobJacobs! I'll run that by our UI guy and see if we can use any of that to modify our layout to be more compatible. I'll be back with the response.

nazrhyn · 2016-06-21T14:50:16Z

In our pursuit of the "minimal example", we removed the reason behind the double wrappers (and some styles that support that reason).

The <body> directly contains the header/nav, which is a sibling of .wrapper. .wrapper contains the sidebar and the .page-content-wrapper, as siblings.

Here's an updated plunk with the new layout: http://plnkr.co/edit/1GcvJ9o9N4tgAxIRqPmU
I left some of the possibly extraneous styles just in case they have some impact on any suggestions you might have. I will say that this is a bit of a frankenstein layout, so there may just be better ways to do what our UI designers were originally trying to do.

RobJacobs · 2016-06-21T15:39:50Z

This all boils down to how your css is set up, the body element overflows the viewport, regardless if the scrollbars are showing or not. You need to set the overflow to occur on the element with the wrapper class and make sure the navbar and wrapper elements do not take up more than 100% height.

nazrhyn · 2016-06-21T17:06:44Z

While I definitely can accept that we may not be creating an optimal layout, the fact that the space is being allocated for something that wasn't there to begin with still seems like a problem, to me. Especially considering that the feature was added to, I assume, fix another issue (being able to scroll the modal window around, I would guess).

If it is desired that the evaluation code continues to conform to the process followed by jQueryUI, perhaps the more-intelligent decision making can be done at the site where the padding is applied.

RobJacobs · 2016-06-21T19:17:14Z

I'll create a PR to address this issue.

nazrhyn · 2016-06-21T19:19:35Z

Thanks @RobJacobs. Please let me know if there's anything else I can do.

icfantv closed this as completed Jun 20, 2016

icfantv added resolution: duplicate component: modal labels Jun 20, 2016

icfantv reopened this Jun 20, 2016

icfantv added needs: investigation and removed resolution: duplicate labels Jun 20, 2016

RobJacobs self-assigned this Jun 21, 2016

RobJacobs added type: bug and removed needs: investigation labels Jun 21, 2016

RobJacobs mentioned this issue Jun 21, 2016

fix(modal): check for overflow hidden #6041

Merged

RobJacobs closed this as completed in #6041 Jun 21, 2016

phazei mentioned this issue Nov 16, 2018

Revert "fix(modal): check for overflow hidden" Morgul/ui-bootstrap4#41

Merged

Morgul mentioned this issue Nov 16, 2018

Modal + Navbar + Styling causes body padding to be added where there was not previously a scrollbar Morgul/ui-bootstrap4#42

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modal + Navbar + Styling causes body padding to be added where there was not previously a scrollbar #6037

Modal + Navbar + Styling causes body padding to be added where there was not previously a scrollbar #6037

nazrhyn commented Jun 20, 2016 •

edited

Loading

icfantv commented Jun 20, 2016 •

edited

Loading

nazrhyn commented Jun 20, 2016

icfantv commented Jun 20, 2016

icfantv commented Jun 20, 2016

nazrhyn commented Jun 20, 2016 •

edited

Loading

RobJacobs commented Jun 21, 2016

nazrhyn commented Jun 21, 2016

nazrhyn commented Jun 21, 2016

RobJacobs commented Jun 21, 2016 •

edited

Loading

nazrhyn commented Jun 21, 2016 •

edited

Loading

RobJacobs commented Jun 21, 2016

nazrhyn commented Jun 21, 2016

Modal + Navbar + Styling causes body padding to be added where there was not previously a scrollbar #6037

Modal + Navbar + Styling causes body padding to be added where there was not previously a scrollbar #6037

Comments

nazrhyn commented Jun 20, 2016 • edited Loading

Bug Description:

Link to minimally-working plunker that reproduces the issue:

Version of Angular, UIBS, and Bootstrap:

icfantv commented Jun 20, 2016 • edited Loading

nazrhyn commented Jun 20, 2016

icfantv commented Jun 20, 2016

icfantv commented Jun 20, 2016

nazrhyn commented Jun 20, 2016 • edited Loading

RobJacobs commented Jun 21, 2016

nazrhyn commented Jun 21, 2016

nazrhyn commented Jun 21, 2016

RobJacobs commented Jun 21, 2016 • edited Loading

nazrhyn commented Jun 21, 2016 • edited Loading

RobJacobs commented Jun 21, 2016

nazrhyn commented Jun 21, 2016

nazrhyn commented Jun 20, 2016 •

edited

Loading

icfantv commented Jun 20, 2016 •

edited

Loading

nazrhyn commented Jun 20, 2016 •

edited

Loading

RobJacobs commented Jun 21, 2016 •

edited

Loading

nazrhyn commented Jun 21, 2016 •

edited

Loading