Skip to content

Expand l_ui_form field types and add multipart override - #86

Merged
thomasgallagher merged 3 commits into
mainfrom
expand-l-ui-form-field-types
May 19, 2026
Merged

Expand l_ui_form field types and add multipart override#86
thomasgallagher merged 3 commits into
mainfrom
expand-l-ui-form-field-types

Conversation

@thomasgallagher

Copy link
Copy Markdown
Collaborator

Add tel, url, search, time, month, week, color, range, and file to the supported field types so l_ui_form covers the HTML5 input types modern browsers expect. Map :tel to telephone_field since Rails has no tel_field.

Add a multipart: option that overrides the auto-detection (true when any field is :file). Document the override-the-partial escape hatch in the skill reference and dummy app docs.

Add tel, url, search, time, month, week, color, range, and file to the
supported field types so l_ui_form covers the HTML5 input types modern
browsers expect. Map :tel to telephone_field since Rails has no tel_field.

Add a multipart: option that overrides the auto-detection (true when any
field is :file). Document the override-the-partial escape hatch in the
skill reference and dummy app docs.
@claude

claude Bot commented May 19, 2026

Copy link
Copy Markdown

Review

Adds HTML5 input types (, , , , , , , , ) to , maps to Rails' , and introduces a override with auto-detection when any field is .

Issues

Severity File Line(s) Description
MEDIUM 1 local_assigns[:multipart] = nil is incorrect. local_assigns is frozen in Rails 7+; this line raises FrozenError if executed. Crucially, even if it were mutable, writing to the hash does not define the local variable multipart in the template binding - so line 4 would raise NameError: undefined local variable or method 'multipart' regardless. In practice this never fires because l_ui_form always passes multipart: explicitly, but it is a latent bug for any direct partial render without that local. The established pattern in this codebase (see _search_select.html.erb, _search_field.html.erb) is <% multipart = local_assigns.fetch(:multipart, nil) %>.

Verdict

🟧 Ship (with known minor issues) - The feature is correct and the bug is latent (unreachable via the public helper), but should be fixed before the guard logic gives a future maintainer a misleading safety net.

@claude

claude Bot commented May 19, 2026

Copy link
Copy Markdown

Review: Expand l_ui_form field types and add multipart override

Adds nine new HTML5 input types to l_ui_form (:tel, :url, :search, :time, :month, :week, :color, :range, :file), a multipart: option with auto-detection when any field is :file, and documents an escape hatch for overriding the form partial.

Issues

Severity File Line(s) Description
MEDIUM test/helpers/form_helper_test.rb No tests added for any of the nine new field types. The :teltelephone_field mapping is the non-obvious custom case most likely to break silently; similarly the multipart auto-detection logic has no coverage. Existing tests cover the old types; new ones are entirely untested.
MEDIUM test/dummy/app/views/pages/forms_helper.html.erb 44–50 CLAUDE.md requires a live example in the dummy app for new helpers/fields. The paragraph lists all nine new types, but the live form only demonstrates :string, :text, and :select — unchanged from before. Readers cannot see what the new inputs look like rendered.

Verdict

🚫 Changes required - The implementation is correct, but the project's own conventions require live dummy-app examples and test coverage for the new field types.

Add tests for all new field types (password, tel, url, search, time,
month, week, color, range, file) including the :tel → telephone_field
mapping, plus multipart auto-detection coverage. Expand the dummy app's
"All field types" example to render every supported type.
@claude

claude Bot commented May 19, 2026

Copy link
Copy Markdown

Review

Adds , , , , , , , , and to the supported l_ui_form field types, plus a multipart: option with auto-detection when any field is :file.

No issues found. The whitelist validation in l_ui_normalise_field keeps public_send safe, the local_assigns.fetch(:multipart, nil) guard correctly handles partial overrides without the local, and the auto-detection check against raw field configs is consistent with :file never being returned by column type inference.


Ship

@thomasgallagher
thomasgallagher merged commit 7208697 into main May 19, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant