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

Import and export subjects colors and names #392

Closed
wants to merge 20 commits into from

Conversation

codeuriii
Copy link
Contributor

@codeuriii codeuriii commented Nov 30, 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

Page des paramètres de matière (SettingsSubjects.tsx)

  • Ajout d'un bouton exporter qui permet de copier un json contenant les informations des matières, leurs nom, couleurs etc.
  • Ajout d'un bouton importer qui permet de remplacer le json existant par le json contenu dans la presse papier. Une simple vérification de si le contenu de la presse papier est un json.

Informations supplémentaires

Je sais pas pourquoi il y a des modifications dans le package.json

TODO

implémenter les fichiers json

@codeuriii codeuriii requested a review from tryon-dev as a code owner November 30, 2024 13:40
@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 30, 2024

C'est normal la modification du fichier package.json, c'est npm ou autre qui a remis les dépendances dans l'ordre alphabétique. Mais ça n'a rien changé de plus, sinon le fichier package-lock.json serait changé

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 30, 2024

Jppas tester, comme on est tjrs en expo 51 (expo 52 obligatoire 🙄)
Bref, ça vérifie uniquement le presse papier ? Ou on peut chercher un fichier JSON depuis le stockage du téléphone ?

@codeuriii
Copy link
Contributor Author

Ca utilise uniquement la presse papier, je trouve que c'est plus simple de copier du texte que de gérer des fichiers sur tel

@codeuriii
Copy link
Contributor Author

je dois faire la migration expo 52 ?

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 30, 2024

Ca utilise uniquement la presse papier, je trouve que c'est plus simple de copier du texte que de gérer des fichiers sur tel

OK, oui c'est compréhensible

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 30, 2024

je dois faire la migration expo 52 ?

Non non, c'est trop de changements. Mais regarde, on l'a fait sur la pr #370 (n'hésite pas à tester d'ailleurs :))

@codeuriii
Copy link
Contributor Author

Du coup concrètement je dois faire quoi a part attendre le review du code ?

@Kgeek33
Copy link
Contributor

Kgeek33 commented Nov 30, 2024

Du coup concrètement je dois faire quoi a part attendre le review du code ?

Rien, faut attendre la review de ton code 😂
À la limite, je peux installer une ancienne version d'expo sur mon tél pour pouvoir tester

@codeuriii
Copy link
Contributor Author

Ok merci bah on va attendre tranquilou

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 dans l'ensemble, juste les fautes de frappe que je t'ai signalé
Cependant, p'têtre qu'il serait mieux d'utiliser showAlert pour Android et Alert.Alert ? faut voir avec les autres

src/views/settings/SettingsSubjects.tsx Outdated Show resolved Hide resolved
src/views/settings/SettingsSubjects.tsx Outdated Show resolved Hide resolved
codeuriii and others added 2 commits December 1, 2024 20:22
Co-authored-by: 𝕂𝕪𝕝𝕚𝕒𝕟 <164187100+Kgeek33@users.noreply.github.com>
Co-authored-by: 𝕂𝕪𝕝𝕚𝕒𝕟 <164187100+Kgeek33@users.noreply.github.com>
@codeuriii
Copy link
Contributor Author

Parfait dans l'ensemble, juste les fautes de frappe que je t'ai signalé Cependant, p'têtre qu'il serait mieux d'utiliser showAlert pour Android et Alert.Alert ? faut voir avec les autres

Pour ca, je me suis inspiré de la popup pour reset les matières donc jsp

@Kgeek33
Copy link
Contributor

Kgeek33 commented Dec 1, 2024

Parfait dans l'ensemble, juste les fautes de frappe que je t'ai signalé Cependant, p'têtre qu'il serait mieux d'utiliser showAlert pour Android et Alert.Alert ? faut voir avec les autres

Pour ca, je me suis inspiré de la popup pour reset les matières donc jsp

Oh, il y avait déjà Alert.Alert?🤦
OK my bad, j'm'en occuperai dans ma pr #378

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 👌

@codeuriii
Copy link
Contributor Author

et pour cette pr ? y a quoi qui bloque ?

@ecnivtwelve
Copy link
Contributor

Pas forcément fan de l'intégration depuis le presse-papier, peut être mieux de faire ça via le stockage comme la V6

@codeuriii
Copy link
Contributor Author

