Skip to content
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

PR de correção #6

Open
wants to merge 12 commits into
base: correcao-projeto
Choose a base branch
from
Open

PR de correção #6

wants to merge 12 commits into from

Conversation

jvpalves
Copy link
Contributor

@jvpalves jvpalves commented Mar 4, 2020

No description provided.

Copy link
Contributor Author

@jvpalves jvpalves left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Esse é o review do projeto de vocês. Os comentários mais pontuais estarão ao longo do código.

Em linhas gerais, eu amei o site de vocês! Está muito bonito e bem escrito, quase não tive o que comentar haha. O uso de HTML5 está óitmo, com as tags semânticas usadas corretamente, sem tags descontinuadas e muito bem diagramado. Senti falta no entanto de uma versão web, mesmo que tenha adorado a versão mobile (adorei que discriminaram como "Mobile-First"). O CSS também está excelente, com bom uso dos seletores, classes e Ids. Os comentários separando as seções de estilização foram um plus bom demais (especialmente para quem vos escreve haha). Continuem assim e até o próximo projeto!

Comment on lines +151 to +173
.container {

display: grid;
grid-template-columns: 1fr 1fr ;
grid-template-rows:repeat(4, 1fr) ;
border:none;
row-gap: 4vw;
column-gap: 2vh;
margin:4vh auto 4vh auto;
width:85vw;
height:100vh;
align-items: center;
justify-items: center;

}

h2 {

text-align: center;
margin-top: 40px;
}

.sapato {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cuidado com a mistura de inglês com português na nomeação de propriedades.

width: 300px;
position: absolute;
transition: all .2s linear;
left: -300px;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cuidado com posicionamentos negativos. Nesse caso é ok, mas um bom jeito de evitar isso é fazer com que o usuário não possa mudar o tamanho da tela. (user-scalable=no) no HTML

Comment on lines +188 to +274
#sapato1 {

border: 1px solid #DFDFDF;
box-sizing: border-box;
background-color: #DFDFDF;
grid-row-start:1 ;
grid-row-end: 2;
grid-column-start: 1;
grid-column-end: 2;
}
#sapato2 {

border: 1px solid #DFDFDF;
box-sizing: border-box;
background-color: #DFDFDF;
grid-row-start:1 ;
grid-row-end: 2;
grid-column-start: 2;
grid-column-end: 3;

}
#sapato3 {

border: 1px solid #DFDFDF;
box-sizing: border-box;
background-color: #DFDFDF;
grid-row-start:2 ;
grid-row-end: 3;
grid-column-start: 1;
grid-column-end: 2;

}
#sapato4 {

border: 1px solid #DFDFDF;
box-sizing: border-box;
background-color: #DFDFDF;
grid-row-start:2 ;
grid-row-end: 3;
grid-column-start: 2;
grid-column-end: 3;

}
#sapato5 {

border: 1px solid #DFDFDF;
box-sizing: border-box;
background-color: #DFDFDF;
grid-row-start:3 ;
grid-row-end: 4;
grid-column-start: 1;
grid-column-end: 2;

}
#sapato6 {

border: 1px solid #DFDFDF;
box-sizing: border-box;
background-color: #DFDFDF;
grid-row-start:3 ;
grid-row-end: 4;
grid-column-start: 2;
grid-column-end: 3;

}
#sapato7 {

border: 1px solid #DFDFDF;
box-sizing: border-box;
background-color: #DFDFDF;
grid-row-start:4 ;
grid-row-end: 5;
grid-column-start: 1;
grid-column-end: 2;

}
#sapato8 {

border: 1px solid #DFDFDF;
box-sizing: border-box;
background-color: #DFDFDF;
grid-row-start:4 ;
grid-row-end: 5;
grid-column-start: 2;
grid-column-end: 3;

}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As propriedades repetidas podiam ser unidas em uma classe

Comment on lines +75 to +130
<div id="sapato1" class="sapato">

<img src="src/sapatoNatalia.png" alt="Sapato cano alto florido" class="imagem">
<div class="texto + preço">
<p>R$ 200,00</p>
</div>
</div>
<div id="sapato2" class="sapato">

<img src="src/sapato2.jpg" alt="Tenis Branco" class="imagem">
<div class="texto + preço">
<p>R$ 160,00</p>
</div>
</div>

<div id="sapato3" class="sapato">
<img src="src/sapato3.jpg" alt="Alpagarta" class="imagem">
<div class="texto + preço">
<p>R$ 100,00</p>
</div>
</div>

<div id="sapato4" class="sapato">
<img src="src/sapato4.jpg" alt="bota masculina" class="imagem">
<div class="texto + preço">
<p>R$ 290,00</p>
</div>
</div>

<div id="sapato5" class="sapato">
<img src="src/sapatoErika.png" alt="Oxford florido" class="imagem">
<div class="texto + preço">
<p>R$ 140,00</p>
</div>
</div>

<div id="sapato6" class="sapato">
<img src="src/sapato6.png" alt="rasteirinha" class="imagem">
<div class="texto + preço">
<p>R$ 120,00</p>
</div>
</div>
<div id="sapato7" class="sapato">
<img src="src/sapato7.png" alt="Mule Floral" class="imagem">
<div class="texto-preço">
<p>R$ 210,00</p>
</div>

</div>
<div id="sapato8" class="sapato">
<img src="src/sapato8.png" alt="sapato loafer" class="imagem">
<div class="texto + preço">
<p>R$ 200,00</p>
</div>
</div>
</div>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tudo perfeito aqui, o uso de section de alt para as imagens, e das classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants