widgets: add MultiSelectWidget (#75655) #10

Merged
lguerin merged 1 commits from wip/75655-MultiSelectWidget into main 2023-03-31 16:31:34 +02:00
6 changed files with 157 additions and 0 deletions

1
.gitignore vendored
View File

@ -5,6 +5,7 @@
/gadjo.egg-info
/gadjo/locale/fr/LC_MESSAGES/django.mo
/gadjo/static/css/gadjo.css
/gadjo/static/css/gadjo.multiselectwidget.css
/gadjo/static/css/icons
node_modules
MANIFEST

0
gadjo/forms/__init__.py Normal file
View File

46
gadjo/forms/widgets.py Normal file
View File

@ -0,0 +1,46 @@
import django
from django import forms
class MultiSelectWidget(forms.MultiWidget):
template_name = 'gadjo/widgets/multiselectwidget.html'
class Media:
js = ('js/gadjo.multiselectwidget.js',)
css = {'all': ('css/gadjo.multiselectwidget.css',)}
def __init__(self, attrs=None):
self.attrs = attrs
widgets = [forms.Select(attrs=attrs)]
super().__init__(widgets, attrs)
def get_context(self, name, value, attrs):
if not isinstance(value, list):
value = [value]
self.widgets = []
for _ in range(max(len(value), 1)):
self.widgets.append(forms.Select(attrs=self.attrs, choices=self.choices))
# all subwidgets must have the same name
if django.VERSION >= (3, 1):
self.widgets_names = [''] * len(self.widgets)
return super().get_context(name, value, attrs)
else:
context = super().get_context(name, value, attrs)
subwidgets = context['widget']['subwidgets']
for widget in subwidgets:
widget['name'] = widget['name'].rsplit('_', 1)[0]
return context
def decompress(self, value):
return value or []
def value_from_datadict(self, data, files, name):
values = [x for x in data.getlist(name) if x]
# remove duplicates while keeping order
return list(dict.fromkeys(values))
def id_for_label(self, id_):
return id_

View File

@ -0,0 +1,35 @@
.gadjo-multi-select-widget {
&--field {
margin-bottom: 0.2em;
}
&--select-button-container {
display: flex;
gap: 0.5em;
}
&--field {
select {
min-width: 0;
}
button {
margin-top: auto;
margin-bottom: auto;
}
&:first-of-type .gadjo-multi-select-widget--button-remove {
display: none;
}
}
&--button-add::before {
content: "\f067"; /* plus */
font-family: FontAwesome;
}
&--button-remove::before {
content: "\f068"; /* minus */
font-family: FontAwesome;
}
}

View File

@ -0,0 +1,58 @@
const multiSelectWidget = (function () {
const addRow = function () {
const widget = this.closest('.gadjo-multi-select-widget')
event.preventDefault()
/* get last row node */
const rows = widget.querySelectorAll('.gadjo-multi-select-widget--field')
const lastRow = rows[rows.length - 1]
/* clone the row */
const newRow = lastRow.cloneNode(true)
/* set new label and ids */
const rowLabel = widget.dataset.rowLabel
const newLabel = rowLabel + ' ' + rows.length
newRow.querySelector('label').textContent = newLabel
const rowId = widget.dataset.rowId
const newId = rowId + '_' + rows.length
newRow.querySelector('label').setAttribute('for', newId)
newRow.querySelector('select').setAttribute('id', newId)
/* add new row after the last row */
lastRow.parentNode.insertBefore(newRow, lastRow.nextSibling)
const removeButton = newRow.querySelector('.gadjo-multi-select-widget--button-remove')
removeButton.addEventListener('click', removeRow)
}
const removeRow = function (event) {
event.preventDefault()
const field = this.closest('.content')
let row = this.closest('.gadjo-multi-select-widget--field')
row.remove()
field.dispatchEvent(new Event('change'))
}
const init = function (container) {
const widgets = container.querySelectorAll('.gadjo-multi-select-widget')
if (!widgets.length) return
widgets.forEach(function (widget) {
const deletBtn = widget.querySelector('.gadjo-multi-select-widget--button-remove')
const addBtn = widget.querySelector('.gadjo-multi-select-widget--button-add')
addBtn.removeEventListener('click', addRow)
addBtn.addEventListener('click', addRow)
deletBtn.removeEventListener('click', removeRow)
deletBtn.addEventListener('click', removeRow)
})
}
return {
init,
}
})()
window.addEventListener('DOMContentLoaded', () => multiSelectWidget.init(document))

Il manque plein de lignes ici pour que ça fonctionne toujours dans combo, peut-être que c'est voulu puisque le côté combo est marqué « WIP » mais je trouve ça bof, il faudrait un truc testable en local même avant de merger juste cette partie.

Et idéalement pour pouvoir suivre les modifs, côté combo il y aurait un commit préliminaire « genericize widget » et le commit gadjo serait juste le résultat d'un mv et d'un s/combo/gadjo :) (ou alors on se dit que pour relire ce genre de patch c'est au relecteurice d'être particulièrement attentif voire de générer les diffs manquants à la mano, pourquoi pas)

Il manque plein de lignes ici pour que ça fonctionne toujours dans combo, peut-être que c'est voulu puisque le côté combo est marqué « WIP » mais je trouve ça bof, il faudrait un truc testable en local même avant de merger juste cette partie. Et idéalement pour pouvoir suivre les modifs, côté combo il y aurait un commit préliminaire « genericize widget » et le commit gadjo serait juste le résultat d'un mv et d'un s/combo/gadjo :) (ou alors on se dit que pour relire ce genre de patch c'est au relecteurice d'être particulièrement attentif voire de générer les diffs manquants à la mano, pourquoi pas)

