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

Modification du champ format #477

Closed
9 of 10 tasks
DaFrenchFrog opened this issue Oct 6, 2022 · 19 comments · Fixed by #566
Closed
9 of 10 tasks

Modification du champ format #477

DaFrenchFrog opened this issue Oct 6, 2022 · 19 comments · Fixed by #566
Assignees
Labels

Comments

@DaFrenchFrog
Copy link
Collaborator

DaFrenchFrog commented Oct 6, 2022

L'implémentation actuelle du champ format est trop limitante, on doit permettre un plus grand choix de valeurs possibles tout en aidant les contributeurs en affichant lors de la saisie les valeurs les plus couramment utilisées.

Schéma

https://github.com/etalab/schema-catalogue-donnees/blob/v0.3.0/schema.json#L189

  • Slug : format
  • Titre : Format
  • Description : Liste des formats associés à ce jeu de données
  • Type : chaîne de caractères
  • Requis : oui

Implémentation

  • Backend : Aligner avec le schéma Table Schema (simple champ chaîne de caractère). Enlever la contrainte enum.
  • Backend: coté import de catalogue, pouvoir importer n'importe quelle chaine de caractère comme format
  • Front côté formulaire : saisie libre, avec affichage d'une liste de valeurs parmi les données du catalogue (pareil que pour les champs Licence et Couverture géographique), saisie de plusieurs valeurs (pareil que pour le champ Mots clés)
  • Front côté fiche : afficher la liste des valeurs, séparées par une virgule (+ espace après la virgule). Saut de ligne si la liste des valeurs ne tient pas sur la ligne
  • Front côté recherche : saisie libre, avec affichage d'une liste de valeurs parmi les données du catalogue, saisie d'une seule valeur (pareil que pour le champ Mots clés)
  • Côté API (export CSV) : les valeurs multiples seraient séparées par une virgule (comme pour le champ couv géo).

Next

Améliorations possibles suite à #555 et #556 :

  • Afficher la liste de valeurs quand il y a un focus sur le champ
  • Retour à zéro (vider le champ) après que l'utilisateur ait choisit une valeur dans la liste et qu'elle se soit ajoutée en dessous (comme quand on entre une nouvelle valeur, pas dans la liste)
  • Valider une nouvelle valeur avec la touche entrée
  • Fermer la liste quand on enlève le curseur (comme c'est le cas par exemple pour le champ licence)
@florimondmanca
Copy link
Collaborator

@DaFrenchFrog Actuellement le champ "Formats" est multiple (on peut cocher plusieurs cases). Cela se maintient-il ? Je dirais que oui car dans le catalogue MC il y a parfois plusieurs valeurs (pas forcément bien encodée = séparées par des virgules). Mais le champ texte libre type "Service" ou "Couverture géographique" n'ont en revanche qu'une valeur. Donc l'utilisation ne sera pas la même. Quelle adaptation peut-on envisager ?

@johanricher
Copy link
Member

C'est vrai que la question d'implémenter un composant select multi attributs doit être abordée également dans ce cadre (même si en 2ème ou 3ème étape, avec un truc intermédiaire simple à faire dans un premier temps). Quitte à mettre dans le backlog des choses souhaitables qu'on devra prioriser (et peut-être malheureusement pas pouvoir faire), autant y aller franco. 👼

@Volubyl
Copy link
Collaborator

Volubyl commented Oct 26, 2022

Nous somems en train de discuter du script d'import avec @florimondmanca et je me rends compte que ne plus avoir une liste fermée de valeur possible pourrait simplifier la vie.

Je développe plus tard

@johanricher
Copy link
Member

Oui je veux bien discuter / brainstormer l'implémentation pour que ça puisse passer en "Prêt à développer".

@Volubyl
Copy link
Collaborator

Volubyl commented Oct 26, 2022

Too long; won't read

Je soutiens la proposition de romain de passer en champs texte libre mais il faudrait essayer de trouver un système pour éviter les doublons.

Est-ce que le système de mot clé pourrait être repris ?

Alors, je développe :

Quand Florimond a écrit le script d'import, il s'est rendu compte que les valeurs renseignées n'était pas toujours les mêmes pour désigner un même format.

Par exemple, il y a du XLS, XLSX, xlsx, xlsx, Excell (et d'autres variantes) qui désigne en réalité le même format de donnée (données tabulaires dans notre cas)

La solution trouvée est donc de créer un système de mapping pour dire si tu vois XLS considère que cela correspond à la valeur file_tabular de l'enum que l'on utilise.

Le problème, c'est que je pense que l'humain peut être très créatif et qu'on pourrait imaginer plein de variations possible autour de la notion de 'XLS'.

Par exemple, on pourrait imaginer qu'une personne indique dans le csv .xls, .xlsx etc

Ce qui impliquerait de toujours passer au crible le fichier csv à la recherche de format "exotique" et d'écrire le mapping ad hoc.

En résumé

Je pense qu'imaginer que ces champs format ne pourra jamais être considérés comme une liste finie d'éléments.

**Une proposition possible **

Considérer ce champ comme un champ texte libre

Oui, mais l'on va se retrouver avec des doublons…

Certes, mais je me rappelle que Romain proposait d'avoir un système d'autocomplétion et de suggestion. Un tel système ne résoudra pas le problème de doublons, mais pourra l'amoindrir

Quels sont les impacts techniques de considérer ce champ non plus comme un enum mais un champ texte libre ?

Cela mérite que je regarde en détail. Affaire à suivre

@DaFrenchFrog
Copy link
Collaborator Author

Je pense que nous sommes tous d'accord pour dire que sur ce champ la liste fermée n'est pas recommandée.

Pour être plus précis sur ma recommandation, il ne s'agit pas d'un champ avec auto-complétion(qui suppose qu'il faut d'abord taper un texte pour voir une liste apparaitre), mais plutôt un champ "auto-suggestion multi-critère" si on devait lui donner un nom. Voici le comportement étape par étape :

  • Au départ, le champ est vide. Il peut y avoir des mot-clés en-dessous si des mot-clés on déjà été sélectionnés. Ces mot-clés possèdent une petite croix à droite, indiquant qu'ils peuvent être supprimés
  • Lorsque je clique sur le champ, une liste avec les mot-clés déjà saisis dans les autres jdd apparait, et le champ est éditable (on a le petit curseur qui clignote)
  • En saisissant un caractère, la liste est filtrée pour ne laisser apparaitre que les mot-clés correspondant
  • Si j'appuie sur la touche entrée ou que je clique sur le bouton "+" le mot-clé de la saisie actuelle est ajouté et le champ est réinitialisé (on conserve le focus mais le champ est vide)
  • Si je clique sur un mot-clé présent dans la liste, le mot-clé est ajouté et le champ est réinitialisé
  • Lorsque je quitte le focus, la liste disparait

Le principe d'auto-suggestion est présent sur les autres champs sur staging, il faut donc ajouter le principe de mot-clés.

Maquettes

J'ai découpé les étapes clés ici :
https://www.figma.com/file/42KOPuvkD1jpubEe2mMtXr/DATALOGUE-maquettes?node-id=3207%3A67243

Dites-moi ce que vous en pensez.

@johanricher
Copy link
Member

johanricher commented Oct 30, 2022

C'est un peu un mix de l'implé actuelle sur les champs "couv géo" / "licence" (liste des saisies existantes.+ autocomplétion) et "mots-clés" (plusieurs valeurs possibles, affichées en-dessous du champ).

Perso ça me parait nickel pour l'implé côté formulaire de contribution.

Côté affichage fiche : la liste des valeurs seraient affichées, séparées par une virgule (+ espace après la virgule). Si la liste des valeurs ne tient pas sur la ligne, on aurait un saut de ligne je suppose ? (ça serait bien de le prévoir sur la maquette)

Côté recherche : on reprendrait l'implé du champ "mots-clés"

Côté API (export CSV), les valeurs multiples seraient séparées par une virgule (comme pour le champ couv géo).

A confirmer, mais selon moi sI ces implémentations s'avèrent satisfaisantes pour ce champ, elles auront vocation à être appliquées également sur les champs "licence", "couv géo", "mots-clés", "fréquence maj" (#481) et peut-être d'autres. Le cas échéant ça fera l'objet de ticket(s) séparé(s).

@Volubyl
Copy link
Collaborator

Volubyl commented Feb 2, 2023

@johanricher @DaFrenchFrog voilà voilà je pense que j'ai enfin une version de cette feature à vous montrer sur staging

Pourquoi c'était si long ?

Pour être honnête, j'ai galéré avec cette feature pour plusieures raisons :

  • mes connaissances en python sont pas encore suffisament bonnes que pour avoir des réflèxes qui m'auraient éviter de coincer sur un certains nombre de truc sur lesquels je me suis pas mal tiré les cheveux
  • j'ai tenté de faire en sorte que le composant soit accessible. Cela implique que celui-ci soit utilisable uniquement au clavier et plus j'avançais plus je me rendais compte qu'il y avait des cas d'usages que je n'avais pas couverts (*)
  • j'ai pris deux jours de recul par rapport à cette feature pcq sincièrement elle me sortait pas les yeux. J'avais besoin de faire autre chose histoire d'y voir plus clair.

Et maintenant ?

La feature est sur staging et je pense que c'est une bonne version à vous soumettre.
J'ai besoin de vos retours pour voir si il y a d'autres cas d'usages auxquels j'ai pas pensé.

N'hésitez donc pas à jouer avec.

Merci

(*) Notes :

  • Ce composant n'est pas qqch de standard dans le DSFR et j'ai pris le temps d'en discuter avec des personnes plus calé en a11é.
  • ce type de composant devrait être standardisé au sein du DSFR car je me suis rendu compte en discutant sur le mattermost de beta que bcp de gens avaient le même genre de besoin

@johanricher
Copy link
Member

johanricher commented Feb 2, 2023

Merci Bertrand pour ce compte-rendu du travail que tu as réalisé. C'est très clair et utile pour avoir une compréhension plus globale et contextualisée.

Donc déjà merci !

En ce qui me concerne en tant qu'utilisateur, l'accessibilité est quelque chose que j'apprécie car ça se traduit par une meilleur UX clavier. J'estime que ce sont des efforts de dév supplémentaires qui valent la peine. 👍

A ce sujet j'ai 2 remarques à faire pour peaufiner le système :

  • Quand on choisit une valeur dans la liste, elle s'ajoute en dessous mais elle reste dans le champ. On s'attend à ce que le champ soit vidé (comme quand on entre une nouvelle valeur)
  • On peut choisir une valeur dans la liste avec les fleches et la touche entrée permet de valider, par contre quand on entre une nouvelle valeur (pas dans la liste) la touche entrée ne permet pas de la valider
  • La liste s'ouvre quand on place le curseur dans le champ mais on s'attend aussi à ce qu'il se ferme quand on l'enlève (comme c'est le cas par exemple pour le champ licence)

Je comprends cependant que tu n'aies pas envie de passer plus de temps dans l'immédiat sur ce composant donc ça peut attendre. :)

ce type de composant devrait être standardisé au sein du DSFR car je me suis rendu compte en discutant sur le mattermost de beta que bcp de gens avaient le même genre de besoin

Clairement ! Est-ce qu'on peut ouvrir un ticket quelque part, qu'on illustrerait par le travail que tu as fait sur ce composant ?

@Volubyl
Copy link
Collaborator

Volubyl commented Feb 2, 2023

@johanricher

Je comprends cependant que tu n'aies pas envie de passer plus de temps dans l'immédiat sur ce composant donc ça peut attendre. :)

J'ai bien envie d'aller jusqu'au bout avec cette feature donc je peux continuer à apporter des améliorations :-)

Mon objectif était d'arriver à ce stade où l'on peut discuter concrètement des améliorations!

Par rapport aux intérractions clavier, elles sont un peu codifiée par le W3C et les
ARIA Authoring Practices

Dans le cadre de ce composant, j'ai suivi celle ci :

https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-autocomplete-none/

Le "ENTER" est déja utilisé pour autre chose.

Le truc c'est que le bouton "+" ne fait pas partie des cas décrit et j'ai donc demandé un avis lundi lors d'une session de la "clinique de l'accessibilité" proposée par l'équipe accessibilité de beta (d'ailleurs bigup à eux :-D) et comme c'est pas standard bah .... c'est sujet à discussion

On en parle lundi ? Je pense que c'est un sujet qu'il est plus simple de traiter verbalement :-D

Clairement ! Est-ce qu'on peut ouvrir un ticket quelque part, qu'on illustrerait par le travail que tu as fait sur ce composant ?

C'est en discussion! le truc c'est que c'est pas la DINUM qui gère le DSFR mais une équipe du SIG et les synergies entre beta et le SIG restent à construire

SI ça t'intéresse, on en discute ici : https://mattermost.incubateur.net/betagouv/pl/j6xy1ahn1ib9bbj3xbggjwes1h

@johanricher
Copy link
Member

Ca marche, on en parle lundi. Dans l'immédiat je serais partant pour merger ce qui a déjà fait car c'est fonctionnel et répond au besoin qu'on a décrit initialement, Mes remarques pourraient par exemple être relégués à l'état de nice to have dans la description du ticket, qui serait poursuivi ou dépriorisé.

@Volubyl
Copy link
Collaborator

Volubyl commented Feb 2, 2023

ok c'est prod. @DaFrenchFrog si tu as des retours, je pourrai les inclure dans une autre PR.

Je propose de garder l'issue ouverte pour le moment

@DaFrenchFrog
Copy link
Collaborator Author

DaFrenchFrog commented Feb 3, 2023

Hello,

Je viens de regarder staging. Quand je clique sur le champ je n'ai pas la liste des saisies déjà existantes (comme un dropdown classique) c'est normal ? Il est nécessaire de l'avoir car cela évitera une multiplication des doublons.

J'ai testé sous chrome.

@johanricher
Copy link
Member

En effet c'est bien le comportement attendu et c'est ce qu'il se passait hier, mais j'ai l'impression que ça ne marche plus.

@johanricher
Copy link
Member

Il semble y avoir un effet collatéral à la mise en production de #555 et #556 : #559

@Volubyl
Copy link
Collaborator

Volubyl commented Feb 13, 2023

@johanricher j'ai fait une PR d'amélioration c'est sur staging

@johanricher
Copy link
Member

johanricher commented Feb 13, 2023

Je vois que ces 3 points ont été implémentés !

  • Afficher la liste de valeurs quand il y a un focus sur le champ
  • Fermer la liste quand on enlève le curseur (comme c'est le cas par exemple pour le champ licence)
  • Retour à zéro (vider le champ) après que l'utilisateur ait choisit une valeur dans la liste et qu'elle se soit ajoutée en dessous (comme quand on entre une nouvelle valeur, pas dans la liste)

👏

Par contre (je ne sais pas si c'est nouveau car je n'avais pas testé avant), la publication d'une nouvelle fiche avec un mot-clé nouveau (qui n'est pas dans la liste) ne semble pas fonctionner :

  1. Créer une nouvelle fiche avec des valeurs aléatoires dans tous les champs obligatoires
  2. Mettre une valeur nouvelle dans le champ mot-clé, par exemple "Format XYZ"
  3. Cliquer sur "Publier"
  4. Rien ne se passe

Par ailleurs, un mot-clé qui n'avait pas été enregistré (donc qui n'est utilisé sur aucune fiche) reste dans la liste des valeurs proposées par la suite, et cette fois la publication fonctionne :

  1. Créer une nouvelle fiche avec des valeurs aléatoires dans tous les champs obligatoires
  2. Dans le champ mot-clé, choisir dans la liste la valeur "Format XYZ" que j'avais essayer d'utiliser précédemment sans succès (donc à ce stade aucune fiche n'existe avec ce mot-clé)
  3. Cliquer sur "Publier"
  4. La fiche est publiée

@Volubyl
Copy link
Collaborator

Volubyl commented Feb 14, 2023

@johanricher c'est corrigé et c'est sur staging

@johanricher
Copy link
Member

Pour moi c'est tout bon maintenant. J'ai ajouté #567 dans le backlog, pas dans le scope de ce ticket, mais sur lequel on pourrait travailler à l'avenir.

Merci et LGTM !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants