@AGutierrezR
Posted
Hello there 👋. Good job on completing the challenge!
I have some suggestions about your code that might interest you. For the Accessibility part:
- In order to fix the landmark warning, wrap everything in a
<main>
tag, for now. - Instead of using a
<div>
for thecategories-tested
and it items, use a<ul>
and a<li>
, remove the list base styles of course.
For the Sass (SCSS) part:
- Avoid trying to fit every screen size in existence, use a limited system that allows you not to lose time in that task, in order to create this system, create variables with the target screen sizes, for example:
$sm: 0;
$md: 550px;
$lg: 768px;
$xl: 1024px;
$xxl: 1200px;
$xxxl: 1400px;
You could limit yourself to the two screen sizes that the Styleguide says. To use this you could create mixins that write the code for you:
$setting-max-width: $xxxl;
/**
* Sets screen media query with min-width set to $min parameter.
* @param $min {Number} [0]
* @content Extends mixin with properties for media query.
*/
@mixin helper-breakpoint-media-min($min: 0) {
@media only screen and (min-width: $min) {
@content;
}
}
/**
* Sets screen media query with breakpoints range passed as parameters.
* @param {Number} $min [0]
* @param {Number} $max [$setting-max-width]
* @content Extends mixin with properties for media query.
*/
@mixin helper-breakpoint-media-between($min: 0, $max: $setting-max-width) {
@media only screen and (min-width: $min) and (max-width: $max) {
@content;
}
}
/**
* Sets screen media query with max-width set to $max parameter.
* @param {Number} $max [$setting-max-width]
* @content Extends mixin with properties for media query.
*/
@mixin helper-breakpoint-media-max($max: $setting-max-width) {
@media only screen and (max-width: $max) {
@content;
}
}
In this case, the media-max
and media-between
use a $setting-max-width
variable that is set with the $xxxl
value.
So, instead of:
body {
width: 100vw;
height: max-content;
overscroll-behavior: none;
@media screen and (min-width: 1024px) {
height: 90vh;
@include verticallyCenter;
}
}
Use:
body {
width: 100vw;
height: max-content;
overscroll-behavior: none;
@include helper-breakpoint-media-min($xl) {
height: 90vh;
@include verticallyCenter;
}
}
Marked as helpful
@itsmesrishti
Posted
@AGutierrezR Thank you so much for your suggestions!!! I'll try implementing them next time.
If I use only certain sizes for the media queries that means that if, for example, an element's layout is breaking at 850px and I am using min-width then I should do the change at 768px (considering the aforementioned values)?
@AGutierrezR
Posted
Hello, @itsmesrishti!!
That could be a good approach, but, it is not a silver bullet. You could avoid micro-breaks limiting the container (or the element) growth, this could be achieved with a max-width
.
For example, in this project, starting from 320px screen size, you let the component width expand with the screen, and at 600px you set a new width of 70%, in this case, you could avoid this media query, and set a max-width: 420px
and a margin: 0 auto
from the beginning, this will prevent the element from expanding and getting wear looking.
Then, you could set your normal media query of 1024px
with the max-width: 50rem
you set. You could remove the width: 64vw
and the width: 52vw
that you set at 1440px,
If you have more questions, please ask them! I'm happy you help you in this journey!