-
Notifications
You must be signed in to change notification settings - Fork 9
Homework #15
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
base: master
Are you sure you want to change the base?
Homework #15
Conversation
aleksandrbelik
left a comment
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.
Need to format css files to avoid blank string and useless tabs
Grid variant works correct only in Chrome unfortunately
Alina Firsova/flex/flex.html
Outdated
| @@ -0,0 +1,39 @@ | |||
| <!DOCTYPE HTML> | |||
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.
@AlinaFirsova It's better to keep main html file named as 'index.html'
Alina Firsova/grid/grid.html
Outdated
| @@ -0,0 +1,43 @@ | |||
| <!DOCTYPE HTML> | |||
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.
@AlinaFirsova Same issue with file name
Alina Firsova/flex/flex.html
Outdated
| <meta name="viewport" content="width=device-width, | ||
| initial-scale=1.0, maximum-scale=1.0, user-scalable=no"> | ||
| </head> | ||
| <body class="background"> |
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.
@AlinaFirsova It's better to avoid such common class names
Alina Firsova/flex/flex.html
Outdated
| <div class="main-container__element"><img src="img\half_moon.png"></div> | ||
| <div class="main-container__week-bottom"> | ||
| <div class="main-container__week"> | ||
| <div class="main-container__day"><div class="main-container__name">MON</div><div class="main-container__image-div"><img height="80%" src="img\rainy_small.png"></div><div class="main-container__value">22°</div></div> |
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.
@AlinaFirsova Please set image height in styles
Alina Firsova/flex/style.css
Outdated
| } | ||
|
|
||
|
|
||
| } |
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.
@AlinaFirsova you can set styles for main-container by defult (for small screens for ex.) and then add media only for bigger cases
`
.main-container {
height: 615px;
}
@media screen and (min-width: 615px) {
.main-container {
width: 615px;
height: 410px;
}
}
`
Alina Firsova/flex/style.css
Outdated
|
|
||
|
|
||
| .main-container__chicago{ | ||
| background-image:url(img/chicago_back.png); |
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.
@AlinaFirsova It's much more better to set background color instead of image filled with one color
Alina Firsova/flex/style.css
Outdated
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| flex-direction:column; |
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.
@AlinaFirsova You can extract several properties to separate class to avoid suplicating them.
These properties looks like common for a few blocks:
background-repeat: no-repeat; width: 205px; height: 205px; display: flex; justify-content: center; align-items: center; flex-direction:column;
Alina Firsova/flex/style.css
Outdated
| background-size: contain; | ||
| width: 205px; | ||
| height: 205px; | ||
| height: 205px; |
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.
@AlinaFirsova duplicated property
No description provided.