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.

Sign up for GitHub

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

Jump to bottom

Implémentation simplifiée du support de svg #5421

Closed
wants to merge 3 commits into from
Closed

Implémentation simplifiée du support de svg #5421

wants to merge 3 commits into from

Conversation

artragis
Copy link
Member

@artragis artragis commented Aug 30, 2019 •
edited
Loading

Résumer

Le but de cette PR est de supporter l'ajout d'image au format SVG pour les forums et les tuto.

J'ai tenté pendant un long moment d'adapter easy_thumbnails pour le SVG mais il y a des appels statics à PIL à peu près partout dans le code et y'a aucun intérêt à convertir des svg en png.

J'ai donc changé mon fusil d'épaule, plutôt que de forcer au talon une adaptation de la lib, j'ai créé un modèle "parallèle" à notre modèle d'image qui n'utilise pas de thumbnail. Du coup je peux manipuler les svg comme je veux. Cette méthode a généré beaucoup de duplication de codes mais avec l'aide de personnes qui s'y connaissent un peu genre @gustavi ça pourra devenir propre.

Etat des lieux

  • Créer l'interface d'ajout sans les thumbnails
  • Permettre l'ajout d'image via l'API
  • Permettre la suppression d'un SVG depuis l'interface d'édition
  • Permettre la suppression d'un svg depuis la liste dans la gallerie

QA

  • builder le front
  • mettre à jour la bdd

step 1

  • aller sur le message d'un forum
  • glisser/déposer un svg (astuce: les fichiers du dossier asset/licenses sont en svg)
  • poster
  • l'image apparaît

step 2

  • aller sur la gallerie par défaut
  • constater que l'image précédemment ajoutée est là
  • cliquer sur "ajouter un dessin"
  • ajouter un svg

step 3

  • ajouter une image non svg
  • dans la liste des images, sélectionner cette image ET un svg, cliquer sur "supprimer"
  • constater que tout a été supprimé

step 4

  • aller sur le premier svg
  • cliquer sur supprimer l'image
  • constater que c'est bon.

step 5

  • cliquer sur ajouter un dessin, ajouter un svg
  • copier le lien "pour une image en une"
  • créer une une avec le svg
  • vérifier que ça marche sans être pixélisé ni tronqué (bon redimensionnement)

@artragis artragis added C-API Concerne une API du site C-Back Concerne le back-end Django C-Front Concerne l'interface du site Feedback Ticket ou PR en attente de retours S-Évolution Ajoute de nouvelles fonctionnalités labels Aug 30, 2019
@coveralls
Copy link

coveralls commented Aug 30, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-14.9%) to 73.795% when pulling 400ab04 on artragis:poc_add_svg into 2eea7f0 on zestedesavoir:dev.

@artragis artragis removed the Feedback Ticket ou PR en attente de retours label Sep 12, 2019
@artragis artragis changed the title [WIP]Implémentation simplifiée du support de svg Implémentation simplifiée du support de svg Sep 12, 2019
firm1
firm1 reviewed Oct 16, 2019
View reviewed changes
Copy link
Contributor

@firm1 firm1 left a comment

Choose a reason for hiding this comment

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

Une première review rapide.

zds/gallery/api/serializers.py
physical = serializers.FileField(write_only=True, required=False)

class Meta:
model = Image
Copy link
Contributor

Choose a reason for hiding this comment

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

je trouve ça déroutant de voir model = Image ici alors que l'on est sur DrawingSerializer

templates/gallery/image/edit-drawing.html


