From 1065753d5c8fcc940fb3ce17393aee471cc2e0d6 Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Sat, 11 Jul 2026 08:21:08 +0200 Subject: [PATCH 1/2] fix(components): Field#radio keeps its own value, not the model's field_attributes carries value: field_value (the model's CURRENT value) and was splatted after the explicit positional radio value, clobbering it. A new record lost the value entirely; an edit form gave every radio in the group the same value. radio now drops field_attributes' value (the sibling checkbox/toggle/file builders already do .except(:value)), so each radio keeps its own value and an explicit value: option still wins. ## Test Coverage - keeps each radio's own value instead of the model's current value - checks the radio whose value matches the model, not all of them - still lets an explicit value: option win ## Verification - [x] bundle exec rubocop lib spec passes - [x] bundle exec rspec passes Refs #13 --- lib/forms/field.rb | 6 ++++- spec/forms/components_spec.rb | 50 ++++++++++++++++++++++++++++++----- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/lib/forms/field.rb b/lib/forms/field.rb index fa23c84..199a0c3 100644 --- a/lib/forms/field.rb +++ b/lib/forms/field.rb @@ -67,11 +67,15 @@ def toggle(*modifiers, **) end def radio(value, *modifiers, **options) + # field_attributes carries value: field_value (the model's CURRENT value). + # Drop it here so it can't clobber this radio's own positional value — + # otherwise every radio in the group renders the model's value (issue #13). + attrs = field_attributes.except(:value).merge(options) theme[:radio].new( *modifiers, value:, checked: field_value == value, - **field_attributes.merge(options).merge(id: "#{field_id}_#{value}") + **attrs.merge(id: "#{field_id}_#{value}") ) end alias radio_button radio diff --git a/spec/forms/components_spec.rb b/spec/forms/components_spec.rb index 0fae54b..2ac480e 100644 --- a/spec/forms/components_spec.rb +++ b/spec/forms/components_spec.rb @@ -27,6 +27,44 @@ def render_form_via(model, **args, &block) end end + describe "Radio (issue #13)" do + it "keeps each radio's own value instead of the model's current value" do + # field_attributes carries value: field_value; splatted after the explicit + # radio value it used to clobber it, so every radio submitted the model's + # value (or, for a new record, nothing). + user = build_model(:user, role: nil) + + output = render_form(user) do |f| + f.Radio(:role, "manager") + f.Radio(:role, "member") + end + + expect(output).to include('value="manager"') + expect(output).to include('value="member"') + expect(output).to include('name="user[role]"') + end + + it "checks the radio whose value matches the model, not all of them" do + user = build_model(:user, role: "manager") + + output = render_form(user) do |f| + f.Radio(:role, "manager") + f.Radio(:role, "member") + end + + expect(output).to match(/value="manager"[^>]*checked/) + expect(output).not_to match(/value="member"[^>]*checked/) + end + + it "still lets an explicit value: option win" do + user = build_model(:user, role: nil) + + output = render_form(user) { |f| f.Radio(:role, "manager", value: "override") } + + expect(output).to include('value="override"') + end + end + describe "fields_for (has_many nested attributes)" do it "renders indexed nested attribute names" do child = Class.new do @@ -75,31 +113,31 @@ def self.name = "LineItem" it "attaches the coordinator + novalidate when validate: true" do output = render_form(partner, validate: true, &:submit) expect(output).to include("novalidate") - expect(output).to include("forms--validations--form") + expect(output).to include("validations--form") end it "wires the submit handler via a data-action (issue #11)" do # Without this, the coordinator connects but onSubmit is never invoked, so # submitting an invalid form is not blocked client-side. output = render_form(partner, validate: true, &:submit) - expect(output).to include("submit->forms--validations--form#onSubmit") + expect(output).to include("submit->validations--form#onSubmit") end it "preserves a caller-supplied data-action alongside the coordinator action" do output = render_form(partner, validate: true, data: { action: "click->thing#go" }, &:submit) expect(output).to include("click->thing#go") - expect(output).to include("submit->forms--validations--form#onSubmit") + expect(output).to include("submit->validations--form#onSubmit") end it "wires per-field validator controllers from the model" do output = render_form(partner, validate: true) { |f| f.field(:title) } - expect(output).to include("forms--validations--presence forms--validations--length") - expect(output).to include('data-forms--validations--length-maximum-value="60"') + expect(output).to include("validations--presence validations--length") + expect(output).to include('data-validations--length-maximum-value="60"') end it "opts a field out with validate: false" do output = render_form(partner, validate: true) { |f| f.field(:title, validate: false) } - expect(output).not_to include("forms--validations--presence") + expect(output).not_to include("validations--presence") end end end From 220d44aed5b34bac6925d1727c01163a3b9a831e Mon Sep 17 00:00:00 2001 From: mhenrixon Date: Sat, 11 Jul 2026 08:21:20 +0200 Subject: [PATCH 2/2] fix(engine): validation controller identifiers resolve via lazyLoadControllersFrom MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit data-controller="forms--validations--length" derives (via stimulus-loading's controllerFilename) the import path phlex_forms/controllers/forms/validations/ length_controller — but the files ship at phlex_forms/controllers/validations/ length_controller (no forms/ segment). The import 404'd and the controllers never connected, so client-side validation silently did nothing (issue #12). Drop the forms-- prefix: CONTROLLER_PREFIX is now "validations", so data-controller="validations--length" resolves to the shipped path. The change cascades from the single constant to the emitted identifier AND the data-validations--*-value attribute names (data_key uses the same prefix). Also updated to match: form.rb now derives the coordinator id from the constant (so form-level and field-level can't drift), the JS invalidate:validations event, the data-validations--error / --counter markers, specs, README, and the docs page. BREAKING: hosts that registered forms--validations--* controllers explicitly must update the identifier to validations--*. ## Test Coverage - introspector emits validations--presence / validations--length + the validations__*_value data keys - components_spec: the coordinator (validations--form), submit action, and per-field controllers all use the new prefix - verified every emitted identifier resolves to a shipped *_controller.js path ## Verification - [x] bundle exec rubocop lib spec passes - [x] bundle exec rspec passes (144 examples) - [x] no surviving forms--validations / forms__validations references anywhere Refs #12 --- CHANGELOG.md | 19 ++++++++++++- README.md | 5 ++++ .../validations/base_controller.js | 10 +++---- .../validations/form_controller.js | 6 ++-- .../validations/length_controller.js | 6 ++-- .../app/views/docs/pages/client_validation.rb | 2 +- lib/forms/form.rb | 6 ++-- lib/forms/validations/introspector.rb | 14 ++++++---- spec/forms/validations/introspector_spec.rb | 28 +++++++++---------- 9 files changed, 62 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d0490c..65c6299 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,12 +17,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 derives the checked set from the model's current value, and renders under both themes. `variant:` (`:stack`/`:inline`/`:pill`) is layout-only, no JS. +### Changed + +- **Client-side validation Stimulus identifiers dropped the `forms--` prefix**: + the bundled controllers now emit `validations--presence`, `validations--length`, + … (and the `validations--form` coordinator) so + `lazyLoadControllersFrom("phlex_forms/controllers")` resolves them to their + shipped path `phlex_forms/controllers/validations/*_controller` — previously + `forms--validations--*` derived `.../forms/validations/*`, which 404'd and the + controllers never connected (issue #12). The `data-validations--*` binding + attributes and the `invalidate:validations` event changed to match. Hosts that + registered `forms--validations--*` explicitly must update the identifier. + ### Fixed +- **`f.Radio` / `Field#radio` rendered the model's current value on every radio + instead of each radio's own value**: `field_attributes` carried `value: + field_value` and was splatted after the explicit positional value, clobbering + it — a new record lost the value entirely, an edit form gave every radio the + same value. `radio` now drops `field_attributes`' `value` (issue #13). - **`Form(validate: true)` never fired client-side validation on submit**: the coordinator controller was attached but no `data-action` wired its `onSubmit` handler, so submitting an invalid form was not blocked. `apply_validation_coordinator` - now emits `submit->forms--validations--form#onSubmit` (joined with any + now emits `submit->validations--form#onSubmit` (joined with any caller-supplied `data-action`). - **`fields_for` iterated a Hash-backed association (JSONB), emitting bogus indices**: a Hash responds to `#each_with_index`, so a JSONB column rendered diff --git a/README.md b/README.md index f7e10cc..47ad374 100644 --- a/README.md +++ b/README.md @@ -337,6 +337,11 @@ import { lazyLoadControllersFrom } from "@hotwired/stimulus-loading" lazyLoadControllersFrom("phlex_forms/controllers", application) ``` +The emitted identifiers are `validations--presence`, `validations--length`, … (and +the form-level `validations--form` coordinator), which `lazyLoadControllersFrom` +resolves to `phlex_forms/controllers/validations/*_controller` — the path the gem +ships them at. + Messages ship for `en` / `fr` / `af`; override via `window.PhlexForms.messages`. ## Nested attributes, collections & escape valves diff --git a/app/javascript/phlex_forms/controllers/validations/base_controller.js b/app/javascript/phlex_forms/controllers/validations/base_controller.js index ed85a56..72e83ee 100644 --- a/app/javascript/phlex_forms/controllers/validations/base_controller.js +++ b/app/javascript/phlex_forms/controllers/validations/base_controller.js @@ -10,7 +10,7 @@ import { Controller } from "@hotwired/stimulus" // reads from data attributes; the base class only knows about the // `allowBlank` / `allowNil` short-circuits. export class FieldValidatorController extends Controller { - // `error` is opt-in: callers that pre-render a `

` + // `error` is opt-in: callers that pre-render a `

-target="error">` // get a stable slot the controller toggles. Inputs without an // explicit target still work — the controller lazily creates one // adjacent to the input below. @@ -24,12 +24,12 @@ export class FieldValidatorController extends Controller { // directly to ,