PR para comentários #3
Conversation
paula-aribeiro
left a comment
paula-aribeiro
left a comment
There was a problem hiding this comment.
Oi pessoal, parabéns pelo trabalho de vocês 😄
Vocês mandaram muito bem nos requisitos obrigatórios e no desafio!
Uma sugestão de melhoria é que vocês poderiam ter componentizado mais, por exemplo, criando um componente para o carrinho e um para os filtros.
Também deixei umas correções em relação à organização de código.
Parabéns, continuem evoluindo 🚀 🚀 🚀
| super(props) | ||
| this.state ={ | ||
| ordem: "Crescente" , | ||
| quantidade: JSON.parse(localStorage.getItem("quantidade")) || [0,0,0,0,0,0,0] , |
| display:flex; | ||
| flex-wrap:wrap; | ||
| ` | ||
| const lista = [ |
There was a problem hiding this comment.
Seria bom utilizar um nome mais significativo para essa lista
| filtroNome: '' | ||
| } | ||
| } | ||
| somar = () => { |
There was a problem hiding this comment.
Essa função poderia ter um nome mais significativo 😉
| ) | ||
| } | ||
| removerItem = (event) => { | ||
| const copy = this.state.quantidade.map((quant, i) => { |
There was a problem hiding this comment.
copy não é um bom nome de variável
| filtrosMin: event.target.value | ||
| }) | ||
| }} | ||
| ></input><br/> |
There was a problem hiding this comment.
Não utilize <br>! Prefira usar o CSS para espaçamentos :)
| if (this.state.quantidade[i] > 0) | ||
| return ( | ||
| <div id={produto.id} key={produto.id}> | ||
| {produto.nome + ":" + " " + this.state.quantidade[i]} |
There was a problem hiding this comment.
Isso poderia ser feito com template string, dessa forma:
`${produto.nome}: ${this.state.quantidade[i]}`|
|
||
| } | ||
| <strong>Total: </strong> | ||
| {this.somar ()} |
There was a problem hiding this comment.
Essa função não deveria estar perdida aqui no meio do render. Seria melhor que algum evento ou método de lifecycle a chamasse
| {this.state.ordem === "Crescente" ? | ||
| lista.filter((produto)=> | ||
| produto.preco > this.state.filtrosMin && produto.preco < this.state.filtrosMax && produto.nome.includes(this.state.filtroNome) | ||
| ).map((produto)=> | ||
| <Produto | ||
| id={produto.id} | ||
| key={produto.id} | ||
| imagem={produto.imagem} | ||
| preco={produto.preco} | ||
| nome={produto.nome} | ||
| passarId={this.adicionarItem} | ||
| /> | ||
| ) | ||
| : | ||
| lista.filter((produto)=> | ||
| produto.preco > this.state.filtrosMin && produto.preco < this.state.filtrosMax && produto.nome.includes(this.state.filtroNome) | ||
| ).map((produto)=> | ||
| <Produto | ||
| id={produto.id} | ||
| imagem={produto.imagem} | ||
| preco={produto.preco} | ||
| nome={produto.nome} | ||
| passarId={this.adicionarItem} | ||
| /> | ||
| ).reverse() | ||
| } |
There was a problem hiding this comment.
Evite deixar lógicas no meio do JSX. Além disso, evite usar ternário para respostas muito grandes, pois dificulta a leitura e manutenção do código. Nesse caso, seria melhor criar uma função com um if/else.
| const Div = styled.div` | ||
| width: 200px; | ||
| border: 1px solid black; | ||
| margin: 10px; | ||
| display:flex; | ||
| flex-direction:column; | ||
| justify-content:space-between; | ||
| font-size: 15px; | ||
| ` | ||
| const Button = styled.button` | ||
| width: 200px; | ||
| color:white; | ||
| background-color:midnightblue; | ||
| ` | ||
|
|
||
| const Img = styled.img` | ||
| width: 200px; | ||
| height: 150px; | ||
| ` |
There was a problem hiding this comment.
Essas variáveis poderiam ter nomes mais específicos
| <Div id={props.id} > | ||
| <Img src={props.imagem}></Img> | ||
| <span>{props.nome}</span><br/> | ||
| <span>{"R$"}{props.preco}{",00"}</span> |
There was a problem hiding this comment.
Aqui poderia ser feito dessa forma:
<span>R${props.preco},00</span>There was a problem hiding this comment.
Cuidado que isso não funcionaria para valores quebrados! Nesse caso, seria melhor utilizar
<span>R${props.preco.toFixed(2)}</span>
No description provided.