Alina Firsova #13
Alina Firsova #13
Conversation
aleksandrbelik
left a comment
aleksandrbelik
left a comment
There was a problem hiding this comment.
Main notes:
- use another way for mark-up: grid or flex instead of table
- avoid of useing inline styles and styles-attributes on tags. Styles sohuld be in separate file
- try to keep you code more clearly, so format it.
| <meta charset="utf-8"> | ||
| <title>Cubes</title> | ||
| <style type="text/css"> | ||
| @import"style.css" |
There was a problem hiding this comment.
@AlinaFirsova Please look through 'link' tag overview
https://www.w3schools.com/tags/tag_link.asp
If you have external styles you need to use link instead of style
| @import"style.css" | ||
| </style> | ||
| </head> | ||
| <body background="img\back.png"> |
There was a problem hiding this comment.
@AlinaFirsova Comment related to the whole doc: please move all inline styles to separate css file. You can set classes to tags and use them in css file to set appropriate properties
| </style> | ||
| </head> | ||
| <body background="img\back.png"> | ||
| <div class="center"> |
There was a problem hiding this comment.
@AlinaFirsova Looks like it can be <main> or <section>
| <div class="center"> | ||
|
|
||
| <!�������� �������> | ||
| <table cellspacing="0" cellpadding="0" height="408" width="612"> |
There was a problem hiding this comment.
@AlinaFirsova Please avoid of using table if it is not representing tabular data.
Look at flex for example
|
|
||
| <! ������ ������> | ||
| <tr align="center" valign="middle" height="204"> | ||
| <td bgcolor="#00c4ef" width="204"><img src="img\rainy_icon.png"><br><br><front_main>CHICAGO / 23°</front_main></td> |
There was a problem hiding this comment.
@AlinaFirsova It is much bore better to use padding instead of br
|
|
||
| <! ������ ������> | ||
| <tr align="center" valign="middle" height="204"> | ||
| <td bgcolor="#00c4ef" width="204"><img src="img\rainy_icon.png"><br><br><front_main>CHICAGO / 23°</front_main></td> |
There was a problem hiding this comment.
@AlinaFirsova Please don't use front_main and front_add tags. Thay can be replaced by span or p
AlinaFirsova/Homework 1/style.css
Outdated
| @@ -0,0 +1,19 @@ | |||
| front_main{ | |||
| font-size: 100%; | |||
There was a problem hiding this comment.
@AlinaFirsova Please use approptiate font-size value, avoid of using percentage in fonts
AlinaFirsova/Homework 1/style.css
Outdated
| padding-bottom: 15%; | ||
| padding-right: 27%; | ||
| margin: 0 auto; | ||
| text-align: center;" |
There was a problem hiding this comment.
@AlinaFirsova Pay attention to inappropriate simbols in css file. " double quote here
| margin: 0; | ||
| } | ||
|
|
||
| .span{ |
There was a problem hiding this comment.
@AlinaFirsova Can't find any .span in code. Please remove unnecessary styles
| font-family: Francois One, bold; | ||
| color: white; | ||
| } | ||
| p{ |
There was a problem hiding this comment.
@AlinaFirsova Can't find any paragraph tag in code. Please remove unnecessary styles
| <div class="week"> | ||
| <div class="day"><div class="name">MON</div><div class="image-div"><img height="80%" src="img\rainy_small.png"></div><div class="value">22°</div></div> | ||
| <div class="day"><div class="name">TUE</div><div class="image-div"><img height="80%" src="img\rainy_small.png"></div><div class="value">19°</div></div> | ||
| <div class="day"><div class="name">WEN</div><div class="image-div"><img height="75%" src="img\cloud_small.png"></div><div class="value">18°</div></div> |
There was a problem hiding this comment.
@AlinaFirsova It's better to set any style property in css. So you can move height property to css with appropriate class name
| <div class="day"><div class="name">MON</div><div class="image-div"><img height="80%" src="img\rainy_small.png"></div><div class="value">22°</div></div> | ||
| <div class="day"><div class="name">TUE</div><div class="image-div"><img height="80%" src="img\rainy_small.png"></div><div class="value">19°</div></div> | ||
| <div class="day"><div class="name">WEN</div><div class="image-div"><img height="75%" src="img\cloud_small.png"></div><div class="value">18°</div></div> | ||
| <div class="day"><div class="name">THU</div><div class="image-sun-div"><img src="img\sun_cloud_small.png"></div><div class="value">14°</div></div> |
There was a problem hiding this comment.
@AlinaFirsova There is modificator approach. You can still set .image-divto img wrapper and add modificator class. So it can look like .image-div .image-sun-div
And that's mean that you don't need to duplicate css styles. In this case you won't duplicate height style
.image-div {
height: 25px;
}
.image-sun-div {
position: relative;
}
| position: relative; | ||
| } | ||
|
|
||
| .image-sun-div img { |
There was a problem hiding this comment.
@AlinaFirsova Please don't use tag names in styles, it's better to add class name to img
| } | ||
|
|
||
| .chicago{ | ||
| background-image:url(img/chicago_back.png); |
There was a problem hiding this comment.
@AlinaFirsova It's much bore better to use background color property instead of colorized image
| display: flex; | ||
| justify-content: center; | ||
| align-items: center; | ||
| flex-direction:column; |
There was a problem hiding this comment.
@AlinaFirsova Please try to avoid of duplicating of styles. So you can add common class for blocks and set it each block. For example
.block {
width: 205px;
height: 205px;
display: flex;
justify-content: center;
align-items: center;
flex-direction:column;
}
.chicago {
background-color: // colo that you need
}
| </head> | ||
| <body> | ||
| <div class="background"></div> | ||
| <div class="container"> |
There was a problem hiding this comment.
@AlinaFirsova try to use appropriate semantic tags in code
2 participants
No description provided.