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

✨ [Homework] : Ajout du sélecteur de date et d'un panneau de réglage de l'onglet - 🚨 iOS ONLY 🚨 #158

Merged

Conversation

godetremy
Copy link
Contributor

Checklist d'avant pull request

Veuillez cocher toutes les cases applicables en remplaçant [ ] par [x].

  • Vous avez testé de build le projet avec vos modifications et ce build a réussi
  • Vous respectez les conventions de codage et de nommage du projet
  • Vous utilisez la tabulation pour l'indentation afin de maintenir un code lisible
  • Cette pull request n'est pas un duplicata d'une autre
  • Cette pull request est prête à être revue (review) et fusionnée (merge)
  • Il n'y a pas de TODO (aka des annotations pour du code manquant) dans vos modifications
  • Il n'y a pas d'erreurs de langue dans votre code (grammaire, vocabulaire, conjugaison, orthographe)
  • Les détails des changements ont été décrits ci-dessous
  • Cette pull-request n'est pas une "breaking-change" (des modifications qui vont entraîner la modification du fonctionnement de certaines fonctionnalités déjà existantes)

Changelogs proposés

Ajout du sélecteur de date et d'un panneau de réglage de l'onglet

Informations supplémentaires

🚨 iOS ONLY 🚨
Une pull request est à prévoir pour Android ! Cela est lié au fait qu'Android ne propose pas de sélecteur de date en inline.

@godetremy godetremy linked an issue Sep 5, 2024 that may be closed by this pull request
@godetremy godetremy added ✨ enhancement New feature or request 🎨 interface Element visible to the user 🚸 user experience UX related issues labels Sep 5, 2024
@Gabriel29306
Copy link
Contributor

Gabriel29306 commented Sep 5, 2024

Si on veut garder le même style pour Android, on va devoir recoder un calendrier entier. Ou sinon adapter la modale. À voir

@tom-theret
Copy link
Contributor

Pourquoi changer le style du calendrier ?

@Gabriel29306
Copy link
Contributor

Pourquoi changer le style du calendrier ?

Car comme dit plutôt, Android ne propose pas de style inline pour les calendriers.

@tom-theret
Copy link
Contributor

Je suis en train de faire un calendrier qui respecte le design de Papillon, je l’envoie cette après-midi !

@tom-theret
Copy link
Contributor

De plus, ce calendrier a été codé dans le mauvais fichier

@tom-theret
Copy link
Contributor

Simulator Screenshot - iPhone 15 Pro - 2024-09-06 at 14 22 05
Screenshot_1725625332

J'arrive !

@Gabriel29306
Copy link
Contributor

Gabriel29306 commented Sep 6, 2024

Simulator Screenshot - iPhone 15 Pro - 2024-09-06 at 14 22 05
Screenshot_1725625332

J'arrive !

J'aime bien, à quoi ressemble le menu défilant pour Android ?

@tom-theret
Copy link
Contributor

tom-theret commented Sep 6, 2024

Screenshot_1725625959

Mais c'est uniquement le temps de trouver une solution, j'en ai une en tête mais faut que je code dans la lib que j'utilise et c'est la merde.

@Gabriel29306
Copy link
Contributor

Bonne chance hein 🤷

@tom-theret
Copy link
Contributor

image
image

@Gabriel29306 je crois qu'on tien un truc là ?

@Gabriel29306
Copy link
Contributor

@Gabriel29306 je crois qu'on tien un truc là ?

Il y a juste l'énorme menu défilant qui est moche, à cause d'Android, sinon c'est bon.
Je sais plus qui fait /veut ça, mais c'est de reproduire le dropdown comme pour la sélection de compte. Mais ce serait à implémenter.

@tom-theret
Copy link
Contributor

@Gabriel29306 je crois qu'on tien un truc là ?

Il y a juste l'énorme menu défilant qui est moche, à cause d'Android, sinon c'est bon.
Je sais plus qui fait /veut ça, mais c'est de reproduire le dropdown comme pour la sélection de compte. Mais ce serait à implémenter.

Je vais essayer d'intégrer la roue sur Android

