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

down or only breakpoint still applies on upper breakpoint #11313

Closed
4 tasks done
ncoden opened this issue May 30, 2018 · 3 comments · Fixed by #11315
Closed
4 tasks done

down or only breakpoint still applies on upper breakpoint #11313

ncoden opened this issue May 30, 2018 · 3 comments · Fixed by #11315

Comments

@ncoden
Copy link
Contributor

ncoden commented May 30, 2018

As reported by @JasonMiller at #10978 (comment), the new breakpoint precision (0.0001em) introduce a new bug on all major browsers (Chrome, Firefox, Safari...).

What should happen?

For the following code:

$breakpoints: (
  small: 0,
  medium: 640px,
  large: 1024px,
);

.test {
    @include breakpoint(medium down) { ... }
}

Properties should be applied for medium and under, so for a resolution strictly inferior to 1024px.

What happens instead?

Due to the breakpoint 63.99999em being rounded to 64em by most browsers for their breakpoint calculations, properties are still applied on the large breakpoint for 1024px exactly.

This issue is related to:

Possible Solution

As seen in #10978, we need a subpixel precision to handle zoomed browsers. But browsers handle sub-pixel mediaqueries badly and a too high precision causes the issue described above. So we have to find the proper balance for a maximum support.

After some research across their own issues (twbs/bootstrap#19197), I took a look at the way Bootstrap handle this (cc @cvrebert @mdo). Here is the description of their Sass breakpoint-max function.

Maximum breakpoint width. Null for the largest (last) breakpoint.
The maximum value is calculated as the minimum of the next one less 0.02px
to work around the limitations of min- and max- prefixes and viewports with > fractional widths.
See https://www.w3.org/TR/mediaqueries-4/#mq-min-max
Uses 0.02px rather than 0.01px to work around a current rounding bug in Safari.
See https://bugs.webkit.org/show_bug.cgi?id=178261

So they use a precision of 0.02px, it would be something like 0.00125em for us, which is close to the 0.001 proposed by @JasonMiller. I would agree to adopt this constant, still we need to do some tests to ensure that the em-px conversion is correctly made by browsers.

Your Environment

  • Foundation version(s) used: develop
  • Browser(s) name and version(s): Tested on latest versions of Chrome, Firefox, Safari
  • Operating System and version (desktop or mobile): macOS 10.13.5 (beta)

Checklist (all required):

  • I have read and follow the CONTRIBUTING.md document.
  • There are no other issues similar to this one.
  • The issue title is descriptive.
  • The template is correctly filled.
@ncoden ncoden added this to the 6.5.0 milestone May 30, 2018
@ncoden ncoden self-assigned this May 30, 2018
ncoden added a commit to ncoden/foundation-sites that referenced this issue May 31, 2018
…n#11313

As seen in foundation#10978, we need a subpixel precision to handle zoomed browsers. But browsers handle sub-pixel mediaqueries badly and a too high precision causes the issue described above. So we have to find the proper balance for a maximum support.

Changes:
* set breakpoint max values to `0.00125em` (`0.02px`)  under the next breakpoint instead of `0.00001`.

Closes foundation#11313
@ncoden ncoden added the PR open label May 31, 2018
@millllllz
Copy link
Contributor

I would be tempted to defer to the bootstrap approach -- my assumption is that it will have already been thoroughly tested by that community if it is in production.

@ncoden
Copy link
Contributor Author

ncoden commented Jun 1, 2018

@JasonMiller I agree. I opened a PR with 0.02px for this reason. See #11315.

@millllllz
Copy link
Contributor

Awesome, thanks for the quick follow-up!

ncoden added a commit that referenced this issue Jun 1, 2018
…ion-11313

fix: lower breakpoint max value precision to avoid rounding #11313
ncoden added a commit to ncoden/foundation-sites that referenced this issue Jun 16, 2018
…h-precision-11313 for v6.5.0

798377d fix: lower breakpoint max value precision to avoid rounding foundation#11313
a36504c fix: revert mistakenly changed `$global-flexbox` setting in foundation#11315

Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants