-
Notifications
You must be signed in to change notification settings - Fork 13
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
Notifications natives #1671
base: develop
Are you sure you want to change the base?
Notifications natives #1671
Conversation
59dcb26
to
4663e85
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pour la PR notifs :
- Je viens de pousser fix car le gen_assets passe pas
- Si ca marche > faire : Notification.requestPermission() pour avoir demande du navigateur
- donc il manque un truc car j'ai du request a la main ou parce que j'avais ptre deja un jour dis non
- du coup si on dit non on a pu rien et pu aucun retour gui donc on sait pas ce qui se passe
- les notifs sont par dessus tout appli ouverte sur le PC
- l'icone erreur bof lol (mais je peux m'en occuper)
- les notifs ne s'empilent pas donc si tu en 15 d'affilé (restore) elles s'affichent les une apres les autres donc ca prend 10min
je suis pas pour meme si le fond d'une dep en moins ok
Je n'ai pas testé, mais se séparer des notifs existantes je suis contre. Ajouter ceci en option éventuellement. |
4663e85
to
9e03afd
Compare
O_o t'as pas testé et t'es contre ? :) |
9e03afd
to
b4bb9e1
Compare
Fix gen_assets
b4bb9e1
to
b392a45
Compare
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Je suis pour cette feature, on se débarasse d'une lib en plus. |
Proposition de notifications natives
On supprime une dépendance et moins de lignes.
Pour tester, faites afficher un message
Ne pas merger sans un accord concret :)