Development #33680
styler de la fonctionnalité +1 dans les formulaires
0%
Description
Sur la base des elements posés dans https://dev.entrouvert.org/projects/publik/wiki/+1_pour_les_signalements
Fichiers
Révisions associées
Historique
Mis à jour par Serghei Mihai il y a plus de 4 ans
- Fichier 0001-forms-decorate-nearby-field-33680.patch 0001-forms-decorate-nearby-field-33680.patch ajouté
- Fichier signalement-lomme.png signalement-lomme.png ajouté
- Fichier signalement-montreuil.png signalement-montreuil.png ajouté
- Fichier signalement-nancy.png signalement-nancy.png ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Reprise de la partie Javascript du thème Toodego.
Les libellé et le sommaire des demandes voisines seront à poser dans des variables de traitement au début du workflow.
Exemples de quelques rendus.
Mis à jour par Frédéric Péters il y a plus de 4 ans
Appeler ça map--nearby-list.html, par rapport au nommage dans toodego nearby-list.html, ça ne va pas prendre le pas et casser Toodego ? Aussi le patch s'applique sur un fichier qui existerait déjà avant alors que non ?
Le côté tableau pas aligné est pas terrible. Plutôt qu'imposer des identifiants de champs (problem_title, problem_summary) pourquoi ne pas utiliser le digest, qui propose déjà un endroit pour définir un résumé succinct du contenu ? (ça empêche pas de taper ça dans un {% block whatever %} pour facilement pouvoir être adapté ailleurs.
Ailleurs, sur la généralisation du signalement (#33595), je note « (peut-être?) rendre optionnel l'affichage du "plus1" », perso ça m'irait bien ça impose moins de chose sur le workflow et ça nous évite de trop nous avancer dans ce bricolage.
Dans ce même autre ticket je note à propos du js "vraisemblablement le placer dans le gabarit" mais ce n'est quand même pas terrible à gérer, ça pourrait aller dans un fichier js/whatever et qu'il y ait juste <script src="..."> utilisé ? (oui il y a dedans un {% if user.is_authenticated %} à gérer différemment).
&.marker-counter {
background: $button-background;
padding: 5px 10px;
color: white;
plutôt utiliser $button-color, pour éviter le blanc sur blanc.
background: lighten($primary-color, 10);
Le +1 apparait déjà comme un bouton via la classe pk-button, qui va assurer que $button-hover-background soit utilisé, c'est mieux que venir imposer ici une couleur alternative.
Sur les trucs flex, en attendant de peut-être avoir un truc pour gérer ça automatiquement, il faut ajouter les préfixes pour les vieux IE.
Mis à jour par Serghei Mihai il y a plus de 4 ans
- Fichier 0001-forms-decorate-nearby-field-33680.patch 0001-forms-decorate-nearby-field-33680.patch ajouté
Je suis plutôt pour garder le JS inline pour maîtriser le détection d'un utilisateur connecté, ajout du bout d'ajax pour l'action +1 si elle est active.
Mis à jour par Frédéric Péters il y a plus de 4 ans
maîtriser le détection d'un utilisateur connecté
Il y a une classe posée sur <body> qui peut t'assurer ça. (authenticated-user vs anonymous-user).
Mis à jour par Frédéric Péters il y a plus de 4 ans
(d'ailleurs j'écrivais ça sans regarder mais c'est exactement ce qui est fait dans le code existant côté toodego)
Mis à jour par Serghei Mihai il y a plus de 4 ans
- Fichier 0001-forms-decorate-nearby-field-33680.patch 0001-forms-decorate-nearby-field-33680.patch ajouté
Ok.
Mis à jour par Frédéric Péters il y a plus de 4 ans
Je posais la question :
Appeler ça map--nearby-list.html, par rapport au nommage dans toodego nearby-list.html, ça ne va pas prendre le pas et casser Toodego ?
En l'état, ça casse Toodego mais c'est plutôt à cause des CSS, ce serait chouette d'éviter (et la voie de sortie la plus simple est peut-être d'appeler ça différemment, genre nearby-items.html ?).
J'écrivais aussi :
Sur les trucs flex, en attendant de peut-être avoir un truc pour gérer ça automatiquement, il faut ajouter les préfixes pour les vieux IE.
Et c'est encore valable dans div#nearby-list. Aussi on met les propriétés préfixées en premier.
~~
À part ça je ne sais pas comment ça marche chez toi, côté firefox et chromium il ne prend pas le js sans faire ça :
-<script type="text/javascript" src="{% static "js/plus1.js" %}" />
+<script type="text/javascript" src="{% static "js/plus1.js" %}"></script>
(aussi, le js n'est pas seulement utilisé pour les +1, devrait tout le temps être chargé)
Mis à jour par Thomas Jund (congés, retour le 29/04) il y a plus de 4 ans
-<script type="text/javascript" src="{% static "js/plus1.js" %}"></script>
+<script src="{% static "js/plus1.js" %}"></script>
l'attribut type n'est plus utile avec un doctype html5. Et oui la balise doit se fermer.
Mis à jour par Serghei Mihai il y a plus de 4 ans
- Fichier 0001-forms-decorate-nearby-field-33680.patch 0001-forms-decorate-nearby-field-33680.patch ajouté
Etrange effectivement car sans la balise fermante mon firefox chargeait le JS.
J'ai rénommé le template en nearby-forms pour plus de clarté.
Préfixes IE pour les flexbox ajoutés.
Mis à jour par Frédéric Péters il y a plus de 4 ans
Il y a moyen de limiter div.leaflet-div-icon aussi (parce que là, ça casse aussi toodego) ?
Mis à jour par Thomas Jund (congés, retour le 29/04) il y a plus de 4 ans
JS l 2-3
function user_is_authenticated() {
- return $('body.authenticated-user').length;
+ return $('body').is('.authenticated-user');
}
C'est pas plus lisible ? au moins on retourne un booleen et pas un number.
Ou en full JS
document.body.classList.contain('.authenticated-user');
Mis à jour par Serghei Mihai il y a plus de 4 ans
- Fichier 0001-forms-decorate-nearby-field-33680.patch 0001-forms-decorate-nearby-field-33680.patch ajouté
Avec limitation de la feuille de style aux markers du widget et la suggestion de ThomasJ pour la fonction user_is_authenticated.
Mis à jour par Thomas Jund (congés, retour le 29/04) il y a plus de 4 ans
Autre petite proposition d'amélioration Js
-var $map_widget = $('.qommon-map');
-var map = $map_widget[0].leaflet_map;
+var map = document.querySelector('.qommon-map').leaflet_map;
Tu charges un DomElement dans un objet jQuery ($map), pour ensuite chercher un objet de ce même domElement au sein de l'objet jQuery (map), Pour ensuite dans le code réinjecté cet élément dans un nouvel objet jQuery ($(map)).
Parfois Js natif est plus simple, non ?
Mis à jour par Thomas Jund (congés, retour le 29/04) il y a plus de 4 ans
Autres suggestions au niveau HTML:
Un peu plus de sémantique ne fait jamais de mal.
Utiliser une ul > li pour les id="nearby-forms"
<ul id="nearvy-forms">
<li …
<li
le +1 ne devrait pas être une balise <a> puisqu'il ne mène nul part. La bonne pratique serait d'utiliser <button>
<button type="button" class="plus1 pk-button" …>+1</button>
<a class="plus1 pk-button" href="#" data-plus1-url="{{formdata.api_url}}hooks/plus1/">+1</a>
Et avec un attribut aria-label explicite. "+1" Ne l'est pas assez je pense.
La span comprenant la date devrait afficher la date au sein d'une balise <time>
<span class="problem-datetime">Signalé le <time datetime="{{ formdata.receipt_date|date:"c" }}" >{{ formdata.receipt_date }}</time></span>
Et enfin (promis après j'arrête)
Pour centrer un flex-item unique verticalement et horizontalement, il suffit de lui appliqué un margin:auto
div.template-nearby-forms div.leaflet-div-icon {
- padding: 3px;
border-radius: 3px;
display: -ms-flexbox;
display: flex;
- -ms-flex-pack: space-around;
- -ms-justify-content: space-around;
- justify-content: space-around;
a {
- line-height: 1em;
+ line-height: 1;
+ margin: auto;
}
}
Mis à jour par Serghei Mihai il y a plus de 4 ans
- Fichier 0001-forms-decorate-nearby-field-33680.patch 0001-forms-decorate-nearby-field-33680.patch ajouté
Thomas Jund a écrit :
Autres suggestions au niveau HTML:
Un peu plus de sémantique ne fait jamais de mal.
Utiliser une ul > li pour les id="nearby-forms"
Ok.
le +1 ne devrait pas être une balise <a> puisqu'il ne mène nul part. La bonne pratique serait d'utiliser <button>
Un button signifie une action, hors ici il s'agit d'un présentation de l'index de la demande. J'utilise plutôt un span.
[...]
Et avec un attribut aria-label explicite. "+1" Ne l'est pas assez je pense.
Je rajoute "Même signalement".
La span comprenant la date devrait afficher la date au sein d'une balise <time>
Ok.
Pour centrer un flex-item unique verticalement et horizontalement, il suffit de lui appliqué un margin:auto
Ok, mais je garde le padding pour que la bordure ne colle pas trop au texte.
Mis à jour par Serghei Mihai il y a plus de 4 ans
Mis à jour par Thomas Jund (congés, retour le 29/04) il y a plus de 4 ans
Débat sur la convention de nommage ouvert : https://dev.entrouvert.org/issues/36831
Sinon à corriger html l37
- <button class="plus1" href="#" data-plus1-url="{{formdata.api_url}}hooks/plus1/" aria-label="Même signalement">+1</button>
+ <button role="button" class="plus1" data-plus1-url="{{formdata.api_url}}hooks/plus1/" aria-label="Même signalement">+1</button>
Ajouter au <button> un role=button. Oui c'est bizarre mais par defaut il a un "role=submit". Et enlever l'attribut href.
Mis à jour par Serghei Mihai il y a plus de 4 ans
- Fichier 0001-forms-decorate-nearby-field-33680.patch 0001-forms-decorate-nearby-field-33680.patch ajouté
En attendant qu'on se mette d'accord j'ai explicité les noms des classes.
Et la correction du bouton.
Mis à jour par Thomas Jund (congés, retour le 29/04) il y a plus de 4 ans
Astuce,
tu peux continuer à utiliser le nesting de sass sans générer de selecteur enfant.
Pour arriver au même résultat que tu as là, ajoute juste une class .nearby-forms à ton element #nearby-forms et une class nearby-forms-list à l'élément li
et tu peux écrire
.nearby-forms {
margin: 0;
padding: 0;
&-list {…}
&-digest, &-datetime, &-marker-counter {…}
&-digest {…}
&-datetime {…}
&-marker-counter {…}
}
Mis à jour par Serghei Mihai il y a plus de 4 ans
Oui, mais ça réduit beaucoup la lisibilité je trouve.
Mis à jour par Thomas Jund (congés, retour le 29/04) il y a plus de 4 ans
Niveau convention, on est parti sur un format block--element.
Tu pourrais donc partir sur des noms de class comme
<section
<div id="map-{{widget.name}}" class="qommon-map nearby-forms--map">
<ul class="nearby-forms">
<li class="nearby-form">
<span class="nearby-form--marker-counter" >
<span class="nearby-form--digest">
<span class="nearby-form--datetime">
<button class="nearby-form--plus1">
Mis à jour par Serghei Mihai il y a plus de 4 ans
- Fichier 0001-forms-decorate-nearby-field-33680.patch 0001-forms-decorate-nearby-field-33680.patch ajouté
Ok.
Mis à jour par Thomas Jund (congés, retour le 29/04) il y a plus de 4 ans
Je me suis rendu compte dernièrement que IE11 était compatible avec la spec flexbox. Il n'est donc plus nécessaire de mettre les -ms-trucmuch qui sont nécessaires uniquement pour le support de IE10 (et qu'on ne support plus).
Mis à jour par Serghei Mihai il y a plus de 4 ans
D'autres remarques? J'aimerais bien envoyer en recette rapidement.
Mis à jour par Thomas Jund (congés, retour le 29/04) il y a plus de 4 ans
- Statut changé de Solution proposée à Solution validée
Mis à jour par Serghei Mihai il y a plus de 4 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit 80bea41c15c64f84a1aba0b0ac75a8043e6852b8 (origin/master, origin/HEAD)
Author: Serghei Mihai <smihai@entrouvert.com>
Date: Wed Jun 5 10:31:14 2019 +0200
forms: decorate "nearby" field (#33680)
Mis à jour par Frédéric Péters il y a plus de 4 ans
- Statut changé de Résolu (à déployer) à Solution déployée
forms: decorate "nearby" field (#33680)