Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/brown-wasps-exercise.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/view-components": patch
---

Fix generated field ids to remove brackets
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changeset description says "Fix generated field ids to remove brackets" but this is incorrect. The fix actually preserves brackets in generated field IDs, not removes them. The PR description and implementation both confirm that brackets are being preserved (e.g., permissions[3]_foo instead of permissions_3_foo).

Copilot uses AI. Check for mistakes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

28 changes: 28 additions & 0 deletions app/lib/primer/forms/check_box.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ def initialize(input:)
@input.add_label_classes("FormControl-label")
@input.add_input_classes("FormControl-checkbox")

# Generate custom ID that preserves brackets from the name
unless @input.input_arguments[:id].present?
generate_custom_id
# Update the label's for attribute to match the new ID
@input.label_arguments[:for] = @input.input_arguments[:id]
end

return unless @input.scheme == :array

@input.input_arguments[:multiple] = true
Expand All @@ -32,6 +39,27 @@ def nested_form_arguments

private

def generate_custom_id
# Generate an ID from the name that preserves special characters like brackets
# For array scheme: name + "_" + value (e.g., "permissions[3]_foo")
# For boolean scheme: just the name (e.g., "long_o")
base_name = @input.name.to_s

# For array scheme, Rails appends [] to the name, so we remove it for ID generation
# but only the trailing [] that Rails adds, not brackets that are part of the original name
# Regex /\[\]$/ matches literal "[]" at the end of the string
base_name = base_name.sub(/\[\]$/, "")

# For array scheme, append the value to make IDs unique
# For boolean scheme, just use the base name
# Note: Rails automatically escapes HTML attributes, so special characters are safe
if @input.scheme == :array && @input.value.present?
@input.input_arguments[:id] = "#{base_name}_#{@input.value}"
else
@input.input_arguments[:id] = base_name
end
end

def checked_value
@input.value || "1"
end
Expand Down
36 changes: 36 additions & 0 deletions test/lib/primer/forms/checkbox_input_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,40 @@ def test_nested_form_can_be_hidden_independently
assert_selector "input[name=foo]"
assert_selector "input[name=bar]", visible: :hidden
end

def test_preserves_brackets_in_generated_ids
render_in_view_context do
primer_form_with(url: "/foo") do |f|
render_inline_form(f) do |check_form|
check_form.check_box(
name: "permissions[3]",
label: "Foo",
value: "bar",
scheme: :array
)
end
end
end

# The ID should preserve brackets from the name
assert_selector "input[id='permissions[3]_bar']"
assert_selector "label[for='permissions[3]_bar']"
end

def test_preserves_brackets_in_generated_ids_with_boolean_scheme
render_in_view_context do
primer_form_with(url: "/foo") do |f|
render_inline_form(f) do |check_form|
check_form.check_box(
name: "user[settings][email]",
label: "Email notifications"
)
end
end
end

# For boolean scheme, the ID should just be the name with brackets preserved
assert_selector "input[id='user[settings][email]']"
assert_selector "label[for='user[settings][email]']"
end
Comment on lines +84 to +99
Copy link

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a test case to verify that when an explicit ID is provided via the id parameter, it is not overridden by the custom ID generation logic. This would ensure the guard condition on line 15 works correctly and that users can still provide custom IDs when needed.

Copilot uses AI. Check for mistakes.
end
Loading