Cubes / Anastasiya Santalova#2
Conversation
cubes widget (HTML+CSS), BEM medothology, responsive design.
svvald
left a comment
svvald
left a comment
There was a problem hiding this comment.
I have some issues displaying the widget in IE11 using both flexbox and grids
I guess it's OK though since IE11 doesn't have full support for them (especially for grids).
In general I'd say that you've done really good work with some points that could be improved.
| @@ -0,0 +1,8 @@ | |||
| // Colors for weather blocks | |||
There was a problem hiding this comment.
There is a good practice to name files that contain some variables, mixins etc. using leading underscore like _variables.scss. It is called partials and will prevent SASS from directly compiling these files entirely but only replacing the parts of the imported file you are using.
https://sass-lang.com/documentation/at-rules/import#partials
|
|
||
| padding: 0 30px; | ||
|
|
||
| @include fullScreen; |
There was a problem hiding this comment.
As far as I understand this mixin will make the block to have full width and height of its parent block but it won't actually be the fullscreen. I don't insist on renaming it here but it could be confusing in real projects.
There was a problem hiding this comment.
You right, thanks. It's my fault.
|
|
||
| // Visible elements before hover | ||
|
|
||
| .weather__block_visible { |
There was a problem hiding this comment.
You can avoid duplicating and rewriting the full classnames using SASS parent selector (sorry for one-line code, it seems that GitHub doesn't support newline symbols):
.weather { display: flex; &__block { width: 100%; } }
https://sass-lang.com/documentation/style-rules/parent-selector
However, be careful with it and always try to make your selectors as flat as possible since deeply nested rules are really hard to maintain.
There was a problem hiding this comment.
I agree with your comment, but didn't fix so much, because my current structure of nesting does not allow to improve it without changing all code.
There was a problem hiding this comment.
Tell me please, how can I check my markup on IE11, if I work on Mac?
There was a problem hiding this comment.
Actually I didn't work on Mac so I can't advice you anything specific. Take a look at this article:
https://dzone.com/articles/7-ways-you-can-test-your-website-on-internet-explo
Maybe it will help, the first point looks quite simple and well-working at the same time
There was a problem hiding this comment.
Also I don't insist on renaming all the classnames using SASS features, it still looks good, I just gave you the recommendation for future :)
| @@ -0,0 +1,317 @@ | |||
| @import "_variables"; | |||
There was a problem hiding this comment.
Didn't SASS fail during the compilation? As far as I'm concerned, you should still import files without using underscore even though filenames contain it.
There was a problem hiding this comment.
Hmm, it works... But, OK! :)
2 participants
No description provided.