Produits Entr'ouvert » Combo

Agate Berriot a écrit :

Je me suis permise de restructurer le layout en passant sur une table, plutôt que sur une liste. Ça permet un affichage plus aéré et l'alignement augmente aussi la lisibilité

Effectivement, c'est plus joli ! Par contre ce layout est aussi utilisé côté wcs (sur la page d'un formulaire/workflow, lien « Historique » à droite), sauf qu'on y a mis des boutons radio pour voir les différences entre deux versions.
Ça pose donc le problème d'introduire une divergence entre les écrans, et plus embêtant de peut-être découvrir en voulant permettre cette fonctionnalité dans combo que c'est galère avec une table.
Donc je dirais soit aller voir ce qu'il est possible de faire dans wcs, soit revenir en arrière ici.

Pour info dans ce genre de cas mon goût personnel est de splitter en deux commits, mais je suis un peu maniaque des diffs :)

Pour la vue snapshot_set_label on est plutôt à toujours faire des class based view.

Il y a un intérêt à avoir null=True pour le CharField ?

Valentin Deniaud a écrit :

Agate Berriot a écrit :

Je me suis permise de restructurer le layout en passant sur une table, plutôt que sur une liste. Ça permet un affichage plus aéré et l'alignement augmente aussi la lisibilité

Effectivement, c'est plus joli ! Par contre ce layout est aussi utilisé côté wcs (sur la page d'un formulaire/workflow, lien « Historique » à droite), sauf qu'on y a mis des boutons radio pour voir les différences entre deux versions.
Ça pose donc le problème d'introduire une divergence entre les écrans, et plus embêtant de peut-être découvrir en voulant permettre cette fonctionnalité dans combo que c'est galère avec une table.
Donc je dirais soit aller voir ce qu'il est possible de faire dans wcs, soit revenir en arrière ici.

Pour info dans ce genre de cas mon goût personnel est de splitter en deux commits, mais je suis un peu maniaque des diffs :)

Pour la vue snapshot_set_label on est plutôt à toujours faire des class based view.

Il y a un intérêt à avoir null=True pour le CharField ?

Pour le null=True, vu qu'il y a des snapshots existants, ça me semblait nécessaire puisqu'on rajoute une colonne sans valeur par défaut.

Je vais voir avec Fred ce qu'il pense de changer le layout sur WCS :)

Pour le null=True, vu qu'il y a des snapshots existants, ça me semblait nécessaire puisqu'on rajoute une colonne sans valeur par défaut.

Je viens d'adresser ce point sur mon dernier patch, comme discuté avec Valentin et expliqué sur https://docs.djangoproject.com/en/4.0/ref/models/fields/#null, le NULL sur un champ texte est pas nécessaire ni souhaitable.

Je vais voir avec Fred ce qu'il pense de changer le layout sur WCS :)

Pas d'objection à l'utilisation d'un tableau mais une note rapide avec le tableau pas besoin de mettre prénom nom entre parenthèses.

Plus important, je préfère ce qui est fait dans wcs où "Enregistrer une sauvegarde" prend la version en cours et la duplique avec un libellé, et dans l'historique ça apparait ainsi :

• date·heure, le nom donné à cette version en gras
• date·heure, le snapshot de la précédente modif
• date·heure, le snapshot de la précédente modif
• ...

Dans la disposition tableau proposée, ça permet d'avoir une seule colonne avec soit le nom d'une version explicite, soit la description automatique, ça évite aux lignes de passer sur deux lignes et au tableau à perdre en lisibilité quand un libellé est un peu long (pas de colonne élargie mais vide sur la plupart des lignes).

En fait la réalisation du ticket correspond bien à la description "pouvoir poser une étiquette sur certaines d'entre elles" mais l'important c'était "comme cela se fait dans wcs" (qui était à lire comme pas besoin de pouvoir poser de nom sur des vieilles versions et une action explicite depuis la page générale, pas la page de l'historique).

Côté code je pense que ça ne demande pas trop de changement,

+        r'^pages/(?P<page_pk>\d+)/history/(?P<pk>\d+)/set-label$',
→
+        r'^pages/(?P<page_pk>\d+)/history/save$',

et dans la vue sur le POST du formulaire qui demande le libellé, PageSnapshot.take(..., label=le nom).

Mon dernier patch intègre les remarques de Valentin et Fred, notamment :

- Passage sur une class based view
- le label n'est plus settable dans l'historique mais lors d'une action explicite de sauvegarde (dans le menu kebab)
- petites remises en formes (parenthèses retirées, gras)

Automatic expiration