Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Navbar expand breaks at 767px #24137

Closed
mhverbakel opened this issue Sep 27, 2017 · 27 comments
Closed

Navbar expand breaks at 767px #24137

mhverbakel opened this issue Sep 27, 2017 · 27 comments
Labels

Comments

@mhverbakel
Copy link

mhverbakel commented Sep 27, 2017

The layout breaks at exactly 767px wide screens. The easiest example is the documentation.

  1. Go to https://getbootstrap.com/docs/3.3/css/#responsive-utilities-tests in Chrome or Firefox.
  2. Use the emulator to limit your screen width to 766 pixels (height is not relevant).
  3. Scroll to the green checks that show which sizes are shown and hidden (it's under Test cases).
  4. Check that the page works as intended (e.g. there are at least some green checks visible).
  5. Increase the screen width to 767 pixels.
  6. Check that the page breaks (e.g. there are no more checks for visible or hidden).
  7. Increase the screen width to 768 pixels.
  8. Check that the page works as intended (e.g. there are green checks visible again).

My OS is Windows 10, I've been able to reproduce this on different machines using Chrome and Firefox (I was unable to reproduce this on MS Edge).

I think this is a browser issue, but it happens in multiple browsers, so this might be another issue. (I narrowed it down to 'media queries under 768 are exclusive, media queries above 767 are inclusive', and I have a minimal html/css test page if needed). Is there a way Bootstrap can work around this issue?

In case you cannot reproduce step 6, this is what it looks like on my PC:
image

Possibly related issues: #18845, #15787

@Johann-S
Copy link
Member

Bootstrap 3 is essentially in maintenance mode as we focus on working towards a stable v4. As such, we're only accepting changes to v3's code on a case-by-case basis, usually only for critical bug fixes or docs improvements.

@mhverbakel
Copy link
Author

Okay, then let me provide an example of the same issue in v4.

<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="utf-8">
  <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no">
  <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/4.0.0-beta/css/bootstrap.min.css" integrity="sha384-/Y6pD6FV/Vv2HJnA6t+vslU6fwYXjCFtcEpHbNJ0lyAFsXTsjBbfaDjzALeQsN6M"
    crossorigin="anonymous">
</head>

<body>
  <!-- START OF THE IMPORTANT CODE -->
  <nav class="navbar navbar-expand-md navbar-light bg-light">
    <div class="container">
      <a class="navlink">Always visible</a>
      <a class="d-none d-md-block navlink">Sometimes visible</a>
    </div>
  </nav>
  <!-- END OF THE IMPORTANT CODE -->
</body>

</html>

If you make the screen 766 or 767 pixels wide, the layout should be equal. If you make it 768 (or bigger), the layout must be switched. However, because of my issue, the navbar already gets padding at 767 pixels, although it should get it at 768 pixels.

The line in the CSS that causes this is this line:

@media (max-width: 767px) {
  .navbar-expand-md > .container,
  .navbar-expand-md > .container-fluid {
    padding-right: 0;
    padding-left: 0;
  }
}

Note, moving this number to 768 is not a solution.

@Johann-S
Copy link
Member

Bug reports must include a live demo of the problem. Per our contributing guidelines, please create a reduced test case via JS Bin or CodePen and report back with your link, Bootstrap version, and specific browser and OS details.

@mhverbakel
Copy link
Author

Bootstrap version: 4.0.0-beta by MaxCDN
Browser: Chrome 61.0.3163.100 (64-bits) / Firefox 55.0.3 (64-bits)
OS: Windows 10 (if you want, I can provide the list of installed updates or something like that)

Steps to reproduce:

  1. Open https://output.jsbin.com/qigutefeti
  2. Use the responsive testing utility from the inspector to change the width to 766, 767 and 768 pixels. Watch the padding on the "Always visible" menu item.

@XhmikosR
Copy link
Member

@andresgalante: can you reproduce? It is possible the navbar-expand is using the wrong mixin thus being off by 1 pixel.

@XhmikosR XhmikosR changed the title v3.3 responsive utilities break at 767px Navbar expand breaks at 767px Sep 28, 2017
@twbs twbs deleted a comment from mhverbakel Sep 28, 2017
@twbs twbs deleted a comment from mhverbakel Sep 28, 2017
@andresgalante
Copy link
Collaborator

andresgalante commented Sep 28, 2017

Hi @XhmikosR I've tested this one. This kind of issues on 1px differences always happen when we use max-width instead of min-width. Since bootstrap is mobile first it should always use min-width.

To be honest I was not able to reproduce the issue yet, I'll have a better look and see what I can find out.

@mhverbakel
Copy link
Author

Hello @andresgalante,

Second example on your link: "When the container is within your navbar, its horizontal padding is removed at breakpoints...", so the container should go in the navbar.

The only reason I put it there, is to show that max-width: 768px is broken for screens with width 767px. Not only in bootstrap v4, also in boostrap v3.3.

I have an HTML page which shows the issue more clearly:

<html>
<head>
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">
<style>
body {
    background-color: black;
    color: white;
}

@media screen and (max-width: 769px) {
    body { background-color: blue; }
}

@media screen and (max-width: 768px) {
    body { background-color: red; }
}

@media screen and (max-width: 767px) {
    body { background-color: orange; }
}

@media screen and (max-width: 766px) {
    body { background-color: green; }
}

@media screen and (max-width: 765px) {
    body { background-color: black; }
}
</style>
</head>
<body>
<p>Test the range 765px - 770px width.</p>
</body>
</html>

As a matter of fact, I couldn't reproduce this on my current laptop (Windows 7), and I couldn't reproduce it on another Windows 10 laptop. So that's 2 devices with the issue, and 2 devices without the issue.

Until the root cause of this issue is found, I think media queries for maximal 767px should be avoided.

@andresgalante
Copy link
Collaborator

@mhverbakel you are right, I missed the second paragraph, sorry :(

I am testing this on out

@andresgalante
Copy link
Collaborator

andresgalante commented Sep 28, 2017

Ok, it's really hard to debug something I can't see but as @mhverbakel mentions, we have our grid media queries set up with min-width and the breakpoints of .nav-expand set up in max-width here:
https://github.com/twbs/bootstrap/blob/v4-dev/scss/_navbar.scss#L149

There is a 1px point, when things are not max nor min, and strange things happen.

@mhverbakel @XhmikosR If it's ok with you guys, I can try work on a version of nav-bar that uses only min-width. Although I can't reproduce the bug, the new version will be as functional as the one we have now. What do you guys think?

@mhverbakel
Copy link
Author

Hello @andresgalante,

I believe the setup for reproducing this is a Windows 10 device with 125% display scaling.

Commenting on your idea to migrate to min-width: I will check if this would resolve the issue (because I haven't tested the edge case for min-width yet).

Kind regards (and thank you in advance),
Martijn Verbakel

@andresgalante
Copy link
Collaborator

So... I've took a deeper look at this one. Although using max-width is not ideal to do mobile first, I don't think that the logic of the navbar is wrong.

I've been pushing it to see any bug and I couldn't, I guess I need a windows machine. I'll leave the min-width change on my todo list to explore a better way to implement the navbar, but for now I think we should be ok with what we have.

Thanks a lot Martijn for bringing this up, it was fun to digg into this part of the codebase.

@mhverbakel
Copy link
Author

I think I've narrowed this down to a browser bug in Chrome and Firefox on Windows 10 with 125% scaling of the screen. The next part of the question: should bootstrap work around this?

@XhmikosR
Copy link
Member

XhmikosR commented Oct 1, 2017

I think it's a browser bug. We should try and find tickets and add those to the wall of bugs.

@patrickhlauke
Copy link
Member

not looked deeply at this, but sounds like this is related to min-, max- and their shortcomings when it comes to fractional values? https://github.com/w3c/csswg-drafts/pull/1083/files

@mdo mdo added the browser-bug label Oct 2, 2017
@mhverbakel
Copy link
Author

mhverbakel commented Oct 3, 2017 via email

@patrickhlauke
Copy link
Member

patrickhlauke commented Oct 3, 2017

when working with a zoom of 125% (or dpr of 1.25), you never get an exact viewport width of 767. you'll be in the sub-pixel value range, where the fact that min- and max- have that 1px no-man's land area will kick in, no?

@mdo
Copy link
Member

mdo commented Oct 3, 2017

Closing as a won't fix if this is about page zooming. Please comment and we can re-open if we need to address anything else.

@mdo mdo closed this as completed Oct 3, 2017
@mhverbakel
Copy link
Author

This is not about page zooming, this is (maybe) about having a scaled desktop (which is default in Windows 10).

@patrickhlauke
Copy link
Member

@mdo note we won’t be able to just close issues on zoom grounds, as dpr will have similar/same effect and that’s not within user’s control

@twbs twbs deleted a comment from patrickhlauke Oct 4, 2017
@mhverbakel
Copy link
Author

mhverbakel commented Oct 4, 2017

@patrickhlauke The CSS spec speaks of inclusive ranges for those values, but the browser apparently treats it as an exclusive range: If I test with simple CSS selectors, every 4th pixel works as expected, and in between it matches against max-width: $width + 1.

I also validated that this is only the case in an 1.25dppx environment.

EDIT: In an 1.50 dppx environment, every second pixel works as expected, and every other pixel matches against the wrong width + 1 rule.

@patrickhlauke
Copy link
Member

patrickhlauke commented Oct 8, 2017

The CSS spec speaks of inclusive ranges for those values, but the browser apparently treats it as an exclusive range

the "inclusive" part means that they include that exact value, not that they also include any decimal/fractional value. so interpret max-width as meaning <=. 767 is <= 767, but 767.25, 767.50, 767.75 are not <= 767 [edited to use 767 rather than 768]

@mhverbakel
Copy link
Author

That sounds like the issue being shown here. The solution for this is to ban either max-width or min-width. I believe v4 almost always relies on min-width, so this is just something that needs to be checked.

@patrickhlauke
Copy link
Member

@mhverbakel no, "banning" use of max-width or min-width is not the solution. again, see https://github.com/w3c/csswg-drafts/pull/1083/files

only solution, while browsers still don't support range context queries, is to modify the values use for the min/max comparisons to include higher precision (e.g. checking for max-width: 767.99 rather than max-width:767 or similar)

@mhverbakel
Copy link
Author

I've confirmed that checking for max-width: 767.99 indeed fixes the issues. Could this also be a patch deployed for v3 please?

@patrickhlauke
Copy link
Member

See #24299. As for v3, that's now in maintenance mode, so there are unfortunately no plans to roll out these sorts of changes there.

@MatinT-SA
Copy link

guys. i have the same issue and i used both min-width and max-width and it's not working properly. i'm using bootstrap 3.3.7 version and i have exactly the same issue of @mhverbakel when i scale down to 767px.
it's ok at 768px and also 766px but exactly at "767px", suddenly the whole page breaks and unfortunately there is nothing i can do about it. does anybody have the same issue and what are the possible ways to solve this?
thanks

@patrickhlauke
Copy link
Member

i'd suggest either updating to a more recent version of bootstrap, or applying the suggested fix above manually to your old copy of bootstrap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants
@mdo @XhmikosR @patrickhlauke @Johann-S @andresgalante @mhverbakel @MatinT-SA and others