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

Vendor la dépendance easyMDE #5196

Merged
merged 3 commits into from
Dec 12, 2024
Merged

Vendor la dépendance easyMDE #5196

merged 3 commits into from
Dec 12, 2024

Conversation

francoisfreitag
Copy link
Contributor

@francoisfreitag francoisfreitag commented Dec 2, 2024

🤔 Pourquoi ?

Éviter le problème de dependabot avec setuptools (exemples: #5181-#5195).
Parce que la lib d’intégration a pas mal de défaut et semble non maintenue.
Pour mieux indiquer nos dépendances JS (plutôt que de passer une lib qui vendor pour nous).

🚨 À vérifier

Plutôt que de passer par un form Media qui est peu clair, j’ai ajouté la dépendance directement dans base.html. Tout le monde devra le charger une fois, puis on n’en parlera plus.

Comment tester ?

Valider les éditeurs de texte riche sur les pages de :

  • description fiche de poste employeur
  • description entreprise et accompagnement employeur
  • description orga prescripteur

💻 Captures d'écran

image

Copy link
Contributor

@EwenKorr EwenKorr left a comment

Choose a reason for hiding this comment

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

Peut-être des {% blocks script %} et {% block extra_head %} ?

Je pense à :

  • itou/templates/companies/create_siae.html
  • itou/templates/companies/edit_job_description_details.html
  • itou/templates/companies/edit_siae_description.html
  • itou/templates/prescribers/edit_organization.html

(introduits dans #4834)

@francoisfreitag
Copy link
Contributor Author

Je les ai mis dans base.html pour ne pas avoir à gérer du JS par page. Les gérer par page est également possible. Pour le JS qui est utile sur plusieurs pages, j’ai tendance à le rendre disponible partout pour éviter la surprise que le script ne soit pas disponible. 🤷

@francoisfreitag francoisfreitag added the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Dec 2, 2024
@EwenKorr
Copy link
Contributor

EwenKorr commented Dec 2, 2024

Mince, j'ai oublié un mot dans mon commentaire 🤦

Peut-être supprimer les blocs dans les fichiers que j'ai listés ?
Je les avais ajoutés page par page justement, et s'ils sont dans base.html, ces blocs sont inutiles dans le template, j'imagine.

@francoisfreitag
Copy link
Contributor Author

Ahh, avec le mot en plus je comprends mieux ! Très bonne remarque, c’est incorporé.

Copy link

github-actions bot commented Dec 2, 2024

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

@francoisfreitag francoisfreitag force-pushed the ff/easymde branch 2 times, most recently from d7eb7a8 to 95d9fc0 Compare December 3, 2024 13:45
@francoisfreitag
Copy link
Contributor Author

Est-ce qu’on ne pourrait pas se passer de fontawesome (en utilisant la config appropriée ?).
J’investiguerai vendredi.

@xavfernandez
Copy link
Contributor

Est-ce qu’on ne pourrait pas se passer de fontawesome (en utilisant la config appropriée ?). J’investiguerai vendredi.

👍 Ça me semblerait effectivement mieux que la CSP un peu moche 😬
A priori mettre autoDownloadFontAwesome à false (https://github.com/gip-inclusion/les-emplois/pull/5196/files#diff-20f58c734e7eca8ec6240b9a15fa3ee29518b99bd6dbb377eb2062bbbc058ec3R7 ) devrait suffire (en incluant bien font-awesome.min.css dans nos fichiers statics)

@EwenKorr
Copy link
Contributor

EwenKorr commented Dec 5, 2024

D'après

className: "ri ri-bold ri-lg",

c'est RemixIcon qu'on utilise, on n'aurait pas besoin de télécharger FontAwesome a priori. La version téléchargée est vieille en plus. Je ne me rappelle plus pourquoi j'ai laissé autoDownloadFontAwesome à true 🤔
Sans doute un oubli après trop de ping-pong dans la petite discussion UI : https://www.notion.so/plateforme-inclusion/L-icone-liste-me-fait-penser-l-option-d-alignement-du-texte-10ee8fa5c35b8083b2e3c218c1f670fe

@francoisfreitag
Copy link
Contributor Author

Je confirme que ça marche bien avec autoDownloadFontAwesome: false :)

We’re using RemixIcon. The fontawesome resources are blocked by the CSP
anyway.
@francoisfreitag francoisfreitag force-pushed the ff/easymde branch 3 times, most recently from 014f165 to 2367936 Compare December 6, 2024 15:53
Makes the dependencies clearer, avoid the dependency on setuptools which
has been an issue with dependabot, and remove some unused support from
the library, as well as using Python3-only helpers.
@francoisfreitag francoisfreitag added this pull request to the merge queue Dec 9, 2024
@francoisfreitag francoisfreitag removed this pull request from the merge queue due to a manual request Dec 9, 2024
@francoisfreitag francoisfreitag added this pull request to the merge queue Dec 12, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 12, 2024
@francoisfreitag francoisfreitag added this pull request to the merge queue Dec 12, 2024
Merged via the queue into master with commit b4415ac Dec 12, 2024
9 checks passed
@francoisfreitag francoisfreitag deleted the ff/easymde branch December 12, 2024 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC tech
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants