Skip to content

Preparando revisão do código#1

Open
yann-melo wants to merge 1 commit into
rafaelarissa:mainfrom
yann-melo:main
Open

Preparando revisão do código#1
yann-melo wants to merge 1 commit into
rafaelarissa:mainfrom
yann-melo:main

Conversation

@yann-melo

Copy link
Copy Markdown

No description provided.

@vercel

vercel Bot commented Jan 13, 2022

Copy link
Copy Markdown

Someone is attempting to deploy a commit to a Personal Account owned by @rafaelarissa on Vercel.

@rafaelarissa first needs to authorize it.

@yann-melo yann-melo left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oi, Rafaela! Tudo bem? Aqui é Yann, trazendo seu feedback de código 🙂

Parabéns por mais uma entrega de projeto! 😄

Gostei bastante como implementou os elementos, teve uma lógica boa ao decorrer de todo o projeto e o proposito da correção foi verificar o uso das ementas de autodidatismo, no geral está de parabéns! 🎉

Abaixo notei alguns pontos que me chamaram a atenção no código.

Comment thread src/App.js
Comment on lines -25 to -27
localStorage.setItem("image", image);
// localStorage.setItem("name", name);
// localStorage.setItem("password", password);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Evite deixar comentários no código, caso não estejam sendo usados, o ideial é remove-los.

Comment on lines -11 to -31
<Footer>
<Nav><StyledLink to="/habitos">Hábitos</StyledLink></Nav>
<Div><StyledLink to="/hoje">
<CircularProgressbar
value={percentage}
text='Hoje'
background
backgroundPadding={6}
styles={buildStyles({
backgroundColor: "#3e98c7",
textColor: "#fff",
pathColor: "#fff",
trailColor: "transparent"
})}
/>
</StyledLink>
</Div>
<Nav><StyledLink to="/historico">Histórico</StyledLink></Nav>
</Footer>
)
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Um dos proposito do styled components é criar tags mais semânticas, Div e Nav poderiam ter sido apenas uma tag comum de div e nav já que não tinham a necessidade de serem semânticos. Pode estilizar dessa forma:

const Footer = styled.div`
     width: 100%;
     height: 70px;
     background-color: #FFFFFF;
     position: fixed;
     bottom: 0;
     display: flex;
     justify-content: space-between;
     align-items: center;
     padding: 22px 31px 26px 36px;
     div {
      ....
     };
     nav {
       ...
     }
     `;

Comment on lines -33 to -35
});
promise.catch(error => console.log(error.response));
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No front não é uma boa prática deixarmos console.log, o ideal é fazer algum tratamento no catch para que o usuário saiba que alguma coisa falhou. Acontece em outras partes do código também

const { image } = useContext(UserContext);
const { token } = useContext(TokenContext);
const { progress, setProgress } = useContext(UserContext);
let accomplished = 0

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Qual a necessidade dessa variável? 🤔

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.

1 participant