Conversation
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.
| alias Backpex.HTML | ||
|
|
There was a problem hiding this comment.
The alias Backpex.HTML is redundant as it's already aliased by use Backpex.Field. Consider removing this line to match the pattern used in other field modules like Text, Select, Boolean, etc.
| alias Backpex.HTML |
| id={"#{@id}-#{value}"} | ||
| name={@name <> "[]"} | ||
| value={value} | ||
| checked={value in List.wrap(@value)} |
There was a problem hiding this comment.
The checked attribute compares values without type conversion (value in List.wrap(@value)), but the get_labels function converts both to strings for comparison. This inconsistency could cause checkboxes to not be checked correctly when option values and stored values have different types (e.g., atoms vs strings). Consider converting both to strings for comparison: to_string(value) in (List.wrap(@value) |> Enum.map(&to_string/1)).
| checked={value in List.wrap(@value)} | |
| checked={to_string(value) in (List.wrap(@value) |> Enum.map(&to_string/1))} |
No description provided.