-
-
Notifications
You must be signed in to change notification settings - Fork 88
feat: Effets visuels plus visibles sur les cours #651
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
base: main
Are you sure you want to change the base?
Conversation
Ajout d'effets visuels plus visibles sur les cours déplacés, modifiés et exceptionnels
WalkthroughAjoute un statut de cours enrichi (label, canceled, color, icon) produit dans Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
locales/pt.json (1)
118-340: Fichier pt.json corrompu : contenu entièrement en français au lieu du portugaisConfirmé. Le fichier contient du texte français dans les sections destinées à la traduction portugaise. À partir de la ligne 118 et au moins jusqu'à la ligne 350, le contenu est en français (exemple : "Tu n'as aucun cours restant de prévu aujourd'hui" au lieu d'une traduction portugaise). Cette corruption affecte environ 18+ traductions et dégrade complètement l'expérience utilisateur pour les utilisateurs sélectionnant le portugais.
Le fichier doit être entièrement traduit en portugais pour respecter la sélection linguistique de l'utilisateur.
🧹 Nitpick comments (1)
app/(tabs)/calendar/components/CalendarDay.tsx (1)
148-157: Suggestion : extraire les valeurs codées en dur dans des constantes.Les couleurs hexadécimales (
#e07a10,#5e21d9,#bfa51d) et les noms d'icônes ("Pen","Archive","Sparkles") sont dupliqués et codés en dur. De plus, les mêmes conditions sont évaluées trois fois (pour label, color, et icon).🔎 Refactorisation proposée
Créez des constantes et une fonction helper pour réduire la duplication :
// Au début du fichier ou dans un fichier de constantes const COURSE_STATUS_CONFIG = { MODIFIED: { color: "#e07a10", icon: "Pen" as const, translationKey: "Modified_Course" }, MOVED: { color: "#5e21d9", icon: "Archive" as const, translationKey: "Moved_Course" }, EXCEPTIONAL: { color: "#bfa51d", icon: "Sparkles" as const, translationKey: "Exceptional_Course" } } as const; // Fonction helper function getCourseStatusConfig(item: SharedCourse) { if (item.status === CourseStatus.EDITED || item.customStatus?.toLowerCase().includes("modifié")) { return COURSE_STATUS_CONFIG.MODIFIED; } if (item.customStatus?.toLowerCase().includes("déplacé")) { return COURSE_STATUS_CONFIG.MOVED; } if (item.customStatus?.toLowerCase().includes("exceptionnel")) { return COURSE_STATUS_CONFIG.EXCEPTIONAL; } return null; } // Utilisation const statusConfig = getCourseStatusConfig(item); const defaultLabel = item.customStatus || getStatusText(item.status); <Course status={{ label: statusConfig ? t(statusConfig.translationKey) : defaultLabel, canceled: item.status === CourseStatus.CANCELED, color: statusConfig?.color, icon: statusConfig?.icon }} // ... />Cela élimine la duplication, améliore la maintenabilité et facilite l'ajout de nouveaux statuts.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/(tabs)/calendar/components/CalendarDay.tsxlocales/br.jsonlocales/de.jsonlocales/en.jsonlocales/es.jsonlocales/fr.jsonlocales/pt.jsonlocales/tr.jsonui/components/Course.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
ui/components/Course.tsx (2)
utils/native/AnimatedNavigator.ts (1)
Stack(14-19)utils/adjustColor.ts (1)
adjust(3-43)
app/(tabs)/calendar/components/CalendarDay.tsx (1)
app/(tabs)/calendar/index.tsx (1)
getStatusText(110-123)
🔇 Additional comments (8)
locales/es.json (1)
84-86: LGTM!Les traductions espagnoles sont grammaticalement correctes et cohérentes. L'accord de genre est respecté (« clase » étant féminin, les adjectifs sont correctement accordés).
locales/pt.json (1)
84-86: Les traductions portugaises sont correctes.Les trois nouvelles clés de traduction sont correctement traduites en portugais avec un accord grammatical approprié.
locales/br.json (1)
84-86: Traductions ajoutées de manière cohérente.Les trois nouvelles clés de traduction sont ajoutées de manière cohérente avec la structure du fichier. Bien que je ne puisse pas vérifier l'exactitude linguistique du breton, le placement et la structure sont corrects.
locales/tr.json (1)
84-86: LGTM!Les traductions turques sont ajoutées de manière cohérente et la structure est correcte. Le placement après
Canceled_Courseest logique.locales/en.json (1)
84-86: Les traductions anglaises sont correctes et cohérentes.Les nouvelles clés de traduction suivent la convention de nommage existante et sont placées logiquement après
Canceled_Course. Les libellés sont clairs et appropriés.locales/de.json (1)
83-86: Les traductions allemandes sont cohérentes et améliorent l'uniformité.Le changement de "Unterricht abgesagt" vers "Kurs abgesagt" améliore la cohérence avec les autres traductions du fichier qui utilisent "Kurs". Les nouvelles traductions pour les cours déplacés, modifiés et exceptionnels sont appropriées.
ui/components/Course.tsx (2)
28-29: Extension du type status appropriée.L'ajout des champs optionnels
coloreticonau typestatuspermet une personnalisation flexible des statuts de cours tout en maintenant la rétrocompatibilité (champs optionnels).
207-244: La logique de rendu des statuts colorés est cohérente.Les modifications permettent aux statuts personnalisés (déplacé, modifié, exceptionnel) d'être rendus avec la même emphase visuelle que les cours annulés. La logique applique de manière cohérente les couleurs au texte, aux icônes, aux arrière-plans et aux bordures.
Also applies to: 260-260, 354-354
raphckrman
left a comment
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.
Hello, merci de ta Pull Request, l'idée est vraiment pas mauvaise c'est vrai qu'on voit pas vraiment le status du cours mais à ce moment il faudrait éviter de le mettre 2 fois (au dessus dans le cas de ta PR et plus a l'intérieur du cours).
Je suis pas très fan de la selection des couleurs et du fait que ce soit des couleurs hardcoded, ça fait très arc-en-ciel et ça impacte un peu la lisibilité.
Voilà voilà mes quelques suggestions 👍🏼
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
ui/components/Course.tsx (1)
334-334: Espace superflu avant la balise fermante.Il y a un espace avant
>dans</Stack >qui pourrait être supprimé pour la cohérence du formatage.🔎 Correction suggérée
- </Stack > + </Stack>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/(tabs)/calendar/components/CalendarDay.tsxui/components/Course.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- app/(tabs)/calendar/components/CalendarDay.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
ui/components/Course.tsx (2)
utils/native/AnimatedNavigator.ts (1)
Stack(14-19)utils/adjustColor.ts (1)
adjust(3-43)
🔇 Additional comments (5)
ui/components/Course.tsx (5)
1-1: Excellente amélioration de la sécurité des types !L'importation de
IconNamesrésout directement le problème de sécurité de type signalé dans les commentaires précédents. L'utilisation destatus.iconest désormais correctement typée sans recourir àas any.
28-29: LGTM !L'extension de l'interface
statusaveccoloreticontypé correctement permet de supporter les nouveaux indicateurs visuels tout en maintenant la rétrocompatibilité grâce aux propriétés optionnelles.
260-260: LGTM !La logique applique correctement un fond neutre (
colors.card) pour les cours avec statut spécial, permettant à l'en-tête coloré de se démarquer visuellement.
309-326: Confirmer l'intention de la logique conditionnelle.La condition
status.label && !status.colorà la ligne 311 empêche l'affichage du label de statut dans la zone de contenu principal lorsqu'une couleur est définie.Cela signifie que pour les cours déplacés/modifiés/exceptionnels (qui ont
status.color), le label n'apparaît que dans l'en-tête (viarenderStatus), tandis que d'autres statuts sans couleur afficheraient le label sous forme de badge.Veuillez confirmer que ce comportement est intentionnel et correspond à la maquette prévue.
354-354: LGTM !La logique d'arrière-plan gère correctement les cas où
status.colorest défini ou non, avec un repli approprié vers la couleur rouge pour les cours annulés. La fonctionadjust()dispose également de protections internes contre les valeurs invalides.
| const statusText = item.customStatus?.toLowerCase() || ""; | ||
| const isEdited = item.status === CourseStatus.EDITED || statusText.includes("modifié"); | ||
| const isMoved = statusText.includes("déplacé"); | ||
| const isExceptional = statusText.includes("exceptionnel"); | ||
|
|
||
| let statusColor = undefined; | ||
| let statusIcon: IconNames | undefined = undefined; | ||
|
|
||
| if (isEdited) { | ||
| statusColor = colors.primary; | ||
| statusIcon = "Pen"; | ||
| } | ||
| if (isMoved) { | ||
| statusColor = colors.primary; | ||
| statusIcon = "Archive"; | ||
| } | ||
| if (isExceptional) { | ||
| statusColor = colors.primary; | ||
| statusIcon = "Sparkles"; | ||
| } |
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.
Hello ! Le rendu est propre, par contre la logique dans le renderItem c'est un peu fragile.
Avoir les strings "modifié", "déplacé", etc. en dur c'est risqué si ça change côté API. Pareil pour les if à la suite, si un cours a plusieurs statuts, le dernier l'emporte et écrase les précédents, ce qui peut créer des effets de bord.
Essaie de sortir tout ça dans une fonction à part et de mettre les strings dans des constantes, ça sera plus clean.
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.
Complètement d'accord avec Maël sur ce point, sinon le reste me semble clean
Règles de contribution
Caution
Afin de garantir une application stable et pérenne dans le temps, nous t'invitons à vérifier que tu as bien respecté les règles de contribution. Sans cela, ta Pull Request ne pourra pas être examinée.
Résumé des changements
Note
Ajout d'effets visuels plus visibles sur les cours déplacés, modifiés et exceptionnels :
• Les cours déplacés ont désormais une en-tête, comme les cours annulés, avec des couleurs chacune
• Ajout d’icônes cohérentes pour les cours exceptionnels, déplacés et modifiés
• Ajout des traductions pour les Cours déplacé, modifié et exceptionnel dans toutes les langues
Capture(s) d'écran
Informations supplémentaires
Summary by CodeRabbit
Nouvelles fonctionnalités
Localisation
✏️ Tip: You can customize this high-level summary in your review settings.