feat: replace multi-select dropdowns with checkboxes in content pack forms#1542
Conversation
…forms - Swap SelectMultiple for BsCheckboxSelectMultipleCompact on skills, skill categories, and rules fields in ContentFighterPackForm - Add traits as a proper form field on ContentWeaponProfilePackForm with pack-aware queryset, replacing hand-rolled wp_traits POST handling - Group skills by category using group_select() - New compact checkbox widget with scrollable bordered container, client-side search filter with clear button, and empty state Closes #1528
There was a problem hiding this comment.
Pull request overview
This PR replaces multi-select <select multiple> dropdowns with a new compact checkbox-based multi-select widget (BsCheckboxSelectMultipleCompact) across content pack forms. It also moves weapon profile traits management from manual template-level POST handling into a proper Django form field with a pack-aware queryset.
Changes:
- Introduce
BsCheckboxSelectMultipleCompactwidget with search filtering, clear button support, and empty state - Update
ContentFighterPackFormto use the new widget for skills, skill categories, and rules fields - Migrate weapon profile traits into
ContentWeaponProfilePackFormas a proper M2M field with pack-scoped queryset
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
gyrinx/core/forms/__init__.py |
Defines the new BsCheckboxSelectMultipleCompact widget class |
gyrinx/pages/templates/pages/forms/widgets/bs_checkbox_select_compact.html |
Template for the compact checkbox widget container with filter input |
gyrinx/pages/templates/pages/forms/widgets/bs_checkbox_option_compact.html |
Template for individual checkbox option rendering |
gyrinx/core/forms/pack.py |
Updates forms to use new widget; adds traits field + pack-aware queryset to ContentWeaponProfilePackForm |
gyrinx/core/views/pack.py |
Updates view calls to pass pack kwarg and uses form.save_m2m() instead of manual trait handling |
gyrinx/core/templates/core/pack/includes/weapon_profile_stats_form.html |
Converts traits rendering to checkbox list; still uses template-level context variables |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {% if weapon_traits %} | ||
| <div> | ||
| <label class="form-label" for="wp_traits">Traits</label> | ||
| <select name="wp_traits" id="wp_traits" class="form-select" multiple size="6"> | ||
| <label class="form-label">Traits</label> |
There was a problem hiding this comment.
The weapon_profile_stats_form.html template still uses the weapon_traits and selected_trait_ids context variables, but both were removed from the view context in pack.py. Since traits are now rendered via the Django form field, this template block will never render (the {% if weapon_traits %} condition is always false). If this template is still included in the add/edit weapon profile pages, the traits widget from the form should be rendered there instead and this block should be removed entirely.
gyrinx/core/forms/__init__.py
Outdated
|
|
||
|
|
||
| class BsCheckboxSelectMultipleCompact(forms.CheckboxSelectMultiple): | ||
| """Compact checkbox multi-select in a scrollable bordered box with columns.""" |
There was a problem hiding this comment.
The docstring mentions "with columns" but the widget template does not appear to implement a column layout — it renders items in a single scrollable vertical list. The docstring should be updated to accurately describe what the widget does, e.g. "Compact checkbox multi-select in a scrollable bordered box with a search filter."
| """Compact checkbox multi-select in a scrollable bordered box with columns.""" | |
| """Compact checkbox multi-select in a scrollable bordered box with a search filter.""" |
| {% endfor %} | ||
| {% if group %}</div>{% endif %} | ||
| {% endfor %} | ||
| {# djlint:off H021 #} |
There was a problem hiding this comment.
There is a duplicate {# djlint:off H021 #} comment — one on line 8 and another on line 23. The second occurrence on line 23 appears to be a copy-paste error, as the directive was already applied earlier. It should be removed.
| {# djlint:off H021 #} |
| self.initial["traits"] = list( | ||
| self.fields["traits"] | ||
| .queryset.filter(contentweaponprofile=self.instance) | ||
| .values_list("pk", flat=True) |
There was a problem hiding this comment.
The reverse relation name contentweaponprofile used in the filter is an implicit Django-generated name based on the model name. If the model is ever renamed, this will silently break. Consider using the explicit related name if one is defined, or using self.instance.traits.all() with a values_list and then intersecting with the pack-aware queryset (e.g., self.fields["traits"].queryset.filter(pk__in=self.instance.traits.values_list("pk", flat=True))), which is both clearer in intent and avoids reliance on the implicit reverse accessor name.
| self.initial["traits"] = list( | |
| self.fields["traits"] | |
| .queryset.filter(contentweaponprofile=self.instance) | |
| .values_list("pk", flat=True) | |
| trait_qs = self.fields["traits"].queryset | |
| instance_trait_ids = self.instance.traits.values_list("pk", flat=True) | |
| self.initial["traits"] = list( | |
| trait_qs.filter(pk__in=instance_trait_ids).values_list("pk", flat=True) |
tgvashworth
deleted the
1528-make-multi-select-custom-attributes-checkbox-rather-than-cmd-click
branch
Summary
<select multiple>fields with checkbox-based multi-select across content pack forms (skills, skill categories, rules, weapon traits)BsCheckboxSelectMultipleCompactwidget: scrollable bordered container with search filter, clear button, and empty stateTest plan
Closes #1528