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

Mise à jour majeure de django_crispy_forms #6571

Merged
merged 2 commits into from
Feb 10, 2024

Conversation

Situphen
Copy link
Member

Mise à jour majeure de django_crispy_forms

  • django-crispy-forms fonctionne avec différents templates (Bootstrap 2, Bootstrap 3...) et ceux-ci ont été séparés du code Python
  • Pour le template Bootstrap 2 qu'on utilise pour nos formulaires, il faut en théorie installer le paquet crispy-forms-bootstrap2 sauf que sa version publiée sur Pypi ne contient pas le dossier templates/ pourtant présent dans le code source. J'ai créé un ticket il y a plusieurs mois mais je ne pense pas que l'on aura une réponse. Cette PR intègre donc ces éléments de templates dans notre code.
  • La version 2.1 est disponible mais on ne peut pas l'installer car on est encore sur Django 3.2

QA : Github Actions + vérifier que les formulaires du site web s'affichent correctement

@coveralls
Copy link

coveralls commented Jan 13, 2024

Coverage Status

coverage: 88.816%. remained the same
when pulling 94e3572 on Situphen:crispy
into cc1e21e on zestedesavoir:dev.

@philippemilink
Copy link
Member

Je vais poser quelques questions sans doute "historiques"...

Pour le template Bootstrap 2 qu'on utilise pour nos formulaires

On utilise leur templates "bootstrap 2" et on redéfinit les classes CSS correspondantes dans notre propre CSS avec notre propre style, c'est bien ça ? Il n'y a pas de CSS Bootstrap 2 dans notre code ?

J'ai créé un ticket il y a plusieurs mois mais je ne pense pas que l'on aura une réponse.

Il y a une trentaine de commits dans ce dépôt, le dernier a un peu moins de deux ans, ça sent le dépôt mort... (et ce n'est pas vraiment étonnant, c'est Bootstrap 2, on est à la version 5...).

Je viens de voir que le paquet crispy-bootstrap3 inclut bien les templates. J'ai regardé (très) rapidement les différences entre les deux templates, je n'ai pas l'impression que ce soit fondamentalement différent... Tu as essayé avec ce paquet ?

En tout cas, si on rapatrie vraiment des templates dans notre dépôt, je suggère de changer de nom de CRISPY_TEMPLATE_PACK (bootstrap -> zds_crispy, par exemple) pour éviter la confusion (cf ma première question) et documenter quelque part que ça provient initialement de tel dépôt, que ça a été rapatrié pour telles raisons, etc (par exemple ici ou/et dans un README dans le dossier des templates).

@Situphen
Copy link
Member Author

On utilise leur templates "bootstrap 2" et on redéfinit les classes CSS correspondantes dans notre propre CSS avec notre propre style, c'est bien ça ? Il n'y a pas de CSS Bootstrap 2 dans notre code ?

Il me semble que le projet a initialement été codé avec Bootstrap 2 pour le frontend, avant de le modifier avec un style personnalisé. La structure de Bootstrap 2 est resté pour les formulaires étant donné l'utilisation de ce module, mais il n'y a plus de CSS Bootstrap 2 dans le code.

Il y a une trentaine de commits dans ce dépôt, le dernier a un peu moins de deux ans, ça sent le dépôt mort... (et ce n'est pas vraiment étonnant, c'est Bootstrap 2, on est à la version 5...).

Je viens de voir que le paquet crispy-bootstrap3 inclut bien les templates. J'ai regardé (très) rapidement les différences entre les deux templates, je n'ai pas l'impression que ce soit fondamentalement différent... Tu as essayé avec ce paquet ?

Ces deux paquets ont le même rôle. Je pense que le paquet pour Bootstrap 2 a simplement été publié sans que personne ne vérifie son bon fonctionnement ensuite (ce qui ne m'étonne pas, c'est une version désuète). On pourrait tenter d'utiliser le paquet Bootstrap 3, mais il est probable que l'on ait des retouches de CSS à faire derrière. Cela demandera donc de vérifier chaque formulaire et réparer ce qui cassera.

@philippemilink
Copy link
Member

@Situphen : django-crispy-forms/crispy-forms-bootstrap2#7 (comment) plus besoin d'inclure les templates

@Situphen
Copy link
Member Author

J'ai mis à jour ma PR et elle est prête à être QAtisée !

Copy link
Member

@philippemilink philippemilink left a comment

Choose a reason for hiding this comment

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

QA OK ✔️

@philippemilink philippemilink enabled auto-merge (squash) February 10, 2024 18:26
@philippemilink philippemilink merged commit 789e743 into zestedesavoir:dev Feb 10, 2024
8 checks passed
@Situphen Situphen deleted the crispy branch February 10, 2024 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants