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

feat: garder le bouton des devoirs non faits visible #452

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

codeuriii
Copy link
Contributor

@codeuriii codeuriii commented Dec 14, 2024

🚀 Nouvelle Pull Request

Proposez vos modifications pour améliorer Papillon

Informations importantes

Merci de vous référer à la documentation sur la contribution si vous avez des questions à propos des pull requests (https://gitbook.getpapillon.xyz/organisation/outils-internes/github)

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 d'une page de paramètre qui permet de choisir si on a envie de laisser le bouton checked square actif et/ou visible.

Informations supplémentaires

J'ai besoin d'aide pour les textes style le nom de la page dans les paramètres.

@codeuriii codeuriii requested a review from tryon-dev as a code owner December 14, 2024 09:47
@Bulgus
Copy link
Contributor

Bulgus commented Dec 14, 2024

Je trouve que c'est une bonne idée dans l'immédiat, mais (selon moi) :

  • Il y a un double affichage de l'état du settings (le bouton que tu as ajouté et le cercle en pointillé déjà présent). Il vaudrait mieux supprimer le cercle je pense.
    Capture d’écran 2024-12-14 à 14 40 19

  • Il y a un lag lors du clic sur le bouton de check, qui n'est pas présent lors du toggle en maintenant appuyé le numéro de semaine.

Enregistrement.de.l.ecran.2024-12-14.a.14.40.49.mov
  • Les deux settings rajoutés on leur place dans l'onglet "Onglets & Navigation" pour moi

(Questions de goûts)
Proposition de description du setting "Devoirs non-faits" : "Activer l'affichage par défaut des devoirs non-faits uniquement"
(Actuel : "Activer par défaut le fait d'afficher uniquement les devoirs non-faits")

Proposition de description du setting "Garder visible" : "Garder visible l'icône de bascule tous les devoirs / devoirs non-faits"
(Actuel : "Garder visible dans la barre du haut a côté du champ de recherche")


Edit : pense à mettre des screen dans la PR, c'est bien plus pratique pour review

@codeuriii
Copy link
Contributor Author

Ok je vais travailler dessus un grand merci

@codeuriii
Copy link
Contributor Author

je viens de vérifier et le lag ne provient pas de moi, il est présent dans papillon normal. Je crains que je ne puisse faire grand chose

@codeuriii
Copy link
Contributor Author

je pense que c'est mieux de laisser une page faite pour dans les settings

@codeuriii
Copy link
Contributor Author

dans le futur pourquoi pas rajouter d'autres choses pour personaliser telle que le rangement des devoirs faits en bas, ou le classement des devoirs en fonctions de l'emploie du temps

@codeuriii
Copy link
Contributor Author

bon je viens de voir que c'est déja implémenté mais tkt

@codeuriii
Copy link
Contributor Author

image

@codeuriii
Copy link
Contributor Author

image

@codeuriii
Copy link
Contributor Author

image
image

@Bulgus
Copy link
Contributor

Bulgus commented Dec 14, 2024

je viens de vérifier et le lag ne provient pas de moi, il est présent dans papillon normal. Je crains que je ne puisse faire grand chose

Mais pourquoi il n'y a aucun lag lorsque le toggle est effectué en maintenant la semaine ? Dans tous les cas il faut patch ça, peut importe d'où ça vient...

dans le futur pourquoi pas rajouter d'autres choses pour personaliser telle que le rangement des devoirs faits en bas, ou le classement des devoirs en fonctions de l'emploie du temps

J'aime bien l'idée du tri fait / non fait comme setting, je pense que la page de ces settings peut s'appeler "Devoirs" tout simplement

bon je viens de voir que c'est déja implémenté mais tkt

De quoi ?

image image

Nice parfait ! C'est plus clair que le rond d'avant en plus

Copy link
Contributor

@Kgeek33 Kgeek33 left a comment

Choose a reason for hiding this comment

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

Voilà ma review
Les fonctionnalités (en général) ajoutées sur Papillon sont très intéressantes, mais faudrait les mettre à l'avant sur la page d'accueil pour que l'utilisateur consulte au moins les nouveautés

Par exemple, les requêtes en arrière-plan, c'est bien la dernière chose que j'ai consulté sur la nouvelle version car on n'est pas au courant (7.6.0)

src/views/settings/SettingsCheck.tsx Outdated Show resolved Hide resolved
src/views/account/Homeworks/Homeworks.tsx Show resolved Hide resolved
@codeuriii
Copy link
Contributor Author

image

@codeuriii
Copy link
Contributor Author

le fait de ranger les devoirs a déja été implémenté

@Bulgus
Copy link
Contributor

Bulgus commented Dec 15, 2024

Bon, j'ai changé d'avis. Je viens de test avec les dernier commit @codeuriii, et c'est très bien lorsque l'icône est gardée affiché par défaut, mais lorsque qu'elle est masquée :

  • Il y a un doublon avec le cercle des semaines encore une fois
  • Et son accès n'est pas intuitif

Donc je suggère tout simplement de laisser tout le temps le bouton affiché, complètement supprimer le cercle, et ne laisser qu'un seul setting, celui de l'activation par défaut du tri.
Je trouve que l'interface avec le bouton "check" est très bien quand il est affiché.

Tu me dis ce que tu en penses, mais à mon avis d'un point de vu expérience utilisateur, le plus simple c'est ça !

@Kgeek33
Copy link
Contributor

Kgeek33 commented Dec 15, 2024

Donc je suggère tout simplement de laisser tout le temps le bouton affiché, complètement supprimer le cercle, et ne laisser qu'un seul setting, celui de l'activation par défaut du tri.
Je trouve que l'interface avec le bouton "check" est très bien quand il est affiché.

Je vais tester tout à l'heure les derniers commits mais c'est vrai que je suis d'accord, c'est plus efficace avec le paramètre affiché par défaut.

Et ça revient à mon commentaire précédent, il y a plein de fonctionnalités sur Papillon mais le problème, c'est que l'utilisateur doit fouiller pour découvrir ces fonctionnalités
En rapport avec la page des devoirs, si je connaissais pas cette fonctionnalité, j'aurai même pas eu l'idée de cliquer sur "Semaine [...]"

@Bulgus
Copy link
Contributor

Bulgus commented Dec 15, 2024

Donc je suggère tout simplement de laisser tout le temps le bouton affiché, complètement supprimer le cercle, et ne laisser qu'un seul setting, celui de l'activation par défaut du tri.
Je trouve que l'interface avec le bouton "check" est très bien quand il est affiché.

Je vais tester tout à l'heure les derniers commits mais c'est vrai que je suis d'accord, c'est plus efficace avec le paramètre affiché par défaut.

Et ça revient à mon commentaire précédent, il y a plein de fonctionnalités sur Papillon mais le problème, c'est que l'utilisateur doit fouiller pour découvrir ces fonctionnalités En rapport avec la page des devoirs, si je connaissais pas cette fonctionnalité, j'aurai même pas eu l'idée de cliquer sur "Semaine [...]"

Oui voilà, la même, j'ai mis du temps à savoir que ça existait
donc pour moi le mieux c'est que ça soit là tout le temps, c'est intuitif comme icône on comprend !

@codeuriii
Copy link
Contributor Author

todo
Je vais laisser afficher le bouton par défaut genre cocher dans le menu vrai par défaut
ensuite je pense laisser le cercle dashed car il montre que c'est coché si c'est caché (de plus on le verra pas souvent car il sera caché par défaut)
fix le long press pour aller dans les parametres

@Kgeek33
Copy link
Contributor

Kgeek33 commented Dec 15, 2024

Oui voilà, la même, j'ai mis du temps à savoir que ça existait

À travailler ça dans une pr, choisir à chaque fois qu'on va dans la page d'accueil, faire un random et afficher une fonctionnalité pas évidente à trouver
Par exemple : Titre : "Le savais-tu?" et Message : "En cliquant sur le graphique de l'évolution de ta moyenne générale, ça va afficher les moyennes générales minimales et maximales (théoriques)" (tu t'en souviens qu'on voulait supprimer cette fonctionnalité @Bulgus 😂)

donc pour moi le mieux c'est que ça soit là tout le temps, c'est intuitif comme icône on comprend !

Oui je suis d'accord !

@codeuriii
Copy link
Contributor Author

codeuriii commented Dec 15, 2024

todo

  • Je vais laisser afficher le bouton par défaut genre cocher dans le menu vrai par défaut
  • ensuite je pense laisser le cercle dashed car il montre que c'est coché si c'est caché (de plus on le verra pas souvent car il sera caché par défaut)
  • fix le long press pour aller dans les parametres

Copy link
Contributor

@Kgeek33 Kgeek33 left a comment

Choose a reason for hiding this comment

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

Juste ça pour que onLongPress fonctionne et soit pas trop chiant et c'est parfait !

Co-authored-by: 𝕂𝕪𝕝𝕚𝕒𝕟 <[email protected]>
Copy link
Contributor

@Kgeek33 Kgeek33 left a comment

Choose a reason for hiding this comment

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

Parfait, LGTM 👍

@yannouuuu yannouuuu changed the title feature: garder le bouton des devoirs non faits visible feat: garder le bouton des devoirs non faits visible Dec 23, 2024
@yannouuuu yannouuuu added the ✨ enhancement New feature or request label Dec 23, 2024
@ecnivtwelve
Copy link
Contributor

je recall @toi-et-moi pour son avis niveau UX

@codeuriii
Copy link
Contributor Author

Je me permet de relancer poliment

Copy link
Contributor

@tryon-dev tryon-dev left a comment

Choose a reason for hiding this comment

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

Quel est le but de la PR ?, réexplique moi stp

@codeuriii
Copy link
Contributor Author

Cette pr rajoute une page dans les paramètres et permet de garder le bouton fais visible et/ou activé par défaut, comme sur l'application Pronote. De plus elle ne supprime rien et c'est possible de checker les deux paramètres pour désactiver la visibilité par défaut et l'activation par défaut. Pour finir, étant sur mobile je ne vois pas tes changements demandés et il est minuit quarantes chez moi donc j'aimerais bien que tu fasse un résumé rapide, merciiii

@godetremy
Copy link
Contributor

Note

Cette review est le résultat de la concertation de toute l'équipe Papillon.

Expérience utilisateur

L'idée semble intéressante, mais l'UX nous semblent trop complexe. (Nous même avons eu du mal à comprendre)

Le plus simple serait de garder le dernière état du check des devoirs. Pour ce qui est de la fonction de l'afficher constamment, nous ne trouvons pas ça intéressant.

Interface utilisateur

Disparition du texte "Recherche"

Simulator Screenshot - iPhone 16 - 2025-01-11 at 21 42 35
Disparu dans l'univers

@codeuriii
Copy link
Contributor Author

codeuriii commented Jan 12, 2025

Je sais mais si je le laisse il est coupé à la fin sur les plus petits appareils
De plus, vous pouvez le faire réapparaitre en désactivant l'affichage du bouton dans les paramètres

@codeuriii
Copy link
Contributor Author

codeuriii commented Jan 12, 2025

Todo:

  • Remettre le texte rechercher (même si il se coupe)
  • Stocker le dernier état du bouton
  • Supprimer le paramètre d'activation par défaut sur la page
  • Déplacer l'autre paramètre dans une autre page
  • Supprimer la page créée
  • Trouver comment récupérer l'état d'activation du bouton

@codeuriii codeuriii requested a review from tryon-dev January 16, 2025 16:44
@codeuriii
Copy link
Contributor Author

J'ai fini j'ai hâte de ma review

@codeuriii
Copy link
Contributor Author

Je comprend pas le eslint, j'ai pas touché a ce code mais il y a une erreur dans src/views/account/Grades/Subjects/Subjects.tsx

@JyhuKo
Copy link
Contributor

JyhuKo commented Jan 21, 2025

je pense c'est pcq le lint du main est cassé , ça a été édit ya 2h

@codeuriii
Copy link
Contributor Author

quand je vais voir en local ca me dit que il peut pas importer jsp quoi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants