eslint
eslint copied to clipboard
chore: fix off-by-one `min-width: 1023px` media queries
Prerequisites checklist
- [x] I have read the contributing guidelines.
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update [ ] Bug fix (template) [ ] New rule (template) [ ] Changes an existing rule (template) [ ] Add autofix to a rule [ ] Add a CLI option [ ] Add something to the core [x] Other, please explain:
Updates css for the new docs site.
We have min-width: 1023px in several media queries. This seems like an off-by-one error as it matches >= 1023px which was probably not the intention.
What changes did you make? (Give an overview)
Changed min-width: 1023px to min-width: 1024px in media queries.
Is there anything you'd like reviewers to focus on?
max-width: 800px in multiple places also looks suspicious, but I wasn't sure about that one so I left it out from this PR.
Deploy Preview for docs-eslint ready!
| Name | Link |
|---|---|
| Latest commit | e4ef3962a0026408c84e1401e9d70714063f078e |
| Latest deploy log | https://app.netlify.com/sites/docs-eslint/deploys/62a096dd09324d0008f8e3e0 |
| Deploy Preview | https://deploy-preview-15974--docs-eslint.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site settings.
@SaraSoueidan can you comment on this? Is this intentional?
@nzakas It was intentional, yes.
It's not a new practice. It ensures the “desktop” media queries aren't triggered on a 1024p-wide viewport, which has typically been a tablet (such as iPad) size. This practice has been carved in my brain for years that it comes naturally now.
If @mdjermanovic has tested the site on medium-size displays and found no issues, then this PR LGTM.
It ensures the “desktop” media queries aren't triggered on a 1024p-wide viewport, which has typically been a tablet (such as iPad) size.
I'm confused now. If we don't want these styles to apply on 1024px, then shouldn't this actually be min-width: 1025px?
Before & after this change are same on 1024px:
1024px before & after this change

But different on 1023px, because both min-width: 1023px and max-width: 1023px styles that we have apply when the width is exactly 1023px:
1023px before this change

1023px after this change

I followed up with Sara (she’s been extremely busy) and she said to go ahead and merge this.