-
-
Notifications
You must be signed in to change notification settings - Fork 73
feat: Better About Page #660
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?
feat: Better About Page #660
Conversation
|
Warning Rate limit exceeded@Dev-LeChacal has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 8 minutes and 50 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughRefactor de la page « À propos » : l'export Changes
Sequence Diagram(s)(aucun diagramme généré — changements principalement refactor/UI et localisation) Estimated code review effort🎯 3 (Modéré) | ⏱️ ~20 minutes Possibly related PRs
Suggested labelsarea: i18n 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 0
🧹 Nitpick comments (3)
app/(settings)/about.tsx (3)
94-99: Envisager une icône plus explicite pour le signalement de bugs.L'icône
Infoest utilisée pour le lien de signalement de bugs. Une icône commeBug,Warning, ouAlertCircle(si disponible dans Papicons) serait plus intuitive et mieux alignée avec la fonction de signalement d'erreurs.
123-149: Améliorer les icônes et la cohérence des liens sociaux.Quelques suggestions d'amélioration :
Icônes génériques : Tous les liens sociaux utilisent la même icône
Link. Si Papicons propose des icônes spécifiques aux plateformes (Twitter/X, Instagram, TikTok, YouTube, LinkedIn), leur utilisation améliorerait la reconnaissance visuelle.Capitalisation de TikTok (ligne 135) : Le nom de la marque devrait être "TikTok" avec un T et un K majuscules.
URL LinkedIn (ligne 147) : L'URL
fr.linkedin.comredirige vers la version française. Privilégiezwww.linkedin.compour un accès international cohérent.♻️ Corrections suggérées
{ - title: "Tiktok", + title: "TikTok", leading: <Papicons name="Link" />, onPress: () => Linking.openURL('https://tiktok.com/@thepapillonapp'), }, { title: "LinkedIn", leading: <Papicons name="Link" />, - onPress: () => Linking.openURL('https://fr.linkedin.com/company/papillonbzh'), + onPress: () => Linking.openURL('https://www.linkedin.com/company/papillonbzh'), },
227-308: Opportunité de réduire la duplication de code.Les trois blocs de rendu pour
CommunityLinks,DevelopperLinks, etSocialLinks(lignes 227-308) partagent une structure très similaire. Vous pourriez extraire un composant réutilisable pour améliorer la maintenabilité du code.💡 Exemple de composant réutilisable
Vous pourriez créer un helper comme celui-ci :
const renderLinkList = (items: typeof CommunityLinks, showDescription = true) => ( <List> {items.map((item, index) => ( <Item key={index} onPress={item.onPress}> <Leading> <Icon>{item.leading}</Icon> </Leading> <Typography variant="title">{item.title}</Typography> {showDescription && item.description && ( <Typography variant="caption" color="secondary"> {item.description} </Typography> )} <Trailing> <Icon> <Papicons name="ChevronRight" /> </Icon> </Trailing> </Item> ))} </List> ); // Utilisation : {renderLinkList(CommunityLinks)} {renderLinkList(DevelopperLinks)} {renderLinkList(SocialLinks, false)}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/(settings)/about.tsxlocales/fr.json
🔇 Additional comments (4)
locales/fr.json (1)
357-364: Les nouvelles traductions sont bien formulées.Les six nouvelles clés de traduction ajoutées sont grammaticalement correctes, cohérentes avec les conventions existantes du fichier, et correctement utilisées dans la page À propos. Bon travail !
app/(settings)/about.tsx (3)
153-167: La logique du mode développeur fonctionne correctement.Le mécanisme de détection des 8 taps pour activer/désactiver le mode développeur est bien implémenté. La réinitialisation du compteur et la gestion de l'état via le store sont correctes.
184-188: Masquage de l'indicateur de défilement conforme aux objectifs.L'ajout de
showsVerticalScrollIndicator={false}correspond bien à l'objectif du PR de supprimer l'indicateur de défilement vertical.
17-60: Pas de changement d'export à vérifier.L'export est déjà nommé
Team(singulier) et nonTeams. De plus, aucun fichier dans le projet n'importe cet export, donc il n'y a pas de risque de rupture de dépendances. La préoccupation soulevée ne s'applique pas.
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/(settings)/about.tsx (1)
102-115: Corriger la faute de frappe dans le nom de la variable.Le nom
DevelopperLinkscontient une faute : il devrait s'écrireDeveloperLinks(avec un seul 'p'). Bien qu'en français "développeur" prenne deux 'p', les identificateurs de code suivent généralement la convention anglaise.✏️ Correction proposée
- const DevelopperLinks = [ + const DeveloperLinks = [ { title: t("Settings_About_Crowdin"), description: t("Settings_About_Crowdin_Description"), leading: <Papicons name="Crown" />, onPress: () => Linking.openURL('https://crowdin.com/project/papillonapp'), }, { title: t("Settings_About_Github"), description: t("Settings_About_Github_Description"), leading: <Papicons name="Ghost" />, onPress: () => Linking.openURL('https://github.com/PapillonApp/Papillon'), }, ];N'oubliez pas de mettre à jour également la référence à la ligne 251 :
<List> - {DevelopperLinks.map((item, index) => ( + {DeveloperLinks.map((item, index) => ( <Item
🧹 Nitpick comments (3)
app/(settings)/about.tsx (3)
94-99: Envisager une icône plus explicite pour le signalement de bug.L'icône
Infoest assez générique. Une icône commeBug,Alert, ouWarningserait plus appropriée visuellement pour un lien de signalement de bug.💡 Proposition d'amélioration de l'icône
{ title: t("Settings_About_Issue"), description: t("Settings_About_Issue_Description"), - leading: <Papicons name="Info" />, + leading: <Papicons name="Bug" />, onPress: () => Linking.openURL('https://github.com/PapillonApp/Papillon/issues'), },Remarque : Vérifiez que l'icône
Bugexiste dans@getpapillon/papicons. Sinon, utilisezAlertouWarning.
117-143: Supprimer le code commenté plutôt que de le conserver.Le code
SocialLinksest entièrement commenté. Il est préférable de le supprimer complètement plutôt que de le laisser en commentaires, car Git conserve l'historique en cas de besoin futur. Le code commenté réduit la lisibilité du fichier.🧹 Nettoyage proposé
Supprimez les lignes 117-143 et 279-302 entièrement. Si ces liens doivent être réintégrés ultérieurement, ils seront récupérables via l'historique Git.
Also applies to: 279-302
192-277: Envisager une abstraction pour réduire la duplication de code.Les trois blocs de rendu (Team, CommunityLinks, DeveloperLinks) suivent une structure très similaire. Bien que fonctionnelle, cette approche présente une duplication importante.
♻️ Suggestion d'abstraction
Vous pourriez créer un composant réutilisable ou une fonction d'aide pour le rendu :
const renderItemsList = (items: any[]) => ( <List> {items.map((item, index) => ( <Item key={index} onPress={item.onPress}> <Leading> <Icon>{item.leading}</Icon> </Leading> <Typography variant="title">{item.title}</Typography> {item.description && ( <Typography variant="caption" color="secondary"> {item.description} </Typography> )} <Trailing> <Icon> <Papicons name="ChevronRight" /> </Icon> </Trailing> </Item> ))} </List> ); // Utilisation : {renderItemsList(Team)} {renderItemsList(CommunityLinks)} {renderItemsList(DeveloperLinks)}Cela dit, la structure actuelle reste lisible et maintenable pour cette échelle.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/(settings)/about.tsxlocales/fr.json
🚧 Files skipped from review as they are similar to previous changes (1)
- locales/fr.json
🧰 Additional context used
🧬 Code graph analysis (1)
app/(settings)/about.tsx (3)
utils/github/contributors.ts (1)
getContributors(9-19)stores/magic/types.ts (1)
Item(8-11)utils/chats/initials.ts (1)
getInitials(1-10)
🔇 Additional comments (4)
app/(settings)/about.tsx (4)
17-60: Renommage cohérent deTeamsenTeam.Le passage au singulier est plus approprié sémantiquement. La structure des données reste correcte et cohérente.
73-73: Mise à jour cohérente avec le renommage.La référence à
Teamest correctement mise à jour, et la logique de filtrage reste intacte.
147-161: La logique de basculement du mode développeur est correcte.L'implémentation d'un easter egg (8 taps) pour activer/désactiver le mode développeur fonctionne correctement et suit un pattern courant.
179-181: Ajustements UI appropriés.La réduction de l'espacement (
gap: 10) et le masquage de l'indicateur de scroll vertical améliorent l'apparence de la page conformément aux objectifs du PR.
| // const SocialLinks = [ | ||
| // { | ||
| // title: "X (Twitter)", | ||
| // leading: <Papicons name="Link" />, | ||
| // onPress: () => Linking.openURL('https://x.com/thepapillonapp'), | ||
| // }, | ||
| // { | ||
| // title: "Instagram", | ||
| // leading: <Papicons name="Link" />, | ||
| // onPress: () => Linking.openURL('https://www.instagram.com/thepapillonapp'), | ||
| // }, | ||
| // { | ||
| // title: "Tiktok", | ||
| // leading: <Papicons name="Link" />, | ||
| // onPress: () => Linking.openURL('https://tiktok.com/@thepapillonapp'), | ||
| // }, | ||
| // { | ||
| // title: "Youtube", | ||
| // leading: <Papicons name="Link" />, | ||
| // onPress: () => Linking.openURL('https://youtube.com/@thepapillonapp'), | ||
| // }, | ||
| // { | ||
| // title: "LinkedIn", | ||
| // leading: <Papicons name="Link" />, | ||
| // onPress: () => Linking.openURL('https://fr.linkedin.com/company/papillonbzh'), | ||
| // }, | ||
| // ]; | ||
|
|
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.
tu peux retirer ça stp ?
ryzenixx
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.
pas mal globalement, il faudrait également que tu fasses les traductions dans les autres langues (minimum l'anglais) s'il-te-plaît ;)
| contentContainerStyle={{ padding: 20, gap: 20 }} | ||
| contentContainerStyle={{ padding: 20, gap: 10 }} | ||
| contentInsetAdjustmentBehavior="always" | ||
| showsVerticalScrollIndicator={false} |
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.
je suis pas sur de comprendre, pourquoi ?
| {/* <List> | ||
| {SocialLinks.map((item, index) => ( | ||
| <Item | ||
| key={index} | ||
| onPress={item.onPress} | ||
| > | ||
| <Leading> | ||
| <Icon> | ||
| {item.leading} | ||
| </Icon> | ||
| </Leading> | ||
| <Typography variant="title"> | ||
| {item.title} | ||
| </Typography> | ||
| <Trailing> | ||
| <Icon> | ||
| <Papicons name="ChevronRight" /> | ||
| </Icon> | ||
| </Trailing> | ||
| </Item> | ||
| ))} | ||
| </List> */} | ||
|
|
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.
ça aussi tu peux y retirer ;)
Résumé des changements
Séparation des liens anciennement appelés
Itemsen deux catégories :CommunityLinksDevelopperLinksAjout du lien
Signaler un bugavec la traduction en français.Ajout du lien
Projet Crowdinavec la traduction en français.J'ai enlevé le
verticalScrollIndicatorcar pas beau.Nettoyage du code (comme d'hab).
Capture(s) d'écran
Avant
Après
Summary by CodeRabbit
Nouvelles Fonctionnalités
Refactorisation
Améliorations UX
Corrections
✏️ Tip: You can customize this high-level summary in your review settings.