Skip to content

LabeEddit#32

Merged
phsarah merged 6 commits intomasterfrom
semana12-projeto
Jan 4, 2021
Merged

LabeEddit#32
phsarah merged 6 commits intomasterfrom
semana12-projeto

Conversation

@phsarah
Copy link
Copy Markdown
Collaborator

@phsarah phsarah commented Dec 3, 2020

LabeEddit

usuario: sarah@dumont.com
senha: 123456

O que funciona

  • DESCREVA O QUE FUNCIONA NO SEU PROJETO

O que não funciona

  • DESCREVA O QUE NÃO FUNCIONA NO SEU PROJETO

Link Surge

upset-kitten.surge.sh

Imagens

TIRE PRINTS DAS TELAS DO SEU SITE E COLE AQUI

Copy link
Copy Markdown

@leticia-chijo leticia-chijo left a comment

Choose a reason for hiding this comment

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

Oi! Passando pra dizer que seu projeto está dentro do esperado, parabéns!

A maioria das funcionalidades básicas está funcionando e seu código está organizado, mas tenho algumas observações:

  • Tem algumas coisas que foram colocadas mas não implementadas. Por exemplo: botão de compartilhar post. Se ele não está sendo usado, é melhor retirar, pois pode deixar o usuário confuso
  • A parte de autorização e autenticação está um pouco estranha. Ele não guarda que eu estou logada e me permite voltar para a tela de login. Se ficou com dúvidas nesse pedaço, acho que o projetinho que o Caio e o Severo fizeram em aula tem uma implementação legal usando custom hooks
  • Na tela de detalhe do post, o texto do post não está aparecendo!

Mas fora isso me parece que tudo funciona bem, então parabéns!

@nizolnier
Copy link
Copy Markdown

oi sarah!! vim revisar seu pr pro peer review!

começando pela organização do pr, tá faltando bastante coisa (acho que foi a correria né? eu faço isso tbm haha), mas tem o link do surge que é o mais importante então é suaveee!

o código tá bem identado, até na parte dos styles, isso facilita bastante a leitura! tá tudo nomeado bonitinho, os components, as variaveis e as funções todas fazem sentido!

eu gostei muito da sua estilização com o material-ui, ficou bem legal e te dou um parabens assim pq eu acho essa lib beeeeem confusa e você fez um bom trabalho lidando com a confusão dela (não sei se isso que eu falei fez muito sentido). ficou massa a estilização!

ah e as funções funcionam direitinho :)

logo abaixo (ou acima, não sei qual é a ordem) vou comentar umas partes do codigo que eu achei massa ou algumas sugestoes!

parabens pelo trabalho <3

}
return (
<div>
{postDetail !== null && <PostCard post={postDetail} hideComment />}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

acho que você esqueceu de passar as props certas pro PostCard, por isso ele renderizou em branco na parte de detalhes

import { useState, useEffect } from 'react'
import axios from 'axios'

export function useRequestData(url, initialState) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

esse hook aqui não está sendo utilizado, acho que é por causa daquela questao de atualizar a pagina né? o que você poderia ter feito pra não disperdiçar o hook era separar o que tá dentro do useEffect e jogar pra uma função dai chamar a função no useEffect e ali naquele return. massss sua solução pro problema de atualizar a pagina tbm foi muito boa!

<Route exact path='/cadastrar'>
<SignUpPage/>
</Route>
<Route exact path='/feed'>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

seria interessante deixar o feed como o / assim quando o usuario ta logado ele nao chega na pagina de login

import { useForm } from '../../hooks/useForm'

function FeedPage() {
const [isLoading, setLoading] = useState(false)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

adorei o loading que você fez

Comment on lines +17 to +19
useEffect(() => {
const authentication = useProtectPage // autenticação
}, [useProtectPage])
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

não entendi direito essa parte

}
try {
await axios.put(`${BASE_URL}/posts/${postId}/vote`, body, axiosConfig)
fetchPost();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

massa a solução pra recarregar a pagina!

Comment on lines +6 to +8
<div className="App">
<Router/>
</div>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

bem limpinho seu app! e o projeto em si tá bem componentizado!!

<ReactionsContainer disableSpacing>
<VotesContainer>
<IconButton onClick={handleVoteDownVote}>
<ArrowDownward />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

acho que dava pra dar uma reciclada na logica dos votos do comentario pro botão ficar pintadinho quando clica!

},
});

export const AvatarContainer = styled(Avatar)`
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

eita pq tem um export aqui?

}
}).then((response) => {
setPostDetail(response.data.post)
console.log(response)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

acho que vc esqueceu um console.log aqui

onChange={handleInputChange}
/>
<Button
type="submit"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

yessss! importantissimo esse type submit hehe

@phsarah phsarah merged commit 1997d6d into master Jan 4, 2021
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.

3 participants