Yep, j'avais oublié une modif en local :/
C'est poussé.

La PR combo est notée WIP, à cause du commit que j'ai ajouté pour pouvoir descendre la bonne branche de gadjo pour jouer les tests.

côté combo il y aurait un commit préliminaire « genericize widget » et le commit gadjo serait juste le résultat d'un mv et d'un s/combo/gadjo

C'est à peu près le cas: côté gadjo, mv + sed, côté combo, rm + sed.

Yep, j'avais oublié une modif en local :/ C'est poussé. La PR combo est notée WIP, à cause du commit que j'ai ajouté pour pouvoir descendre la bonne branche de gadjo pour jouer les tests. > côté combo il y aurait un commit préliminaire « genericize widget » et le commit gadjo serait juste le résultat d'un mv et d'un s/combo/gadjo C'est à peu près le cas: côté gadjo, mv + sed, côté combo, rm + sed.

Dac, j'imaginais qu'on essaierait de garder les bouts qui font référence aux events combo dans combo, tu ne trouves pas que ça risque d'être un peu galère de maintenir du code aussi spécifique dans un module générique ?

Je ne sais pas trop comment réorganiser le code pour séparer générique de spécifique mais j'invoquerais bien @tjund qui a écrit une bonne partie de ce JS

Dac, j'imaginais qu'on essaierait de garder les bouts qui font référence aux events combo dans combo, tu ne trouves pas que ça risque d'être un peu galère de maintenir du code aussi spécifique dans un module générique ? Je ne sais pas trop comment réorganiser le code pour séparer générique de spécifique mais j'invoquerais bien @tjund qui a écrit une bonne partie de ce JS
Outdated
Review

Bon s'il faut générer un diff à la mano, je repasse plus tard, j'ai la tête vraiment dans autre chose.
Mais plutôt ok avec le commentaire de Valentin : la partie gadjo devrait porter la partie générique du script et permettre à chaque brique de l'utiliser avec ses specificités, quit à refactoriser encore un peu plus.

Bon s'il faut générer un diff à la mano, je repasse plus tard, j'ai la tête vraiment dans autre chose. Mais plutôt ok avec le commentaire de Valentin : la partie gadjo devrait porter la partie générique du script et permettre à chaque brique de l'utiliser avec ses specificités, quit à refactoriser encore un peu plus.

Désolée, je ne comprends rien à ce que vous racontez :)
(j'aime pas le JS)

ça m'embête aussi d'avoir des triggers sur des events combo dans gadjo, mais je ne savais pas comment gérer ça; conseils bienvenus.

Désolée, je ne comprends rien à ce que vous racontez :) (j'aime pas le JS) ça m'embête aussi d'avoir des triggers sur des events combo dans gadjo, mais je ne savais pas comment gérer ça; conseils bienvenus.

Bon s'il faut générer un diff à la mano, je repasse plus tard, j'ai la tête vraiment dans autre chose.

Non du tout ça c'était ma première lecture du patch où je pensais devoir aller à la pêche aux modifs, en fait il n'y en a pas donc tout va bien.

Mais plutôt ok avec le commentaire de Valentin : la partie gadjo devrait porter la partie générique du script et permettre à chaque brique de l'utiliser avec ses specificités, quit à refactoriser encore un peu plus.

Ouep c'est là dessus qu'on veut bien ton aide, on ne sait pas comment séparer le code

> Bon s'il faut générer un diff à la mano, je repasse plus tard, j'ai la tête vraiment dans autre chose. Non du tout ça c'était ma première lecture du patch où je pensais devoir aller à la pêche aux modifs, en fait il n'y en a pas donc tout va bien. > Mais plutôt ok avec le commentaire de Valentin : la partie gadjo devrait porter la partie générique du script et permettre à chaque brique de l'utiliser avec ses specificités, quit à refactoriser encore un peu plus. Ouep c'est là dessus qu'on veut bien ton aide, on ne sait pas comment séparer le code

J'ai déplacé les triggers spécifiques à combo dans combo, ça vous va ?

J'ai déplacé les triggers spécifiques à combo dans combo, ça vous va ?

View File

@ -0,0 +1,17 @@
{% load i18n %}
<div class="gadjo-multi-select-widget" data-row-id="{{ widget.name }}" data-row-label="{% trans "Value" %}">
<div class="gadjo-multi-select-widget--fields" role="group" aria-labelledby="{{ widget.name }}_title">
{% for widget in widget.subwidgets %}
<div class="gadjo-multi-select-widget--field">
<label for="{{ widget.name }}_{{ forloop.counter }}" class="sr-only">{% trans "Value" %} {{ forloop.counter }}</label>
<div class="gadjo-multi-select-widget--select-button-container">
{% include widget.template_name %}
<button type="button" name="{{ widget.name }}$remove_element" class="gadjo-multi-select-widget--button-remove" title="{% trans "Remove" %}" aria-label="{% trans "Remove value" %} {{ forloop.counter }}"></button>
</div>
</div>
{% endfor %}
</div>
<button type="button" name="{{ widget.name }}$add_element" class="gadjo-multi-select-widget--button-add" title="{% trans "Add" %}" aria-label="{% trans "Add" %}"></button>
</div>