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

Add News Image Above Title #635

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

Add News Image Above Title #635

wants to merge 11 commits into from

Conversation

Kertie2
Copy link

@Kertie2 Kertie2 commented Jan 11, 2025

🚀 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 de l'affichage de l'image d'une actualité si elle est disponible, au-dessus du titre de l'actualité. Cela a été intégré en s'appuyant sur le décodeur d'illustrations existant dans decodeNews. (Merci à @NathanBnm pour l'amélioration de la PR)

Correction du décallage entre la date et nom de la personne qui a publié l'actualité.

Informations supplémentaires

Cette modification améliore l'expérience utilisateur en rendant les actualités plus visuelles et engageantes. Elle respecte également les conventions établies dans le projet et utilise les utilitaires déjà existants pour gérer les illustrations des actualités.

Aperçu

oriionn
oriionn previously approved these changes Jan 11, 2025
Copy link
Collaborator

@oriionn oriionn left a comment

Choose a reason for hiding this comment

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

LGTM

@Slysoks
Copy link
Contributor

Slysoks commented Jan 11, 2025

On peut pas plutôt la mettre en dessous ?

@oriionn
Copy link
Collaborator

oriionn commented Jan 12, 2025

On peut pas plutôt la mettre en dessous ?

J'ai du mal à imaginer, mais ça me parait bof dit comme ça

oriionn
oriionn previously approved these changes Jan 12, 2025
Copy link
Collaborator

@oriionn oriionn left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@NathanBnm NathanBnm left a comment

Choose a reason for hiding this comment

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

Faudrait décaler la logique d'affichage de l'image dans le composant Item je pense plutôt que dans la liste

@oriionn
Copy link
Collaborator

oriionn commented Jan 12, 2025

Faudrait décaler la logique d'affichage de l'image dans le composant Item je pense plutôt que dans la liste

En effet ça peut être intéressant de faire ça

@Kertie2
Copy link
Author

Kertie2 commented Jan 12, 2025

Faudrait décaler la logique d'affichage de l'image dans le composant Item je pense plutôt que dans la liste

En effet ça peut être intéressant de faire ça

Je vais faire ça

@Kertie2
Copy link
Author

Kertie2 commented Jan 12, 2025

Faudrait décaler la logique d'affichage de l'image dans le composant Item je pense plutôt que dans la liste

En vrai j'y arrive pas j'ai plein d'erreur et sa marche comme ça donc pourquoi se compliquer la vie ?

@Kgeek33
Copy link
Contributor

Kgeek33 commented Jan 12, 2025

Tu pourrais envoyer @Kertie2 une capture d'écran de ce que ça donne stp ?

@Kertie2
Copy link
Author

Kertie2 commented Jan 12, 2025

Tu pourrais envoyer @Kertie2 une capture d'écran de ce que ça donne stp ?

Je l'ai ajouté

@NathanBnm
Copy link

NathanBnm commented Jan 12, 2025

@Kertie2 j'ai testé avec un compte Pronote et y'a un souci car c'est pas forcément que des images en pièce jointe.

Si tu veux j'ai poussé un commit sur mon fork : NathanBnm@8f7d8b2

Dessus j'ai fait en sorte de déplacer la logique d'affichage de l'image dans l'item plutôt que dans la liste.

Et j'ai fait en sorte de prendre la première pièce jointe de type image.

Je te laisse reprendre ce que j'ai fait et tester si tu veux

@Kertie2
Copy link
Author

Kertie2 commented Jan 12, 2025

@Kertie2 j'ai testé avec un compte Pronote et y'a un souci car c'est pas forcément que des images en pièce jointe.

Si tu veux j'ai poussé un commit sur mon fork : NathanBnm@8f7d8b2

Dessus j'ai fait en sorte de déplacer la logique d'affichage de l'image dans l'item plutôt que dans la liste.

Et j'ai fait en sorte de prendre la première pièce jointe de type image.

Je te laisse reprendre ce que j'ai fait et tester si tu veux

Merci beaucoup Nathan c'est plus beau comme ça et c'est mieux géré.

@Kgeek33
Copy link
Contributor

Kgeek33 commented Jan 12, 2025

@Kertie2 j'ai testé avec un compte Pronote et y'a un souci car c'est pas forcément que des images en pièce jointe.

Si tu veux j'ai poussé un commit sur mon fork : NathanBnm@8f7d8b2

Dessus j'ai fait en sorte de déplacer la logique d'affichage de l'image dans l'item plutôt que dans la liste.

Et j'ai fait en sorte de prendre la première pièce jointe de type image.

Je te laisse reprendre ce que j'ai fait et tester si tu veux

Merci beaucoup Nathan c'est plus beau comme ça et c'est mieux géré.

Je review tout à l'heure

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.

beaucoup d'erreurs eslint sur l'indentation mais fonctionne bien
et dans ma review, j'ai demandé pour Ecole Directe, bcp nous demande d'afficher directement les images (j'ai link l'issue) mais jspas si c possible

src/views/account/News/Atoms/Item.tsx Outdated Show resolved Hide resolved
src/views/account/News/Atoms/Item.tsx Outdated Show resolved Hide resolved
src/views/account/News/Atoms/Item.tsx Outdated Show resolved Hide resolved
src/views/account/News/Atoms/Item.tsx Outdated Show resolved Hide resolved
src/views/account/News/Atoms/Item.tsx Outdated Show resolved Hide resolved
src/views/account/News/Atoms/Item.tsx Show resolved Hide resolved
src/views/account/News/News.tsx Outdated Show resolved Hide resolved
src/views/account/News/News.tsx Outdated Show resolved Hide resolved
Kertie2 and others added 7 commits January 12, 2025 20:23
Co-authored-by: 𝕂𝕪𝕝𝕚𝕒𝕟 <[email protected]>
Co-authored-by: 𝕂𝕪𝕝𝕚𝕒𝕟 <[email protected]>
Co-authored-by: 𝕂𝕪𝕝𝕚𝕒𝕟 <[email protected]>
Co-authored-by: 𝕂𝕪𝕝𝕚𝕒𝕟 <[email protected]>
Co-authored-by: 𝕂𝕪𝕝𝕚𝕒𝕟 <[email protected]>
Co-authored-by: 𝕂𝕪𝕝𝕚𝕒𝕟 <[email protected]>
Co-authored-by: 𝕂𝕪𝕝𝕚𝕒𝕟 <[email protected]>
@Kertie2 Kertie2 requested review from Kgeek33 and oriionn January 13, 2025 07:09
Comment on lines +189 to +194
newsImage: {
width: "100%",
height: 200,
borderRadius: 8,
marginBottom: 8,
},

Choose a reason for hiding this comment

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

@Kertie2 tu peux retirer ça du coup vu que l'image n'est plus dans ce fichier

@JyhuKo
Copy link
Contributor

JyhuKo commented Jan 15, 2025

peut etre jss con mais moi ca change rien dutout faut toujours cliquer pour aller voir l'image

@Kgeek33
Copy link
Contributor

Kgeek33 commented Jan 15, 2025

c'est ce que j'ai dis précédemment, faudrait une intégration avec Ecole Directe

pask Pronote, les images sont en pj. Sur ED, c'est du code HTML

@JyhuKo
Copy link
Contributor

JyhuKo commented Jan 15, 2025

ah oui mb

@JyhuKo
Copy link
Contributor

JyhuKo commented Jan 15, 2025

ecoledirecte c'est vrm pas la meta

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants