Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request adds a new Backpex.Fields.Checkgroup field type for handling multiple checkboxes with predefined options. This field provides a simpler, more straightforward alternative to MultiSelect for cases where users need to select multiple values from a flat list of options without search functionality or grouping.
Changes:
- Adds new
Backpex.Fields.Checkgroupfield module with render_value and render_form implementations - Extends HTML form component to support "checkgroup" input type with multiple checkboxes
- Updates demo to replace MultiSelect with Checkgroup for user permissions, including schema and migration changes
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/backpex/fields/checkgroup.ex | New field module implementing checkbox group functionality with options support |
| lib/backpex/html/form.ex | Adds "checkgroup" input type with hidden input for empty arrays and accessible checkbox rendering |
| demo/priv/repo/migrations/20251205230743_set_default_for_permissions.exs | Sets database default to empty array and updates existing NULL values for permissions field |
| demo/lib/demo_web/live/user_live.ex | Replaces MultiSelect with Checkgroup for permissions field with simplified flat options |
| demo/lib/demo/user.ex | Adds Ecto schema default value for permissions field |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The `alias Backpex.HTML` is already injected by `use Backpex.Field` via BackpexWeb's field helpers. Drop the duplicate alias; sibling fields like Select and Text rely on the injected alias as well.
Compare option values and the current selection as strings, matching the coercion already performed by Checkgroup.get_labels/2. Without this, atom or integer option values never register as checked when the wrapped value is stored as strings (or vice versa).
Wrap the checkgroup inputs in a <fieldset> with a <legend> so the
group has a native accessible name. Remove the `{@rest}` spread on
individual checkboxes: group-level attributes such as
aria-labelledby or required must not land on every box.
Nest the checkbox input inside its <label> for a robust implicit
association (no more stale `for=` pair) and add `min-h-11` to
satisfy the 44px touch-target requirement.
Drop `aria-labelledby` from Checkgroup.render_form/1 — the <legend>
now provides the accessible name natively.
Each checkbox now advertises its invalid state via aria-invalid when the field has errors, and points at the help text via aria-describedby when one is provided. Give the help_text component an optional `id` attribute so we can target it stably as `<id>-help`.
Add a `readonly` option to Checkgroup's config schema, pass it
through Checkgroup.render_form/1 to BackpexForm.input, and honor it
on each checkbox via `disabled={@readonly}`. Matches the convention
already used by the Text and Select fields.
Because `{@rest}` is no longer spread onto individual checkboxes,
the checkgroup clause pulls `readonly` out of `@rest` explicitly.
Call out in the moduledoc that users must set `default: []` on both the Ecto schema field and the SQL column, otherwise unchecking all boxes will persist NULL instead of an empty array — a consequence of Ecto's `filter_empty_values/3` treating the hidden sentinel input's scalar "" as empty.
List Backpex.Fields.Checkgroup alongside the other built-in field types so it's discoverable from the fields guide.
The `min-h-11` touch target already enforces 44px rows, so the additional `space-y-2` on the options wrapper produced ~52px rows which felt too airy. Drop the extra gap and let the min-height carry the rhythm.
The 44px min-height is a mobile touch-target requirement; on desktop it makes rows feel overly tall. Drop the min-height at the `md:` breakpoint and up.
pehbehbeh
approved these changes
Apr 22, 2026
Comment on lines
+12
to
+14
| def down do | ||
| execute "ALTER TABLE users ALTER COLUMN permissions DROP DEFAULT" | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.