Cubes widget #8
Cubes widget #8
Conversation
Krivorotova Tatyana/index.html
Outdated
| </head> | ||
| <body> | ||
| <div id="widget"> | ||
| <div id="a" class="elem"> |
There was a problem hiding this comment.
Can you use more verbose names for classes and ids instead of a, b, c... and widget, elem? 😄
Krivorotova Tatyana/index.html
Outdated
| <body> | ||
| <div id="widget"> | ||
| <div id="a" class="elem"> | ||
| <img src="images/chicago.png"> |
There was a problem hiding this comment.
by specification tag <img> has two required attributes - src and alt https://www.w3schools.com/tags/tag_img.asp
Krivorotova Tatyana/index.html
Outdated
| <img src="images/brooklyn_cloud.png"> | ||
| <span>BROOKLYN / 55°</span> | ||
| </div> | ||
| <div id="d" class="elem"> |
There was a problem hiding this comment.
hmm, #a ,#b ,#c, #d #f looks like the similar elements. Can you change <div> to <section> and add some css class like city-card?
There was a problem hiding this comment.
you mean I have to change all the divs from class “elem” to sections or add sections which would contain class=“elem” elements and have class=“city-card”?
There was a problem hiding this comment.
yeap - change all the divs from class “elem” to sections (exclude #e element - see another comment) and add to them class city-card
Krivorotova Tatyana/index.html
Outdated
| <img src="images/portland.png"> | ||
| <span>PORTLAND / 27°</span> | ||
| </div> | ||
| <div id="e" class="elem"> |
There was a problem hiding this comment.
as for me #e looks like the main part of the widget and you can change <div> to <main>
Krivorotova Tatyana/index.html
Outdated
| <img src="images/montue_cloud.png"> | ||
| <div id="g"> | ||
| <div id="i"> | ||
| <div class="row"> |
There was a problem hiding this comment.
hmm, looks like elements from the next 3 rows below - logically is the one entity ( I mean that here is just 4 entities like <span>MON</span><img/><span>22°</span> if I'm correct you need to change this markup to something like
<ul class=row>
<li class="something with display: flex and flex-direction: column">
<span>MON</span><img/><span>22°</span>
</li> and so on
Krivorotova Tatyana/style.css
Outdated
| margin: 0; | ||
| } | ||
|
|
||
| #widget{ |
There was a problem hiding this comment.
It's not good idea to use id selector for styling your markup. This selector has big specifity and it will be hard to override it if needed (see https://developer.mozilla.org/ru/docs/Web/CSS/Specificity). In general, you should use classes for this purpose.
| height: 70px; | ||
| margin-left: 20px; | ||
| margin-top: 15px; | ||
| display: flex; |
There was a problem hiding this comment.
try to use some ordering for your css properties - there is good example http://codeguide.academy/html-css.html. Order should be consistent for all css rules (if in one you add display before width - do the same for other rules as well)
| padding-inline-start: 0; | ||
| margin-block-start: 0; | ||
| margin-block-end: 0; | ||
| } |
There was a problem hiding this comment.
The .city-card-schedule__days-list element's padding and margins styles have to be changed in order to be properly displayed on IE/Edge
There was a problem hiding this comment.
you can use padding: 0; and margin: 0; for this purposes. It should overwrite padding-inline-start and margin-block-start, margin-block-end in modern browsers and will work for old
| </head> | ||
| <body> | ||
| <div class="weather-widget"><!-- Widget --> | ||
| <section class="weather-widget__city-card city-card-chicago"><!-- First card "Chicago" (img and name of the city) --> |
There was a problem hiding this comment.
I think comments in html are excessive. In general, other developers should understand your markup by semantic (using suitable html tags, verbose class names, good naming for custom elements).
There was a problem hiding this comment.
Are class names that I've chosen correct and verbose enough? I mean, there was an issue with first version of names and I would like to know how good it is done now.
There was a problem hiding this comment.
I think it's good now 😄 - for now I pretty easily can understand what you do in this file)
2 participants
No description provided.