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

Improve CSS media queries #576

Merged
merged 1 commit into from
Apr 17, 2018
Merged

Improve CSS media queries #576

merged 1 commit into from
Apr 17, 2018

Conversation

yangshun
Copy link
Contributor

Motivation

Realized that the CSS of the site was using device-width media queries, which is deprecated. Use of device-width is not encouraged because it uses the hardware width rather than the viewport width.

This Stack Overflow thread explains well why it's a bad idea:

If your resolution is large enough to get you in one break point, but the width of the browser is small enough to get you in another one, you'll end up with an odd combination of both.

In fact, the site is already suffering from this bug, for example, for the mobile web version of the installation page, the "Edit" button is shown even though it's not supposed to, going by the media query:

@media only screen and (min-device-width: 360px) and (max-device-width: 735px) {
  a.edit-page-link.button {
    display: none;
  }
}

I replaced all device-width with the width variants and removed redundant min-widths. Having the min-width breakpoint does not make much sense because there are is no CSS written to handle the widths smaller than the ones the CSS currently has.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Loaded the page on Chrome Devtools Pixel 2 resolution and navigated all the pages. No visible issues found.

Related PRs

None

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 17, 2018
Copy link
Contributor

@JoelMarcey JoelMarcey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

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

Successfully merging this pull request may close these issues.

3 participants