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

Mensagens no app #39

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

Mensagens no app #39

wants to merge 19 commits into from

Conversation

viniciusvts
Copy link

Suavização da fonte na mensagem: 'Não foi dessa vez sorte na próxima', a mensagem também será exibida se já estiver perdido antes de fazer a substitutiva da ap1 ou ap2.

Adição da mensagem: 'Parabéns, você está passado' caso o aluno já esteja passado na disciplina com apenas duas provas realizadas. A mensagem também será exibida se já estiver passado antes de fazer a substitutiva da ap1 ou ap2.

…, a mensagem tambem será exibida se já estiver perdido antes de fazer a substitutiva ap1 ou ap2.

Adição da mensagem: 'Parabéns, você está passado' caso o aluno já esteja passado. A mensagem tambem será exibida se já estiver passado antes de fazer a substitutiva ap1 ou ap2.
Copy link
Owner

@RodolfoSilva RodolfoSilva left a comment

Choose a reason for hiding this comment

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

Bom trabalho @viniciusvts, ficou 💯0

Não foi dessa vez, sorte na próxima....
</ion-card-content>
<ion-card-content class="titulo" *ngIf="disciplina.notas.notaParaAP3 < 0 || disciplina.notas.notaParaAP2 < 0 || disciplina.notas.notaParaAP1 < 0" text-center>
Parabéns, já está passado.
Copy link
Owner

Choose a reason for hiding this comment

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

Acho que seria interessante a gente avisar ao cara que se ele for aprovado na AP1 e na AP2, mesmo estando aprovado, ele tem que fazer a AP3. Porque se ele não fizer a AP3 ele pode perder.

Copy link
Author

Choose a reason for hiding this comment

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

Feito!

<previsao-nota titulo="Necessário tirar na AP1" [nota]="disciplina.notas.notaParaAP1"></previsao-nota>
<previsao-nota titulo="Necessário tirar na AP2" [nota]="disciplina.notas.notaParaAP2"></previsao-nota>
<previsao-nota titulo="Necessário tirar na AP3" [nota]="disciplina.notas.notaParaAP3"></previsao-nota>
<previsao-nota *ngIf="disciplina.notas.notaParaAP1 > 0" titulo="Necessário tirar na AP1" [nota]="disciplina.notas.notaParaAP1"></previsao-nota>
Copy link
Owner

Choose a reason for hiding this comment

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

Bala 👍 ... Dá pra fazer um pequeno refactor nessa parte @viniciusvts, veja o component https://github.com/RodolfoSilva/DeVryCalc/blob/master/src/components/previsao-nota/previsao-nota.ts
Essa condição de verificar se é maior que 0 pode ser adicionado lá no componente de previsão de nota.

Copy link
Author

Choose a reason for hiding this comment

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

Com um boolean seria suficiente?

Copy link
Author

Choose a reason for hiding this comment

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

e usar um [hidden]="oBooleanAqui"
é suficiente?

Copy link
Owner

Choose a reason for hiding this comment

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

Pode ser também...

Mas o que eu estava sugerindo, era adicionar essa condição direto no component:

https://github.com/RodolfoSilva/DeVryCalc/blob/master/src/components/previsao-nota/previsao-nota.ts#L8

nota !== null && nota <= 10

Porque ele hoje já verifica se é diferente de nullo e menor ou igual a 10, no caso vc iria adicionar a verificação se nota for maior que 0.

Copy link
Author

Choose a reason for hiding this comment

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

Feito! Obrigado grande Jedi

@vaporwavie
Copy link
Collaborator

Boa, @viniciusvts! Fico feliz que tenha nos enviado o PR, o projeto é pra isso mesmo :D
+1 no comentário de Rodolfo com as alterações que ele sugeriu. Dá uma olhada no componente de previsão de notas, você pode tratar a condição que você colocou no if por lá, ao invés de usar logo na diretiva.

@viniciusvts
Copy link
Author

Feito!

@RodolfoSilva
Copy link
Owner

@viniciusvts tá tudo ok, só não aprovei o pull request ainda porque preciso arrumar o CI que tá quebrado. Assim que eu arrumar faço o merge. 😄

@viniciusvts
Copy link
Author

Valeu!

@RodolfoSilva
Copy link
Owner

Obrigado @viniciusvts irei migrar esse PR para a nova versão.

@RodolfoSilva
Copy link
Owner

@viniciusvts estava olhando as modificações que você fez na versão antiga 4ebacb9. Tentarei fazer isso na nova versão e no PR indicarei que foi você quem contribuiu. Mas caso você queira fazer, pode ficar a vontade. Vlw pela contribuição. 👍 😄

@viniciusvts
Copy link
Author

Flw, nao conheço o react
:(
irei dar um olhada depois

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants