Development #57760
Couche cartographique : enlever les "propriétés" pour les mettre dans la cellule carte
0%
Description
Pour clarifier cet écran, enlever les propriétés et les déporter dans la cellule "Carte".
Fichiers
Révisions associées
maps: duplicate map layer options (#57760)
Historique
Mis à jour par Valentin Deniaud il y a plus de 2 ans
- Fichier 0001-maps-move-properties-from-layer-to-cell-57760.patch 0001-maps-move-properties-from-layer-to-cell-57760.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
En espérant avoir bien compris le ticket.
Mis à jour par Frédéric Péters il y a plus de 2 ans
Il y a différentes choses, en fait les propriétés elles sont avant tout là pour déterminer ce qui apparaitra en popup. Donc ça serait plutôt à afficher uniquement selon la valeur posée dans marker_behaviour_onclick. Surtout ça doit quand même rester associé à la couche (on peut avoir une couche "parking voiture" et "parking vélo" avec des propriétés différentes à afficher), ça fait que ça devrait plutôt aller dans MapLayerOptions.
Aussi, c'est éventuellement à faire en rapport avec #58072, en plus des propriétés définies ici qui déterminent ce qu'il y aura dans la popup, il y a marker_colour à également inclure,
- 'properties': [x.strip() for x in l.properties.split(',')], + 'properties': [x.strip() for x in self.properties.split(',')],
mais ça ne fera à mon avis que ramener vers #58072, parce que le js il va juste afficher tout ce qui est dispo dans la popup.
Bref tu dois inclure ici marker_colour pour ne pas casser #57296 mais ça ne résoud pas #58072.
Mis à jour par Valentin Deniaud il y a plus de 2 ans
Frédéric Péters a écrit :
ça fait que ça devrait plutôt aller dans MapLayerOptions.
Yep je vais faire ça.
Aussi, c'est éventuellement à faire en rapport avec #58072, en plus des propriétés définies ici qui déterminent ce qu'il y aura dans la popup, il y a marker_colour à également inclure,
[...]
mais ça ne fera à mon avis que ramener vers #58072, parce que le js il va juste afficher tout ce qui est dispo dans la popup.
Bref tu dois inclure ici marker_colour pour ne pas casser #57296 mais ça ne résoud pas #58072.
J'ai peu envie de toucher au js ici donc j'ai envie de laisser #58072 tranquille, par contre je n'ai pas envie de casser quelque chose : et là je ne comprends pas du tout ce qui casse ici, le diff que tu as cité me laisse perplexe, je dois manquer de compréhension du code ?
Mis à jour par Frédéric Péters il y a plus de 2 ans
S'il y a une option "attributs à inclure dans la popup" et qu'il y a dedans comme valeur ["titre", "description"] et que ça fait que dans le geojson il y a comme propriétés "titre" et "description", le js qui s'attend à trouver aussi la propriété définie dans marker_colour ne va pas la trouver.
Si alors tu inclus dans le geojson les propriétés "titre" et "description" + la propriété "marker_colour", l'utilisation de celle-ci ne sera pas cassée.
Mais dans la popup il y aura l'info marker_colour également, pas juste "titre" et "description". <-- le statu quo, #58072.
Mis à jour par Valentin Deniaud il y a plus de 2 ans
- Fichier 0002-maps-duplicate-map-layer-options-57760.patch 0002-maps-duplicate-map-layer-options-57760.patch ajouté
- Fichier 0001-maps-move-properties-from-layer-to-cell-57760.patch 0001-maps-move-properties-from-layer-to-cell-57760.patch ajouté
En fait dans
- 'properties': [x.strip() for x in l.properties.split(',')], + 'properties': [x.strip() for x in self.properties.split(',')],
je ne vois vraiment pas ce qui n'est pas équivalent puisque les propriétés sont copiées d'un objet à l'autre par la migration. J'aimerais bien comprendre ce qui « casse » pour pouvoir écrire un test de non régression.
D'ailleurs à tester cette fonctionnalité en local, je n'arrive à rien casser avec mon (nouveau) patch.
Mis à jour par Frédéric Péters il y a plus de 2 ans
L'option dans la carto c'était "propriétés à inclure" (générique, pas précis), ici ça devrait être "propriétés à afficher dans la popup" (via mon commentaire "Donc ça serait plutôt à afficher uniquement selon la valeur posée dans marker_behaviour_onclick.").
Copier les données lors de la migration ça va aller, puis quelqu'un verra "couleur" dans les "propriétés à afficher dans la popup" et ira la retirer et à ce moment le rendu des marqueurs sera cassé, parce que "couleur" était la propriété précisée dans "marker colour".
On a :
- 1/ propriétés dans le geojson
- 2/ propriété "marker colour"
- 3/ propriétés affichées dans la popup
Aujourd'hui on définit 1/, on pense à inclure la propriété du point 2 dedans et les propriétés du point 3, et on a tout ça qui s'affiche dans la popup 3/, qui n'est jamais définie explicitement.
Ici on va définir explicitement le point 3, mais dans le geojson il ne faudrait pas avoir exclusivement les propriétés du point 3, il faudra aussi celle du point 2.
et faire ça fera que dans la popup il y aura les propriétés de 2 et de 3 et c'est le bug et le statu quo #58072.
Mis à jour par Valentin Deniaud il y a plus de 2 ans
Frédéric Péters a écrit :
L'option dans la carto c'était "propriétés à inclure" (générique, pas précis), ici ça devrait être "propriétés à afficher dans la popup"
Dac
(via mon commentaire "Donc ça serait plutôt à afficher uniquement selon la valeur posée dans marker_behaviour_onclick.").
C'était compréhensible dans la première version du patch où le champ propriétés s'affichait dans la cellule, là je trouverais ça très peu clair d'avoir le bouton d'édition de la couche geojson qui apparaît/disparaît en fonction de la valeur de marker_behaviour_onclick. Et quand bien même on ferait ça il faudra trouver une autre idée le jour où il y aura d'autres choses éditables que les propriétés. Donc autant trouver l'idée maintenant : ça pourrait le faire de désactiver le champ si marker_behaviour_onclick est « Aucun », et dans ce cas modifier le help_text pour dire d'aller changer ça avant toute chose ?
Copier les données lors de la migration ça va aller, puis quelqu'un verra "couleur" dans les "propriétés à afficher dans la popup" et ira la retirer et à ce moment le rendu des marqueurs sera cassé, parce que "couleur" était la propriété précisée dans "marker colour".
OK c'est plus clair, j'aurais eu tendance à faire l'hypothèse que #58072 pourrait être traité assez vite après pour que ce genre de malheur n'arrive pas.
Mis à jour par Frédéric Péters il y a plus de 2 ans
Ah oui j'étais sur l'idée que le bouton d'édition était déjà tout le temps présente mais c'est juste là pour le paramètre opacité uniquement pour les couches "tuiles".
Donc si je comprends bien ta proposition elle est de tout le temps l'afficher, mais que dedans il y ait une note sur l'help_text du champ "propriétés pour la popup" rappelant à l'usager que le comportement actuel n'est pas d'afficher une popup. Ça m'irait de s'arrêter à ça et de laisser quand même le champ éditable.
que #58072 pourrait être traité assez vite après
oui c'était mon idée, tu auras vu maintenant un patch de Benjamin dedans, je vous laisse voir entre vous.
Mis à jour par Valentin Deniaud il y a plus de 2 ans
- Fichier 1636542918.png 1636542918.png ajouté
Branche à jour avec ça.
Mis à jour par Frédéric Péters il y a plus de 2 ans
- Statut changé de Solution proposée à Solution validée
Mis à jour par Valentin Deniaud il y a plus de 2 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit efa733b2cf3256f8890955cefa9f1b52d347b6ee Author: Valentin Deniaud <vdeniaud@entrouvert.com> Date: Tue Nov 9 17:17:42 2021 +0100 maps: duplicate map layer options (#57760) commit 04a4ff60d9d63795fb1912418bd2c2af3bad28a9 Author: Valentin Deniaud <vdeniaud@entrouvert.com> Date: Thu Nov 4 16:32:47 2021 +0100 maps: move properties from layer to cell (#57760)
Mis à jour par Frédéric Péters il y a plus de 2 ans
- Statut changé de Résolu (à déployer) à Solution déployée
maps: move properties from layer to cell (#57760)