Book cover #4
Book cover #4
Conversation
maquetado/scss/styles.scss
Outdated
| display: flex; | ||
| flex-direction: column; | ||
| width: 400px; | ||
| margin-left: 100px; |
There was a problem hiding this comment.
margin-right in the image. Also, I think it's too big, it looks smaller in the design, don't you think?
There was a problem hiding this comment.
Hmm, at least the values that are @ invision match with these. It did look big but due to the fact that the text was kinda small.
maquetado/scss/styles.scss
Outdated
| .column { | ||
| display: flex; | ||
| flex-direction: column; | ||
| width: 400px; |
There was a problem hiding this comment.
I wouldn't fix this width. You can set flex-grow: 1 so it grows all it can grow inside the "row"
Also, if you are going to make this column class, it might be better to make it more "generic" in case you eventually want to have another "column". I would remove this and leave only the display and flex-direction, and I'd make another class called book-information-container or something that has the specific properties this div needs, then add both this classes to the div.
There was a problem hiding this comment.
The same applies to classes like .book-container, you can use row or column in the html instead of setting display: flex inside it. For that type of things it's better to use more generic classes and leave these for more specific things like font-size, position, etc.
maquetado/scss/styles.scss
Outdated
| .row { | ||
| display: flex; | ||
| flex-direction: row; | ||
| align-items: baseline; |
There was a problem hiding this comment.
As the column class, I'd remove the align-items from here as it's specific to this row in particular.
maquetado/index.html
Outdated
| <h3 class="book-detail-value height">Año de publicación</h3> | ||
| <img src="./assets/book-cover.png" class="book-cover"> | ||
| <div class="column book-info-container"> | ||
| <div class="row book-row-info-container"> |
There was a problem hiding this comment.
I would change this name into something more semantic like book-title-container
maquetado/scss/styles.scss
Outdated
| @@ -4,19 +4,19 @@ | |||
|
|
|||
| .book-container { | |||
| display: flex; | |||
There was a problem hiding this comment.
You can use row class instead and use this for other css properties. It's a good idea to re use css layout classes, because that way you can know how it is supposed to look by just checking the html tags and their classes
done |
maquetado/scss/styles.scss
Outdated
| margin-bottom: 20px; | ||
| .row { | ||
| display: flex; | ||
| flex-direction: row; |
There was a problem hiding this comment.
This is not necessary. By default the flex-direction is set to row.
maquetado/index.html
Outdated
| <h3 class="book-detail-name bold height">Año de publicación:</h3> | ||
| <h3 class="book-detail-value height">Año de publicación</h3> | ||
| <div class="book-container row"> | ||
| <img src="./assets/book-cover.png" class="book-cover"> |
There was a problem hiding this comment.
this img tag doesn't self-close
| <img src="./assets/book-cover.png" class="book-cover"> | |
| <img src="./assets/book-cover.png" class="book-cover" /> |
maquetado/index.html
Outdated
| <h3 class="book-detail-value height">Año de publicación</h3> | ||
| <div class="book-container row"> | ||
| <img src="./assets/book-cover.png" class="book-cover"> | ||
| <div class="column book-info-container"> |
There was a problem hiding this comment.
Remove a level of indentation, it looks like these divs that follow are children of the img
Summary
Added book cover picture
Screenshots
Trello Card
https://trello.com/c/o6flXrfb/32-detalle-de-libro-ii-imagen