{% block title %}
{% trans "Éditer une image" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Éditer un dessin

Copy link
Contributor

Choose a reason for hiding this comment

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

Je n'aime pas trop dessin, les svg ne sont pas forcement des dessins.

Image vectorielle ? Ou directement mettre "SVG" ça évite toute confusion et met en évidence que les SVG c'est ici.

templates/gallery/image/edit-drawing.html

{% block breadcrumb %}
<li><a href="{{ gallery.get_absolute_url }}">{{ gallery.title }}</a></li>
<li>{% trans "Éditer une image" %}</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Éditer une dessin

templates/gallery/image/edit-drawing.html


{% block headline %}
{% trans "Éditer une image" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Éditer une dessin

templates/gallery/image/edit-drawing.html
{% block content %}
<div class="gallery-col-image">
<p>
{% trans "Image :" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Dessin

templates/gallery/image/edit-drawing.html

{% if perms.featured.change_featuredresource %}
<p>
{% trans "Lien pour utiliser cette image en une :" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Lien pour utiliser ce dessin en une

templates/gallery/image/edit-drawing.html
{% endif %}

<p>
{% trans "Code markdown pour insérer cette image :" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Code markdown pour insérer ce dessin

templates/gallery/image/edit-drawing.html
{% trans "Code markdown pour insérer cette image :" %}
<br>

{% trans "Taille normale : " %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Est-ce qu'on a besoin de préciser ici taille normal, si on a une seule taille ? J'aurai juste parlé de "code markdown"

templates/gallery/image/edit-drawing.html
<form id="form-delete-image" name="form" method="post" action="{% url "gallery-drawing-delete" gallery.pk %}" class="modal modal-flex">
{% csrf_token %}
<input type="hidden" name="image" value="{{ image.pk }}">
<p>{% trans "Attention, vous vous apprêtez à supprimer cette image." %}</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Attention, vous vous apprêtez à supprimer ce dessin.

@firm1
Copy link
Contributor

firm1 commented Oct 16, 2019

Rapport de QA

Probleme 1

J'ai un svg dans ma gallerie, je clique dessus pour l'éditer, je choisi un svg, je clique sur mettre à jour, j'ai comme message d'erreur sur le champ : "Transférez une image valide. Le fichier que vous avez transféré n'est pas une image, ou il est corrompu."

Probleme 2

Lorsque je supprime le svg de la galérie, il est bien supprimé dans l'interface, mais le fichier existe toujours sur le serveur, contrairement à un fichier non-svg qui lui se supprime bien du serveur.

Problème 3

Le bouton pour rajouter un svg s'appelle "Ajouter un dessin" et pour les non svg c'est "Ajouter une image". C'est cool!

Par contre, lorsqu'on clique sur "Ajouter un dessin", dans la barre de navigation on à des résidus de "Ajouter une image", "Éditer une image", "Sélectionnez votre image". Le mieux serait d'harmoniser tout le terme dessin, partout là ou on est au niveau du dessin.

Capture d’écran de 2019-10-16 14-10-19

Problème 4

Lorsque j'utilise le bouton "Importer une archive" dans lequel il y a du svg et du jpeg. L'import du jpeg se fait, mais j'ai un message visuel qui me dit que le svg n'a pas pu être importé.

On ne peut donc pas importer d'archive contenant du svg. C'est dommage, parce que ça ne me parait pas compliqué a corriger.

Review

  • Je ne vois pas de tests associés à cette nouvelle fonctionnalité. C'est normal ? Il ne faudrait pas qu'il y ait des régressions dessus par la suite.

Je n'ai pas encore testé la partie api celà dit.

@A-312
Copy link
Contributor

A-312 commented Oct 16, 2019

Ce n'était pas possible d'utiliser le même formulaire, pour rendre ça transparent à l'utilisateur ?
Ca complexifie grandement la chose de mettre deux pages.

On ne pouvait pas rajouter un if file.type == "svg": ? Et modifier la validation du formulaire, la vue utilisée en fonction du type (svg ou image) ?

@artragis
Copy link
Member Author

artragis commented Mar 7, 2021

Il y a une PR sur easythumbnails pour supporter SVG SmileyChris/easy-thumbnails#560

@artragis artragis closed this Mar 9, 2021
@artragis artragis deleted the poc_add_svg branch March 9, 2021 07:48
@artragis
Copy link
Member Author

artragis commented Mar 9, 2021

Reprise avec la nouvelle version de easy-thumbnail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@firm1 firm1 firm1 left review comments

@A-312 A-312 A-312 left review comments

Assignees
No one assigned
Labels
C-API Concerne une API du site C-Back Concerne le back-end Django C-Front Concerne l'interface du site S-Évolution Ajoute de nouvelles fonctionnalités
Projects
Suivi des PR
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@artragis @coveralls @firm1 @A-312