@Gabriel29306
Copy link
Contributor

Je sais pas si c'est possible

@oriionn
Copy link
Collaborator

oriionn commented Sep 6, 2024

image image

@Gabriel29306 je crois qu'on tien un truc là ?

Moyen convaincu par vos propositions, autant faire un truc propre sur iOS, puis attendre d'avoir un truc propre sur Android pour pouvoir l'intégrer...

@tom-theret
Copy link
Contributor

image image
@Gabriel29306 je crois qu'on tien un truc là ?

Moyen convaincu par vos propositions, autant faire un truc propre sur iOS, puis attendre d'avoir un truc propre sur Android pour pouvoir l'intégrer...

Faut encore que tes image soit visible

@Vilerio
Copy link
Contributor

Vilerio commented Sep 6, 2024

Niveau visuel, j'aime bien ce que Remy propose, bravo !
Mais d'un autre côté, cette PR, comme beaucoup d'autres, montrent que le choix du react native pour être "cross plateforme" n'est pas forcément le plus pertinent. (Mais cela ne dépend pas de vous bien sûr, ce n'est pas un reproche)

@tom-theret
Copy link
Contributor

Niveau visuel, j'aime bien, bravo ! Mais d'un autre côté, cette PR, comme beaucoup d'autres, montrent que le choix du react native pour être "cross plateforme" n'est pas forcément le plus pertinent. (Mais cela ne dépend pas de vous bien sûr, ce n'est pas un reproche)

Quel version aimes-tu ? Celle de @godetremy ou la mienne ?

Copy link
Contributor

@Vilerio Vilerio left a comment

Choose a reason for hiding this comment

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

Testé de mon côté de façon très basique, pas encore d'analyse détaillée du code ni des possibles erreurs.

@Vilerio
Copy link
Contributor

Vilerio commented Sep 6, 2024

Niveau visuel, j'aime bien, bravo ! Mais d'un autre côté, cette PR, comme beaucoup d'autres, montrent que le choix du react native pour être "cross plateforme" n'est pas forcément le plus pertinent. (Mais cela ne dépend pas de vous bien sûr, ce n'est pas un reproche)

Quel version aimes-tu ? Celle de @godetremy ou la mienne ?

J'ai edit mon commentaire sans voir ton message, sry
Je n'ai pas pu tester l'ergonomie de ton travail, je n'ai pas de smartphone sous Androïd à portée de main malheureusement.

@tom-theret
Copy link
Contributor

Je n'ai rien posté sur ce PR mais sur mon fork

@tryon-dev
Copy link
Contributor

image image

@Gabriel29306 je crois qu'on tien un truc là ?

Y'a volonté de nuire !!!

@tom-theret
Copy link
Contributor

Tout le monde kiffe le design il y a juste l'équipe Papillon qui n'est pas heureuse... à la fin c'est à se demander si c'est pas fait exprès ...

Et ces gens qui kiffent le design, ils sont dans la pièce avec nous ?

@Gabriel29306 @raphckrman

Dans les commentaires de la PR @Gabriel29306 n'a pas dit "kiffer" le design, il a même qualifié de moche la version sur Android.

Pas de chance maintenant on travail à deux sur une version Android mais bref...

@oriionn
Copy link
Collaborator

oriionn commented Sep 7, 2024

Ah oui ? Bah va dire à un utilisateur de choisir une semaine en sélectionnant un jour ... vachement logique

Au pire @godetremy, tu ne veux pas faire en sorte que quand l'utilisateur choisi le mercredi de la semaine, ça le scroll automatique sur le mercredi de cette dite semaine, comme ça, c'est logique selon lui. ;)

@oriionn
Copy link
Collaborator

oriionn commented Sep 7, 2024

Tout le monde kiffe le design il y a juste l'équipe Papillon qui n'est pas heureuse... à la fin c'est à se demander si c'est pas fait exprès ...

Et ces gens qui kiffent le design, ils sont dans la pièce avec nous ?

@Gabriel29306 @raphckrman

Dans les commentaires de la PR @Gabriel29306 n'a pas dit "kiffer" le design, il a même qualifié de moche la version sur Android.

Pas de chance maintenant on travail à deux sur une version Android mais bref...

Je vois pas le rapport, il a bien qualifié de "moche" la version sur Android, c'est un fait. Il n'a pas dit "kiffer" le design

@tom-theret
Copy link
Contributor

Ah oui ? Bah va dire à un utilisateur de choisir une semaine en sélectionnant un jour ... vachement logique

Au pire @godetremy, tu ne veux pas faire en sorte que quand l'utilisateur choisi le mercredi de la semaine, ça le scroll automatique sur le mercredi de cette dite semaine, comme ça, c'est logique selon lui. ;)

Mais connard tu comprends pas quoi dans le concept de demander à l'utilisateur l'intervalle de dates qui correspond à la semaine? c'est de la logique !

@oriionn
Copy link
Collaborator

oriionn commented Sep 7, 2024

ratio
image

@tom-theret
Copy link
Contributor

OUI....

@tom-theret
Copy link
Contributor

Bah écoute, je crois qu'on a vraiment pas la même logique mdr

@yannouuuu
Copy link
Member

Mais de toute façon ça parait simple : tant que les deux plateformes n'ont pas la même UX la PR ne sera pas acceptée. Papillon est multiplateforme mais elle reprend les mêmes logiques pour chaque OS. Ici le fait d'avoir une liste sur android ne correspond tout simplement pas avec le choix fait sur iOS (qui oui est meilleur).
Je la mets en draft pour l'instant comme j'ai vu que tu voulais refaire le "calendrier roue" à l'identique sur Android.

@yannouuuu yannouuuu marked this pull request as draft September 7, 2024 22:37
@tom-theret
Copy link
Contributor

Mais de toute façon ça parait simple : tant que les deux plateformes n'ont pas la même UX la PR ne sera pas acceptée. Papillon est multiplateforme mais elle reprend les mêmes logiques pour chaque OS. Ici le fait d'avoir une liste sur android ne correspond tout simplement pas avec le choix fait sur iOS (qui oui est meilleur). Je la mets en draft pour l'instant comme j'ai vu que tu voulais refaire le "calendrier roue" à l'identique sur Android.

Donc on ai d'accord que la wheel est une bonne idée ?

@oriionn
Copy link
Collaborator

oriionn commented Sep 7, 2024

Mais de toute façon ça parait simple : tant que les deux plateformes n'ont pas la même UX la PR ne sera pas acceptée. Papillon est multiplateforme mais elle reprend les mêmes logiques pour chaque OS. Ici le fait d'avoir une liste sur android ne correspond tout simplement pas avec le choix fait sur iOS (qui oui est meilleur). Je la mets en draft pour l'instant comme j'ai vu que tu voulais refaire le "calendrier roue" à l'identique sur Android.

Tu as mal compris la PR...

La PR propose de rajouter ça uniquement sur iOS :

Mais monsieur Tom n'est pas d'accord, et veut mettre ça :

iOS :

Android :

@oriionn oriionn marked this pull request as ready for review September 7, 2024 22:45
@tom-theret
Copy link
Contributor

Oui mais il attend mon interprétation sur android hors la 'est uniquement un proto, je n'ai rien PR

@tom-theret
Copy link
Contributor

Mais remet le en draft, merde

@oriionn
Copy link
Collaborator

oriionn commented Sep 7, 2024

Mais remet le en draft, merde

Bah nan, la PR est ready, il a mal compris dans son message, il a dit que la PR voulait mettre une wheel sur iOS et un select sur Android... hors là c'est ton idée et pas celle de la PR

@tom-theret
Copy link
Contributor

Mais remet le en draft, merde

Bah nan, la PR est ready, il a mal compris dans son message, il a dit que la PR voulait mettre une wheel sur iOS et un select sur Android... hors là c'est ton idée et pas celle de la PR

Bah oui mais il attend un truc de prod pour les 2 OS

@yannouuuu
Copy link
Member

mb j'avais mal capté le tournant de la PR et oui c'est bien le calendrier classique qu'il faudrait refaire à l'identique

@tom-theret
Copy link
Contributor

Bon bah faut vraiment aller se faire voir ...

@tom-theret
Copy link
Contributor

C'est vraiment pas logique votre histoire mais bref...

@TinAD17tin
Copy link

TinAD17tin commented Sep 8, 2024

Puedo faire le chiant ?
Voici ce que je préfère dans chaque solution :
Celle de Godetremy :

  • +À la PRONOTE
  • -Long pour bien choisir les bons jour mais :
    Contre intuitif si la sélection d’un jour donne la semaine
    Pose problème si l’utilisateur sélectionne deux semaine nan !?

Celle de Tom :

  • +Plus intuitif, si as un devoir pour dans deux semaines tu as juste à descendre deux fois.
  • -Version android délaissée, mais c’est déjà le cas de la version de Rémy

Par contre, je trouve qu’il faudrait mettre le bouton pour les devoirs faits ailleurs, car ça veut dire qu’il faudrait obligatoirement passer par le calendrier/la roue pour masquer les devoirs fait.

@TinAD17tin
Copy link

TinAD17tin commented Sep 8, 2024

Et en tant qu’utilisateur normal, qui va juste défilé la semaine d’après ou juste quelques semaines après, et qui n’est pas (encore) développeur, bah je préfère la version de Tom, mais c’est à vous de voir. Car je trouve que c’est plus rapide/compréhensible pour l’utilisateur lambda, qui ne va pas lire les détails écrit au dessus des calendriers + pas savoir c’est qu’elle pour qu’elle jour son devoir voir même qu’elle jour il est (même si le jour en question est coloré). Alors qu’à contrario un mec normal si il sait que c’est 2 semaines après, il ouvre la roue, descend 2 fois et hop c’est good, sans même savoir c’est quelle jour et quelle jour il est actuellement

@godetremy
Copy link
Contributor Author

Bon c'est pas compliqué, on va créer le composant PapillonCalendar ça serra plus simple visiblement...

@oriionn
Copy link
Collaborator

oriionn commented Sep 8, 2024

Et en tant qu’utilisateur normal, qui va juste défilé la semaine d’après ou juste quelques semaines après, et qui n’est pas (encore) développeur, bah je préfère la version de Tom, mais c’est à vous de voir. Car je trouve que c’est plus rapide/compréhensible pour l’utilisateur lambda, qui ne va pas lire les détails écrit au dessus des calendriers + pas savoir c’est qu’elle pour qu’elle jour son devoir voir même qu’elle jour il est (même si le jour en question est coloré). Alors qu’à contrario un mec normal si il sait que c’est 2 semaines après, il ouvre la roue, descend 2 fois et hop c’est good, sans même savoir c’est quelle jour et quelle jour il est actuellement

Je vois ce que tu veux dire, mais sa version comprend plusieurs désavantages :

  • Tu te perds rapidement si tu veux voir au-delà de 1 à 2 semaines de décalage (je me suis déjà perdu dans un select menu plus petit 💀)
  • Ça ne respecte pas trop le design de Papillon (comparé à celle de Rémy qui est 🤩)

Et par rapport à la simplicité de faire +2 semaines, on peut dire de même pour celle de Rémy, elle offre même de la précision selon moi.

@Yaya-Cout
Copy link

