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

Change integer pixels by .02px rather than 1px #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

andersk
Copy link

@andersk andersk commented Feb 4, 2021

According to the Bootstrap developers, using .02px rather than .01px should be sufficient to work around the Safari rounding bug.

Fixes #19.

andersk added a commit to andersk/zulip that referenced this pull request Feb 4, 2021
On a high-DPI display or with a non-default zoom level, the browser
viewport may have a width strictly between md_max = 767px and md_min =
768px.  Use only the *_min bounds for consistency.

This requires queries with strict inequalities to express upper
bounds (width < md_min).  Fortunately, that functionality is provided
by range context queries.  Unfortunately, those are not supported in
all browsers.  Fortunately, we can compile them away using
postcss-media-minmax.  Unfortunately, postcss-media-minmax currently
subtracts 1px for strict inequalities anyway to work around a Safari
rounding bug.  Fortunately, 0.02px should be sufficient for that, so I
submitted a PR:

postcss/postcss-media-minmax#28

Signed-off-by: Anders Kaseorg <anders@zulip.com>
@yisibl
Copy link
Member

yisibl commented Feb 4, 2021

Has this been tested in Safari? 1px will have bugs?

@andersk
Copy link
Author

andersk commented Feb 4, 2021

I don’t have a Mac myself, but twbs/bootstrap#25177 has been tested extensively in Safari, having been part of Bootstrap for the last three years.

I’m not sure how to interpret “1px will have bugs?”, but if the question is whether this solves a real problem with the existing 1px offset, the answer is yes. On high-DPI screens or at non-default zoom levels, Firefox and Chrome do in practice evaluate media queries at fractional px sizes, so that it’s possible for neither @media (width <= 599px) nor @media (width >= 600px) to match. I have observed this. See also

https://stackoverflow.com/questions/51566916/why-does-bootstrap-use-a-0-02px-difference-between-screen-size-thresholds-in-its
w3c/csswg-drafts#1083
https://bugzilla.mozilla.org/show_bug.cgi?id=1120090
https://bugs.chromium.org/p/chromium/issues/detail?id=689096

timabbott pushed a commit to zulip/zulip that referenced this pull request Feb 5, 2021
On a high-DPI display or with a non-default zoom level, the browser
viewport may have a width strictly between md_max = 767px and md_min =
768px.  Use only the *_min bounds for consistency.

This requires queries with strict inequalities to express upper
bounds (width < md_min).  Fortunately, that functionality is provided
by range context queries.  Unfortunately, those are not supported in
all browsers.  Fortunately, we can compile them away using
postcss-media-minmax.  Unfortunately, postcss-media-minmax currently
subtracts 1px for strict inequalities anyway to work around a Safari
rounding bug.  Fortunately, 0.02px should be sufficient for that, so I
submitted a PR:

postcss/postcss-media-minmax#28

Signed-off-by: Anders Kaseorg <anders@zulip.com>
@yisibl
Copy link
Member

yisibl commented Jul 15, 2022

I'm sorry to reply to you so late. I ran git rebase origin/master on this branch to make sure there are no extra messages on the commit.

I think the change to 0.02 is better, it fixes some browser bugs that I was able to reproduce in Chrome 103 on my MacBook Pro.

I'll need to do some further research on the JS logic changes, and may be able to merge them after some tweaking.

According to the Bootstrap developers, using .02px rather than .01px
should be sufficient to work around the Safari rounding bug.

Fixes postcss#19.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
@andersk
Copy link
Author

andersk commented Jul 15, 2022

Thanks. It looks like you rewrote all the Git history to change your email address—FYI in the future, better practice for that is to add a mailmap.

I removed the empty commit left on this PR by your rebase.

@yisibl
Copy link
Member

yisibl commented Jul 15, 2022

Okay

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

Successfully merging this pull request may close these issues.

Fractional part is not generated when using < or > with px values
2 participants