en pratique ca va etre des gens qui partage leurs config a le reste de la classe donc

  1. pour exporter, il faut faire un fichier, l'enregistrer au bon endroit, faire en sorte que cet endroit est accessible et retrouvable par "l'émetteur" pour envoyer aux gens, qui téléchargent, mettent au bon endroit, retrouve dans papillon et enfin import.
  2. l'émetteur copie, envoie te texte sur n'importe quel réseaux social (insta gère uniquement les images) et les autres copie le texte en un clique et import dans papillon

Le copié collé est beaucoup plus simple

@ecnivtwelve
Copy link
Contributor

Le copié collé est beaucoup plus simple

C'est pas forcément très propre, ça peut créer des très longs messages et des bugs assez facilement, donc pas forcément convaincu par cette intégration perso

@codeuriii
Copy link
Contributor Author

comment ca des bugs ?
et puis je parle pas de la messagerie papillon, et oui effectivement des messages peuvent etre longs mais les gens savent

@codeuriii
Copy link
Contributor Author

todo
document picker avec react native ou jsp quoi
ptet expo jsp

@codeuriii
Copy link
Contributor Author

voila c'est pret

@Kgeek33
Copy link
Contributor

Kgeek33 commented Dec 16, 2024

Résout tes conflits, je testerai plus tard

@codeuriii
Copy link
Contributor Author

comment je fais ca ?

@Kgeek33
Copy link
Contributor

Kgeek33 commented Dec 17, 2024

Tu utilises VSCode ?

@codeuriii
Copy link
Contributor Author

Oui

@Kgeek33
Copy link
Contributor

Kgeek33 commented Dec 17, 2024

Oui

Pour résoudre les conflits :

  • Déjà, faut être sur la branche de ta pr (main)
  • Ouvre un terminal (Affichage => Terminal)
  • Écris tout d'abord git fetch --all --prune
  • Ensuite quand c'est bon tu écris (tjrs dans la console) git merge upstream/main
  • Et là, sur ton VSCode, tu pourras résoudre les conflits qui s'affichent là où tu fais les commits

@codeuriii
Copy link
Contributor Author

j'ai utilisé github et ca m'a l'air bon

@codeuriii
Copy link
Contributor Author

J'ai bien besoin d'une review svp

@Kgeek33
Copy link
Contributor

Kgeek33 commented Dec 19, 2024

J'ai bien besoin d'une review svp

Déso, j'ai oublié de review, extrêmement concentré sur le lycée et sur la mise à niveau vers expo 52
Je fais ça ce soir ou ce midi. Appuie sur l'icône pour me redemander une review comme la dernière fois

@codeuriii codeuriii requested a review from Kgeek33 December 19, 2024 15:21
@codeuriii
Copy link
Contributor Author

Tkt c pas grave

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.

ok ça fonctionne bien en général mais applique mes recommandations au cas où si une erreur surviendrait lors de l'import depuis le presse papier/fichier

et aussi, il y a des rendus bizarres sur les Alert.Alert (des padding ou des margin, jspas)

src/views/settings/SettingsSubjects.tsx Show resolved Hide resolved
src/views/settings/SettingsSubjects.tsx Show resolved Hide resolved
@yannouuuu yannouuuu added ✨ enhancement New feature or request 🚸 user experience UX related issues labels Dec 23, 2024
@tryon-dev tryon-dev closed this Dec 23, 2024
@codeuriii
Copy link
Contributor Author

mais wsh ta un problème ?

@JyhuKo
Copy link
Contributor

JyhuKo commented Dec 23, 2024

mais wsh ta un problème ?

mdrrrr pq

@codeuriii
Copy link
Contributor Author

bah le reuf il ferme alors que je travaille dessus et j'ai rien demandé

@JyhuKo
Copy link
Contributor

JyhuKo commented Dec 23, 2024

j'avoue il est en pierre, il a meme pas mis de comment

@godetremy
Copy link
Contributor

godetremy commented Dec 23, 2024

mais wsh ta un problème ?

Hello,

Si t'as PR a été close c'est que sont intégration ne nous semble pas optimisée.

En revanche, peu importe la PR et la raison tu dois rester respectueux envers n'importe qui au risque de te retrouver exclu de la contribution.

Merci 🙃

@codeuriii
Copy link
Contributor Author

a quel moment j'ai manqué de respect ?
si vous pouviez etre encore moins précis j'apprécierais

@PapillonApp PapillonApp locked as spam and limited conversation to collaborators Dec 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
✨ enhancement New feature or request 🚸 user experience UX related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants