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. Não precisa mergeear. #58

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

Conversation

pedro-severo
Copy link

@pedro-severo pedro-severo commented Dec 16, 2020

PR de correção. Não precisa mergeear.

https://future-eats4-dumont.surge.sh/

nizolnier and others added 30 commits December 7, 2020 13:44
add pastas e alguns components
O nome da branch é loading, mas eu fiz a página de restaurante.
criando o formulário de cadastro
EditProfilePage criada com forms, falta fazer a appbar e criar funções de API
 estilização do loading em andamento
criando feedpage, falta testar API e estilizar
adicionando função de adicionar endereço
nizolnier and others added 23 commits December 11, 2020 14:37
criando o componente Snackbar
estilização do feed e search ok
botão de contituar comprando funcionando, falta arrumar margem
bug de requisições de login e signup resolvido
cart limpo dps de pedir e snackbar certinho
Comment on lines +55 to +67
<Select required onChange={handleSelect} value={option} label="Quantidade">
<option aria-label="None" disabled />
<option value={1}>1</option>
<option value={2}>2</option>
<option value={3}>3</option>
<option value={4}>4</option>
<option value={5}>5</option>
<option value={6}>6</option>
<option value={7}>7</option>
<option value={8}>8</option>
<option value={9}>9</option>
<option value={10}>10</option>
</Select>
Copy link
Author

Choose a reason for hiding this comment

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

Acho que seria legal isolar esse select em um componente separado. Esse tipo de select é comum ser usado mais de uma vez no projeto, além de ser comum de aparecer em outros projetos. Isolando, vcs podem reaproveitar o componente criado.

Comment on lines +5 to +95

const FoodContainer = styled.div`
display:flex;
width: 360px;
max-width:360px;
height: 112px;
border-radius: 8px;
border: solid 1px #A9A9A9;
margin-top:10px;
`

const Img = styled.img`
width: 90px;
/* width: 112px; */
height: 112px;
margin-right:10px;
border-radius: 8px 0px 0px 8px;
margin: 0 16px 0 0;
/* object-fit: contain; */
`

const Title = styled.p`
width: 157px;
/* height: 18px; */
margin: 10px 0px 0 1px;
font-family: Roboto;
font-size: 16px;
letter-spacing: -0.39px;
color: #5cb646;
`
const Description = styled.p`
/* width: 120px; */
/* height: 60px; */
/* margin: 8px 16px 8px; */
font-family: Roboto;
font-size: 12px;
letter-spacing: -0.29px;
color: var(--greyish);
`
const Price = styled.h3`
width: 118px;
/* height: 19px; */
margin: 0px 0px 0px 0px;
font-family: Roboto;
font-size: 16px;
letter-spacing: -0.39px;
color: #000000;
`
const ButtonContainer = styled.div`
display:flex;
flex-direction:column;
align-items:flex-end;
justify-content:space-between;
margin: 1px 0 0 0px;
padding: 0 0 0 5px;
border-radius: 0px 8px 0px 8px;


`
const ButtonAdd = styled.div`
width: 70px;
height: 25px;
margin: 75px 0 0 0;
padding: 5px 8px 5px 12px;
border-radius: 8px 0 8px 0;
border: solid 1px #5cb646;
color:#5cb646;
cursor: pointer;
`
const ButtonRemove = styled.div`
width: 70px;
height: 25px;
/* margin: 0 0 0 45px; */
padding: 5px 8px 5px 12px;
border-radius: 8px 0 8px 0;
border: solid 1px #e02020;
color:#e02020;
cursor: pointer;
`
const Count = styled.div`
width: 15px;
height: 25px;
/* margin: 0x 0 0 0px; */
padding: 3px 5px 6px 10px;
border-radius: 0px 8px 0px 8px;
border: solid 1px #5cb646;
color:#5cb646;

`

