-
Notifications
You must be signed in to change notification settings - Fork 22
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
Patternlab/dp 2080 reset breakpoints #597
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comment for assets/scss/00-base/_breakpoints.scss. This is more like a question.
$bp-large-extended-min: "min-width: " + ($bp-l-ex + 1); | ||
$bp-x-large-min: "min-width: " + ($bp-xl + 1); | ||
$bp-x-large-max: "max-width:" + $bp-xl; | ||
$bp-x-small-min: "min-width: 30.06rem"; // 481px |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason to use specific values (ex. $bp-x-small-min: "min-width: 30.06rem";
) instead of the variables like original (ex. $bp-x-small-min: "min-width: " + ($bp-xs + 1);
) by updating the variables below from pixel to rem?
$bp-xs: 480px;
$bp-s: 620px;
$bp-m: 780px;
$bp-l: 910px;
$bp-l-ex: 1050px;
$bp-xl: 1200px;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue being addressed by the ticket involved overlapping sections when the screen was zoomed in. The example video shows the issue on a howto page with a sticky sidebar menu.
Zoom levels don't change the screens pixel width, so zooming in can lead to overlapping of positioned elements.
By changing the values from pixels to rem, when you zoom in the media queries kick in on zoom as well as screen size.
So, using the howto page as an example again, when you zoom into the page, the mobile styles kick back in, rather than the elements overlapping.
I left the original variables in place because they are also used in styling some specific elements scattered around the site that require a px value to do some kind of math or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. That makes sense. Thank you for the answer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Description
Change breakpoints to rems for better accessibility at zoom levels
Related Issue / Ticket
Steps to Test
The page shown in links in the ticket are the howto pages
Screenshots