Produits Entr'ouvert » Combo

Comme la branche en question est partie de mon code, elle sera plus facile à relire avec des commits supplémentaires, plutôt que l'affaire mélangée. Tout simplement parce qu'il y aura moins à relire, et si c'est divisé en commits simples, ça permettra de les justifier très facilement, genre :

-        return urls.manager_urlpatterns
+        return urls.gallery_manager_urls

→ rename manager url patterns variable to match other modules

Et ça permettra de manière évidente d'identifier les points de mon précédent commentaire,

• un commit "manager_form_template"
• un commit "import/export"
• un commit "tests"

Aussi, je vois qu'il y avait https://git.entrouvert.org/combo.git/log/?h=wip/gallery, qui ajoute un attribut "title". Je viens de rebaser la branche (juste un petit conflit sur une modif à combo.manager.css).

J'attache une capture d'écran de la trace.

Tu es le deuxième en queqlues jours à taper une capture d'écran de trace. Je n'essaie pas de déchiffrer. Il doit y avoir dans la capture un bout "cliquez ici pour avoir un truc à copier/coller".

Sur le fond, en trois ans il y a mille choses qui ont changé dans combo, c'est normal qu'il y ait des modifications à faire.

J'ai poussé une série de patches dans origin/wip/7344-Cellule-de-type-galerie-d-images- qui couvrent ce qui énoncé dans #7344#note-5.

Au moins un souci: le code d'import risque d'exploser si plusieurs galeries non-nommées dans une même page (hmm, dans un même site), je n'ai pas encore trouvé comment identifier proprement la GalleryCell parente, je me basais sur le slug de GalleryCell pour assigner une image à sa galerie, il doit y avoir une meilleure façon :

gallery = GalleryCell.objects.get(slug=json_image['gallery_slug'])

Au moins un souci: le code d'import risque d'exploser (...)

Ça se résoud en exportant la liste des images comme étant un attribut de la cellule, plutôt qu'un truc indépendant. (comme les cellules sont posées en attribut de page, plutôt que chargées indépendamment).

+<img src="{{ im.url }}" alt="gallery front image"/>

Les images ont un titre, c'est ça qu'il faut utiiser pour l'accessibilité.

+        for index, cell in enumerate(cells):
+            if cell.__class__.__name__ == 'GalleryCell':
+                serialized_page['cells'][index]['images'] = cell.get_serialized_images()

Ça doit plutôt être fait en appelant une méthode des cellules, genre,

for index, cell in enumerate(cells):
     serialized_page['cells'][index].update(cell.export_subobjects())

(méthode qui dans CellBase retournerait juste {}).

(et même démarche à mettre pour l'import)

OK, j'ai adapté avec un CellBase.export_subobjects().

Ok, détails :

• que les vignettes soient des liens,

--- a/combo/apps/gallery/templates/combo/gallerycell.html
+++ b/combo/apps/gallery/templates/combo/gallerycell.html
@@ -13,8 +13,8 @@
   {% endif %}
 {% thumbnail image.image "60x60" crop="50% 25%" as im %}
 {% thumbnail image.image "640x480" crop="50% 25%" as im_large %}
-<span data-image-large="{{ im_large.url }}" class="image-switch"><img src="{{ im.url }}" 
-        {% if image.title %} title="{{image.title}}" {% endif %}/></span>
+<a data-image-large="{{ im_large.url }}" class="image-switch" href=""><img src="{{ im.url }}" 
+        {% if image.title %} title="{{image.title}}" {% endif %}/></a>
 {% endthumbnail %}
 {% endthumbnail %}
 {% endfor %}
@@ -27,6 +27,7 @@ $(function() {
     $gallery.find('div.main-image img').attr('src', $(this).data('image-large'));
     var $title = $(this).find('img').attr('title') || '';
     $gallery.find('div.main-image img').attr('title', $title);
+    return false;
   });
 });
 </script>

• ne pas coller le premier import au commentaire de préambule,

@@ -13,6 +13,7 @@
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
 import json

• dans l'export json ça aurait été super de ne pas avoir "model": "gallery.image" mais c'est plus simple de le garder.

• il y aura un peu de style de base à taper dans publik-base-theme (a minima, centrer).

Pour moi tu peux appliquer les deux diff ci-dessus, tout rebaser sur le premier commit, y taper ton nom si tu veux, et ça sera ok.

que les vignettes soient des liens,

Est-ce qu'on signifierait la présence d'un lien? par exemple en ajoutant ".image-switch { cursor: pointer; }" (je ne sais pas dans quel fichier css)

Je ne comprends pas pourquoi on ajoute "return false;" au code jquery, je lis rapidemment que ça permet d'annuler un comportement par défaut, est-ce qu'on veut annuler une éventuelle autre référence à document.ready()?

dans l'export json ça aurait été super de ne pas avoir "model": "gallery.image"

pouquoi?

déplacer l'action d'ajout d'une image comme étant un bouton intégré aux miniatures.

cool!

Est-ce qu'on signifierait la présence d'un lien? par exemple en ajoutant ".image-switch { cursor: pointer; }" (je ne sais pas dans quel fichier css)

Sémantiquement un élément sur lequel on clique pour afficher un nouveau contenu est un lien (à la limite, un bouton), ça vient notamment par défaut avec cette indication du curseur, mais par exemple ça met aussi l'élément dans la navigation par clavier.

Je ne comprends pas pourquoi on ajoute "return false;" au code jquery, je lis rapidemment que ça permet d'annuler un comportement par défaut, est-ce qu'on veut annuler une éventuelle autre référence à document.ready()?

C'est dans le handler de l'événement click(), c'est le comportement par défaut de celui-ci (charger une nouvelle page) qu'on veut éviter.

dans l'export json ça aurait été super de ne pas avoir "model": "gallery.image"

pouquoi?

Parce que ça n'apporte aucune information, est juste un artefact de la sérialisation django. (mais comme je notais, pas important).

+<a data-image-large="{{ im_large.url }}" class="image-switch" href=""><img src="{{ im.url }}" 

Le href était important, sinon c'est un ancre, pas un lien.

commit 2510ef70d4605bb6298a3074f35e8a6863866483
Author: Frédéric Péters <fpeters@entrouvert.com>
Date:   Tue May 14 13:52:04 2019 +0200

    gallery: add missing link target on thumbnails (#7344)