Le calendrier est quand-même beaucoup plus lisible que la roue, car tu as le mois visible facilement (c'est moins facile à lire sous forme de nombre), et que quand tu sais que tu as un travail à faire pour le 9 septembre, tu n'as pas à réfléchir à quelle semaine c'est en regardant les intervalles mais juste à cliquer sur le bon jour.
La roue donne juste des intervalles en vrac alors que le calendrier permet de mieux comprendre. Surtout si tu dois sélectionner un mois précis.
Si tu dois aller deux semaines plus tard, tu as juste à aller deux lignes en dessous.
Le problème initial est aussi que Android ne propose pas de calendrier inline, mais je ne vois pas pourquoi ça devrait être inline alors que sur l'écran d'à côté (devoirs), c'est un popup avec un calendrier natif, donc pourquoi faire quelque chose de différent au lieu d'unifier ?

@godetremy
Copy link
Contributor Author

Le calendrier est quand-même beaucoup plus lisible que la roue, car tu as le mois visible facilement (c'est moins facile à lire sous forme de nombre), et que quand tu sais que tu as un travail à faire pour le 9 septembre, tu n'as pas à réfléchir à quelle semaine c'est en regardant les intervalles mais juste à cliquer sur le bon jour. La roue donne juste des intervalles en vrac alors que le calendrier permet de mieux comprendre. Surtout si tu dois sélectionner un mois précis. Si tu dois aller deux semaines plus tard, tu as juste à aller deux lignes en dessous. Le problème initial est aussi que Android ne propose pas de calendrier inline, mais je ne vois pas pourquoi ça devrait être inline alors que sur l'écran d'à côté (devoirs), c'est un popup avec un calendrier natif, donc pourquoi faire quelque chose de différent au lieu d'unifier ?

Le problème c'est comment ajouter les paramètre de l'onglet sur android ?

@tom-theret
Copy link
Contributor

Et en tant qu’utilisateur normal, qui va juste défilé la semaine d’après ou juste quelques semaines après, et qui n’est pas (encore) développeur, bah je préfère la version de Tom, mais c’est à vous de voir. Car je trouve que c’est plus rapide/compréhensible pour l’utilisateur lambda, qui ne va pas lire les détails écrit au dessus des calendriers + pas savoir c’est qu’elle pour qu’elle jour son devoir voir même qu’elle jour il est (même si le jour en question est coloré). Alors qu’à contrario un mec normal si il sait que c’est 2 semaines après, il ouvre la roue, descend 2 fois et hop c’est good, sans même savoir c’est quelle jour et quelle jour il est actuellement

Je vois ce que tu veux dire, mais sa version comprend plusieurs désavantages :

  • Tu te perds rapidement si tu veux voir au-delà de 1 à 2 semaines de décalage (je me suis déjà perdu dans un select menu plus petit 💀)
  • Ça ne respecte pas trop le design de Papillon (comparé à celle de Rémy qui est 🤩)

Et par rapport à la simplicité de faire +2 semaines, on peut dire de même pour celle de Rémy, elle offre même de la précision selon moi.

@godetremy ne respecte pas le design Papillon à moins que c'est un changement surprise ?

@Rexxt
Copy link
Contributor

Rexxt commented Sep 8, 2024

Et en tant qu’utilisateur normal, qui va juste défilé la semaine d’après ou juste quelques semaines après, et qui n’est pas (encore) développeur, bah je préfère la version de Tom, mais c’est à vous de voir. Car je trouve que c’est plus rapide/compréhensible pour l’utilisateur lambda, qui ne va pas lire les détails écrit au dessus des calendriers + pas savoir c’est qu’elle pour qu’elle jour son devoir voir même qu’elle jour il est (même si le jour en question est coloré). Alors qu’à contrario un mec normal si il sait que c’est 2 semaines après, il ouvre la roue, descend 2 fois et hop c’est good, sans même savoir c’est quelle jour et quelle jour il est actuellement

bah après pour moi c'est une question d'UX localisée, à titre perso j'ai l'habitude de voir un calendrier assez lambda et ça m'a jamais dérangé plus que ça, à titre purement personel je m'axe plutôt sur la proposition de rémy

par contre tom je te trouve assez coincé du postérieur et ça me plaît pas

@Yaya-Cout
Copy link

Pronote utilise un calendrier, d'ailleurs (sur téléphone en tout cas).
Les jours de la semaine en cours sont encadrés et il suffit de cliquer sur l'un des jours de la semaine pour la sélectionner.

@godetremy godetremy changed the base branch from main to experiment/calendar September 8, 2024 14:23
@godetremy godetremy merged commit 4b11bfa into PapillonApp:experiment/calendar Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request 🎨 interface Element visible to the user 🚸 user experience UX related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PR OUVERTE][Feature]: Toggle devoir fait / non fait