-
Notifications
You must be signed in to change notification settings - Fork 0
PR para correção e comentários #12
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
base: correcao
Are you sure you want to change the base?
Conversation
Container do produto concluido
Criei parte estática do menu do carrinho e botão com renderização condicional
criei parte estática do filtro lateral
atualização do app com as fotos implementadas
Ajustes no carrinho
iniciando filtro lateral
LojaCarrinho, Produto e MainContainer atualizados
Ajustes finais
paula-aribeiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oi pessoal,
Bem bacana o projeto de vocês, mas ficou faltando boa parte das funcionalidades que pedimos.
Sobre o código, vocês componentizaram bastante, mas acredito que vocês poderiam ter separado melhor as responsabilidades de cada componente. Por exemplo, um componente para tratar dos produtos, outro para tratar dos filtros, outro para o carrinho e um que utilizasse todos esses. Além disso, em alguns arquivos vi componentes sendo importados e não utilizados, o que dificultou bastante o entendimento do código.
Parabéns pelo trabalho em equipe e continuem evoluindo 🚀
|
|
||
|
|
||
|
|
||
| export default class ProdutosContainer extends React.Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uma boa prática é nomear o componente com o mesmo nome da pasta, nesse caso, MainContainer
| const MainContainer = styled.div` | ||
| /* border: 1px solid black; */ | ||
| width: 100%; | ||
| height: 650px; | ||
| display: flex; | ||
| `; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Esse styled component deveria ter um nome diferente da pasta, para não confundir no render
| onChangeMin = event => { | ||
| this.setState({ | ||
| valorMinInput: Number(event.target.value) | ||
| }) | ||
| } | ||
|
|
||
| onChangeMax = event => { | ||
| this.setState({ | ||
| valorMaxInput: Number(event.target.value) | ||
| }) | ||
| } | ||
|
|
||
| onChangeMin = event => { | ||
| this.setState({ | ||
| valorBuscaInput: event.target.value | ||
| }) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A função onChangeMin foi criada duas vezes
| // const ProdutosFiltrados = this.props.produtos.filter() | ||
| return ( | ||
| <MainContainer> | ||
| <FiltroDeProduto/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acredito que esse componente <FiltroDeProduto/> poderia ter sido chamado fora desse componente ProdutosContainer, pois ele é de outro contexto.
| <Select> | ||
| <option>Preço: Crescente</option> | ||
| <option>Preço: Decrescente</option> | ||
| </Select> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isso poderia fazer parte do componente de filtros
| @@ -0,0 +1,83 @@ | |||
| import React from 'react'; | |||
| import styled from 'styled-components'; | |||
| import FiltradosPorPreco from '../FiltroDeProduto/FiltradosPorPreco' | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
O FiltradosPorPreco foi importado mas não foi utilizado
| constructor(props) { | ||
| super(props); | ||
| this.state = { | ||
| listaCarrinho1: [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Por que listaCarrinho1 e não apenas listaCarrinho?
|
|
||
| render() { | ||
| const listaDeItensNoCarrinho = this.props.listaCarrinho2.map((cadaItem) => { | ||
| return <ItemProduto><ExcluirItem>X</ExcluirItem>{cadaItem.nome} - R$ {cadaItem.valor},00</ItemProduto> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cuidado que isso não funcionaria para valores quebrados! Nesse caso, seria melhor utilizar
{cadaItem.valor.toFixed(2)}
| const listaDeItensNoCarrinho = this.props.listaCarrinho2.map((cadaItem) => { | ||
| return <ItemProduto><ExcluirItem>X</ExcluirItem>{cadaItem.nome} - R$ {cadaItem.valor},00</ItemProduto> | ||
| }) | ||
| console.log(listaDeItensNoCarrinho) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Evite deixar console.log pelo código
| <p> | ||
| Total: <b>R$ 0,00</b> | ||
| </p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isso está estático
No description provided.