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

added button like and dislike #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ocarlic
Copy link

@ocarlic ocarlic commented Oct 11, 2019

#17

@ocarlic ocarlic changed the title add button like and deslike added button like and deslike Oct 11, 2019
Copy link
Contributor

@Pliavi Pliavi left a comment

Choose a reason for hiding this comment

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

Nem sabia que qualquer um podia dar review, mas fiz algumas anotações aqui se puder dar uma olhada com carinho @ocarlic ^^

@@ -0,0 +1,48 @@
<script>
import IconCollab from '../components/IconCollab.svelte'
let deslikes = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

A tradução correta de "não gostei" é "dislike"

</script>

<style>
button {
Copy link
Contributor

Choose a reason for hiding this comment

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

Evite usar tags diretamente, sempre tenha preferencia pelo uso de classes para manter a mantenabilidade e a facilidade da leitura.

E não sei ao certo sobre o scoped do Svelte, se ele adiciona escopo em tags diretamente, o que pode gerar conflitos com outros componentes. Apesar que acho que o Svelte também coloca escopo.

Recomendo sempre utilizar o nome do componente na tag raiz, assim como os demais componentes presentes no repositório.
Neste caso a classe .dislike-button no CSS
E no HTML:

<button on:click={handleClick} class="dislike-button"> 

align-items: center;
}

button > div > p {
Copy link
Contributor

Choose a reason for hiding this comment

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

Este é um exemplo, div e p podem ser qualquer coisa, o que uma alteração futura no HTML do componente pode gerar problemas e ter que reler todo lugar onde div está contido

</script>

<style>
button {
Copy link
Contributor

Choose a reason for hiding this comment

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

O mesmo caso dos comentários sobre os usos de classes vale para este componente também

@ocarlic
Copy link
Author

ocarlic commented Oct 12, 2019

Nem sabia que qualquer um podia dar review, mas fiz algumas anotações aqui se puder dar uma olhada com carinho @ocarlic ^^

Eu também não sabia, vou fazer as correções dessas anotações que você comentou, valeu @Pliavi

@ocarlic ocarlic changed the title added button like and deslike added button like and dislike Oct 12, 2019
Copy link
Member

@marcobrunodev marcobrunodev left a comment

Choose a reason for hiding this comment

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

Como estamos usando GitFlow você sempre tem que criar sua a feature branch pela develop não pela maste. E quando abrir a PR você também tem que enviar ela para develop. Por favor, faça essa alterações.

@ocarlic
Copy link
Author

ocarlic commented Oct 17, 2019

Como estamos usando GitFlow você sempre tem que criar sua a feature branch pela develop não pela maste. E quando abrir a PR você também tem que enviar ela para develop. Por favor, faça essa alterações.

Ok, irei fazer a correção @MarcoBrunoBR

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