const FoodCard = (props) => {
Copy link
Author

Choose a reason for hiding this comment

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

Isolem as constantes do styled-components.

Comment on lines +5 to +8
// import CardActions from '@material-ui/core/CardActions';
import CardContent from '@material-ui/core/CardContent';
import CardMedia from '@material-ui/core/CardMedia';
// import Button from '@material-ui/core/Button';
Copy link
Author

Choose a reason for hiding this comment

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

Não se esqueçam de apagar os códigos comentados e não usados no projeto.

Comment on lines +28 to +31
// const Image = styled(CardMedia)`
// width: 360px;
// height: 120px;
// `
Copy link
Author

Choose a reason for hiding this comment

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

Vide comentário anterior hahaha

return <MuiAlert elevation={5} variant="filled" {...props} />;
}

// fomos obrigada a usar useStyles pq deu erro ao inicializar com o styled
Copy link
Author

Choose a reason for hiding this comment

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

Normal! Quando se usa o material o estranho é não dar erro hahaha

Comment on lines +3 to +5
export const axiosConfig = {
headers: { auth: window.localStorage.getItem("token") },
};
Copy link
Author

Choose a reason for hiding this comment

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

Isolar a axiosConfig dessa forma, nesse caso, não é uma boa. Nos casos em que a configuração do headers necessita de um valor do localStorage, que pode mudar com o tempo, é melhor isolar tudo em uma função e chamar a função sempre que for usar a axiosConfig, dessa forma:

export const generateAxiosConfig = () =>{
     const axiosConfig = {
         headers: { auth: window.localStorage.getItem("token") }
     }

    return axiosConfig
}

Comment on lines +4 to +22
export const useRequestData = (url, initialState) => {
const [data, setData] = useState(initialState)


useEffect(() => {
axios.get(url, {
headers: {
auth: window.localStorage.getItem("token") },
})
.then((response) => {
setData(response.data);
})
.catch((erro) => {
console.log(erro);
});
}, [url])

return data;
}
Copy link
Author

Choose a reason for hiding this comment

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

Nesse caso de usar o hook useRequestData, é uma boa criar uma função para pegar os dados do banco de dados sempre que vcs precisarem. Isso é bem importante para atualizar os dados de uma página sem precisar de dar refresh no site. Ficaria dessa forma o hook:

export const useRequestData = (url,initialState)=>{
    const [data,setData]=useState(initialState)

    const getData = () => {
        axios.get(url,headers).then((res)=>{
            setData(res.data)
        }).catch((err)=>{
            console.log(err.message)
        })
    }


    useEffect(()=>{
         getData()
    },[getData])


    return [data, getData]
} 

Com isso, na hora de usar a função, vcs sempre poderão chamar, além do data em si, a função que atualiza o data, para atualizar as informações na tela sempre que precisarem

@pedro-severo
Copy link
Author

Oi, meninas!! Parabéns pelo trabalho, ficou sensacional!

O trabalho ficou com alguns bugzinhos de código e de UX. Mas em trabalhos complexos assim e com muita gente, e levando em conta que vcs tiveram só uma semana pra fazer (muito pouco tempo!), era esperado. Nada que tira o excelente trabalho feito!

Algumas coisas específicas para que vcs melhorem nos próximos projetos:

  • se atentem para console.log perdidos pelo código. Enquanto eu fui navegando pelo código, foi aparecendo tokens, objetos, etc. Isso não é legal chegar para o usuário, por questões de segurança e de experiência.

  • quando eu faço um pedido, está gerando um bug no valor do preço total do carrinho (SUBTOTAL). Não condiz com o valor dos pedidos. Além disso, depois que o pedido é feito e o carrinho se esvazia, o valor do SUBTOTAL anterior continua aparecendo.

  • quando eu faço logout e faço login novamente, com um pedido em andamento, os cards dos restaurantes somem do feed.

Mas como disse, nada que tira o excelente trabalho de vcs! Ficou muito bom!

Vcs fecharam o módulo de fornt com chave de ouro! Parabéns!!

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.

6 participants