Agenda: affichage multi-tuiles#831
Conversation
a6c3102 to
3d7466f
Compare
3d7466f to
2b1251c
Compare
magopian
left a comment
There was a problem hiding this comment.
Gros boulot, propre, merci ! 💯 🙏
Quelques remarques dont une sur une question de sécurité.
Pour info, cliquer sur "fermer" dans les paramètres de choix de zone devrait ramener vers l'agenda. Pour le moment, ça ramène vers la page de paramètres, qui ne permet qu'un retour vers l'accueil avec la flèche en haut à gauche. Peut-être que ça pourrait être réglé dans une autre PR ?
| if (!userStore.connected?.isSchoolHolidayConcernedByPreferences(holiday)) { | ||
| // don't display OTV if holiday match user preferences | ||
| return null; | ||
| } | ||
| if (startDate > date) { | ||
| // don't display OTV too early, only display them when they're close enough to their associated holiday | ||
| return null; |
There was a problem hiding this comment.
Peut-être remonter plus tôt ces deux conditions, pour les grouper avec les autres "return null" de createOTVItem ?
There was a problem hiding this comment.
Hum non, car dans ces cas-ci je veux quand même notifier le back de la scheduled notification:
- si la zone n'est pas sélectionnée, mais que la période de vacances matche l'adresse du user, le notifier quand même à J-3 semaines
- si la période de vacances n'est pas encore entamée, on n'affiche pas l'OTV mais on crée quand même la notif pour J-3 semaines
Les autres cas plus haut sont des cas où on ne veut jamais avoir de scheduled notification.
There was a problem hiding this comment.
Ca marche, merci !
c'est l'objectif de la PR avec la modale (#847) |
2b1251c to
cfa980a
Compare
cfa980a to
635f1d9
Compare
magopian
left a comment
There was a problem hiding this comment.
avec les dernières modifications ça me paraît impeccable, merci !
🚢
5a8d99f to
a03df6f
Compare
a03df6f to
690b93d
Compare
fixes #802