diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a5c743..475177a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the Control's own visible `label:` / `hint:` get stable ids and the field wires `aria-labelledby` / `aria-describedby` at them (no duplicate markup). Absent a name, output is unchanged. +- **`checkbox_group` `item_label:`** — the per-item text accessor, so a + `f.field(:tags, as: :checkbox_group, label: "Tags", item_label: ->(t){…})` + renders a visible group heading (`label:`, via the Control) AND custom item + labels at once — previously `label:` on the `f.field` path was eaten by the + heading and items fell back to `to_s`. `item_label:` wins over `label:` for the + item text; on the bare verb `label:` stays the item accessor and `item_label:` + is an alias. Absent it, behavior is unchanged. ### Changed diff --git a/README.md b/README.md index a7b7b4d..3ffad23 100644 --- a/README.md +++ b/README.md @@ -368,9 +368,13 @@ f.checkbox_group(:tag_ids, Tag.all, value: :id, variant: :pill, # :stack (default) | :inline | :pill size: :sm, # daisyUI checkbox size aria: { label: "Tags" }) # names the group for screen readers -# ...or through field inference (the field's label/hint name the group): +# ...or through field inference. Here `label:`/`hint:` are the field's VISIBLE +# heading + description (rendered by the Control, which also names the group); +# `item_label:` gives the per-item text, so you get a heading AND custom item +# labels at once: f.field :tag_ids, as: :checkbox_group, collection: Tag.all, value: :id, - label: "Tags", hint: "Pick any" + label: "Tags", hint: "Pick any", + item_label: ->(t) { t.name.presence || t.slug }, variant: :pill f.collection_select(:country_id, Country.all, :id, :name, prompt: "Select…") ``` @@ -381,6 +385,13 @@ set comes from the model's current value matched by each item's resolved `value:` — re-rendering an edit form pre-checks the right boxes. The `:pill` variant styles the active chip with Tailwind's `has-[:checked]:` (no JS). +**Two labels, no collision.** Through `f.field`, `label:` is the field's visible +group heading (the Control renders it); the per-item text comes from `item_label:` +(a Symbol method or Proc). On the bare `f.checkbox_group` verb there is no Control +heading, so `label:` *is* the per-item accessor (and `item_label:` is accepted as +an alias). Either way `value:` is the submitted value; `item_label:` wins over +`label:` for the item text when both are present. + A `role="group"` needs an **accessible name** for assistive tech. The verb has no bespoke naming option — HTML/ARIA attributes pass straight through to the group, so name it with plain `aria:` (`aria: { label: "Tags" }` for a literal diff --git a/lib/forms/checkbox_group.rb b/lib/forms/checkbox_group.rb index fb3f09f..d025fb2 100644 --- a/lib/forms/checkbox_group.rb +++ b/lib/forms/checkbox_group.rb @@ -8,10 +8,13 @@ module Forms # resolved value: of its item against the model's current set. # # f.checkbox_group(:tag_ids, Tag.all, value: :id, - # label: ->(t) { t.name.presence || t.slug }, variant: :pill, size: :sm) + # item_label: ->(t) { t.name.presence || t.slug }, variant: :pill, size: :sm) # - # value: method or proc -> the submitted value of each item (default :id) - # label: method or proc -> the visible text of each item (default :to_s) + # The item value/text accessors (value:/label:/item_label:) live on the BUILDER + # (Forms::Field#checkbox_group), which pre-resolves each item to + # { value:, label:, checked:, id: } before this leaf renders. This leaf is + # presentation-only — it receives the resolved options: array, never the raw + # accessors. # variant: :stack (default) | :inline | :pill — layout only, zero JS # size: daisyUI checkbox size modifier (:xs :sm :md :lg :xl) # diff --git a/lib/forms/field.rb b/lib/forms/field.rb index 199a0c3..bb85d7f 100644 --- a/lib/forms/field.rb +++ b/lib/forms/field.rb @@ -112,8 +112,20 @@ def tag_field(*modifiers, suggestions: [], **) # # field.checkbox_group(Tag.all, value: :id, label: ->(t) { t.name }) # - # value:/label: are a method name (Symbol) or a proc taking the item. - def checkbox_group(collection, value: :id, label: :to_s, **) + # value: is a method name (Symbol) or a proc taking the item -> its submitted + # value. The per-item visible text comes from item_label: (Symbol/Proc/String) + # if given, else label:; when NEITHER is given each item is labelled by the + # first of name/title/label/to_s it responds to (the same LABEL_METHODS chain + # Inference uses for association choices) — so a plain + # `f.field(:tags, as: :checkbox_group, label: "Tags")` shows readable item + # text without an explicit accessor. + # + # item_label: exists so the `f.field` path can pass a visible group heading as + # `label:` (consumed by the Control) AND still customize the per-item text + # here — the two no longer collide. item_label: is consumed here; it never + # leaks to the group div. + def checkbox_group(collection, value: :id, label: nil, item_label: nil, **) + text = item_label || label # The model's current value is already the raw values (e.g. record.tag_ids # => [1, 3]), so compare against them directly — don't re-resolve value:. selected = Array(field_value) @@ -121,7 +133,7 @@ def checkbox_group(collection, value: :id, label: :to_s, **) item_value = resolve_item(item, value) { value: item_value, - label: resolve_item(item, label), + label: text ? resolve_item(item, text) : infer_item_label(item), checked: selected.include?(item_value), id: "#{field_id}_#{item_value}" } @@ -256,9 +268,25 @@ def conditional?(validator) validator.options.key?(:if) || validator.options.key?(:unless) || validator.options.key?(:on) end - # value:/label: for checkbox_group: a Proc taking the item, or a method name. + # value:/label:/item_label: for checkbox_group. A Proc is called with the + # item; a String is literal text (the same for every item — no method + # dispatch, so a stray string can't NoMethodError); anything else (a Symbol) + # is sent to the item as a method name. def resolve_item(item, accessor) - accessor.respond_to?(:call) ? accessor.call(item) : item.public_send(accessor) + case accessor + when Proc then accessor.call(item) + when String then accessor + else item.public_send(accessor) + end + end + + # Default per-item label when no label:/item_label: was given: the first of + # name/title/label/to_s the item responds to (mirrors PhlexForms::Inference's + # LABEL_METHODS for association choices, so option text is picked the same way + # across the gem). + def infer_item_label(item) + method = PhlexForms::Inference::LABEL_METHODS.find { |m| item.respond_to?(m) } + item.public_send(method || :to_s) end def field_attributes diff --git a/spec/forms/checkbox_group_spec.rb b/spec/forms/checkbox_group_spec.rb index a694c73..a2acea4 100644 --- a/spec/forms/checkbox_group_spec.rb +++ b/spec/forms/checkbox_group_spec.rb @@ -225,4 +225,89 @@ end end end + + # Feedback follow-up: the f.field path took `label:` as the visible Control + # heading, so the per-item label proc had nowhere to go (items fell back to + # to_s). `item_label:` supplies the per-item text alongside a visible heading — + # the marketplace tag-picker shape (heading + custom item labels) in one call. + describe "item_label: (visible heading + custom item labels at once)" do + it "renders the Control heading AND custom item labels via f.field" do + output = render_form(user) do |f| + f.field(:tag_ids, as: :checkbox_group, collection:, value: :id, + label: "Tags", hint: "Pick any", + item_label: ->(t) { t.name || t.slug }) + end + + # The visible Control heading (label:) is present and names the group. + expect(output).to include(">Tags") + expect(output).to match(/role="group"[^>]*aria-labelledby="user_tag_ids_label"/) + # Item labels come from the proc — the third item's name is nil -> its slug. + expect(output).to include(">Ruby<") + expect(output).to include(">hotwire<") # name nil -> slug fallback + # NOT the struct's to_s (the pre-fix behavior). + expect(output).not_to include("#ruby<") + expect(output).to include(">rails<") + end + + it "on the bare verb, item_label: is an alias for the item accessor" do + output = render_form(user) do |f| + f.checkbox_group(:tag_ids, collection, value: :id, + item_label: :slug, aria: { label: "Tags" }) + end + + expect(output).to include(">ruby<") + expect(output).to include(">hotwire<") + end + + it "item_label: wins over label: when both reach the group" do + # label: :name would give nil for the third item; item_label: forces slug. + output = render_form(user) do |f| + f.checkbox_group(:tag_ids, collection, value: :id, label: :name, item_label: :slug) + end + + expect(output).to include(">hotwire<") # slug, not the nil name + end + + it "keeps an explicit label: accessor when item_label: is absent" do + # Backward compatible with the bare-verb usage. + output = render_form(user) do |f| + f.checkbox_group(:tag_ids, collection, value: :id, label: :name) + end + + expect(output).to include(">Ruby<") + expect(output).to include(">Rails<") + end + + it "infers item text (name/title/label/to_s) when neither label: nor item_label: given" do + # The heading-only f.field case: label: is the Control heading, so nothing + # reaches the group as an item accessor — it should still read the item's + # name, not dump #. + output = render_form(user) do |f| + f.field(:tag_ids, as: :checkbox_group, collection:, value: :id, label: "Tags") + end + + expect(output).to include(">Ruby<") # item.name, inferred + expect(output).to include(">Rails<") + expect(output).not_to include("#Pick me<").size).to eq(2) + end + end end