feat: checkbox_group item_label — visible heading + custom item text at once - #19
Merged
Merged
Conversation
…ext at once f.field(:x, as: :checkbox_group, label:) took `label:` as the visible Control heading, so the per-item label proc had nowhere to go and items fell back to a struct dump. Add `item_label:` (Symbol/Proc/String) for the per-item text, so a single f.field call renders a visible heading AND custom item labels — the marketplace tag-picker shape the app-local UI::CheckboxGroup existed for. - item_label: wins over label: for the item text; consumed in the builder, never leaks to the group div. - Bare verb keeps label: as the item accessor; item_label: is an alias there. - NEW default: when neither is given, per-item text infers via PhlexForms::Inference::LABEL_METHODS (name/title/label/to_s) — the same chain used for association choices — so a plain f.field(label: "Tags") shows readable item text instead of #<struct ...>. - resolve_item now treats a String as literal text (no method dispatch), so a stray string can't NoMethodError on objects lacking that reader. ## Test Coverage - f.field: visible heading (label:) + custom item labels (item_label: proc/symbol) together; item_label: wins over label:; bare-verb alias; explicit label: kept - heading-only f.field infers item text (no #<struct> dump) - String item_label: is literal (plain objects, no crash) ## Verification - [x] bundle exec rubocop lib spec passes - [x] bundle exec rspec passes (160 examples) - [x] adversarial review: no item_label leak, back-compat intact, footgun fixed
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.
Summary
Feedback follow-up to #9/#17.
checkbox_groupcouldn't give both a visiblegroup heading and custom per-item labels in one call —
label:wasoverloaded (the field heading on
f.field, the per-item accessor on the bareverb), so on
f.fieldthe item text had nowhere to go and fell back to a structdump. This is what the app-local
UI::CheckboxGroupexisted to work around.Add
item_label:(Symbol / Proc / String) for the per-item text, so themarketplace tag-picker is one gem call:
The Control already renders the visible heading and wires
aria-labelledbyat it(#17) — this PR only unblocks the item accessor, so there is no second
heading code path.
Why not the block API
The original sketch was
f.checkbox_group(:x, coll) { |g| g.heading …; g.item … }.We compared it against a flat keyword and chose the keyword: the block would
re-implement the heading/hint the Control already renders (two heading code paths
to keep in sync) and diverge from the flat
f.fieldshape the rest of the gemuses.
item_label:is a one-keyword fix, no new render context.Behavior
item_label:wins overlabel:for item text; it's consumed in the builderand never leaks to the group
div.f.checkbox_group(:x, coll, label: ->(t){…})stilluses
label:as the item accessor;item_label:is accepted there as an alias.label:noritem_label:is given, per-itemtext infers via
PhlexForms::Inference::LABEL_METHODS(name→title→label→to_s), the same chain used for association choices. So a plainf.field(:tags, as: :checkbox_group, label: "Tags")shows readable item textinstead of
#<struct …>(the previous foot-gun).item_label:is literal text (nopublic_send), so a stray stringcan't
NoMethodErroron an object lacking that reader.Test coverage
f.field: visible heading (label:) + custom item labels (item_label:procand symbol) together;
item_label:wins overlabel:; bare-verb alias;explicit
label:preserved.f.fieldinfers item text — no#<struct …>.item_label:is literal (plain objects, no crash).Full suite: 160 examples, 0 failures ·
rubocop lib specclean. Anadversarial review pass independently confirmed no
item_label:leak to thegroup, byte-identical back-compat with no
item_label:, and flagged thestruct-dump foot-gun (fixed here by the inference default).
Deviations & judgment calls
item_label:reusesthe Control heading that already works; the block would duplicate it.
label: :to_sto inference (name/title/label/to_s).An adversarial review and you both flagged the heading-only case dumping
#<struct …>; inferring matches how the gem already labels association choicesand makes the verb a genuine drop-in for
UI::CheckboxGroup. A Struct/PORO withno
name/title/labelstill falls toto_s, so nothing that worked beforeregresses.
item_label:is now literal text, not a method name — the intuitivereading and it removes a
NoMethodErrorfoot-gun the review found.item_label || labelprecedence (any-falsy, not nil-only):true/falseare nonsense for an accessor (the contract is Symbol/Proc/String), so aboolean is a caller error not worth a special case.
Closes #N— this is feedback-driven, not a filed issue.