Skip to content

Commit

Permalink
Fix DOM ids and labels of ingredient editors
Browse files Browse the repository at this point in the history
The boolean ingredient editor label tag must
not have a `for` attribute since it nests the input
tag. That input does not need an `id` as it is
nested. Solves an issue with non-clickable
label on the checkbox.

The image editor label must not have a `for`
attribute as well, as there is no focusable form
element.
  • Loading branch information
tvdeyen committed Jul 31, 2023
1 parent 4dd42b2 commit a37545b
Show file tree
Hide file tree
Showing 6 changed files with 21 additions and 5 deletions.
4 changes: 2 additions & 2 deletions app/helpers/alchemy/admin/ingredients_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ def render_ingredient_role(ingredient)
end

# Renders the label and hint for a ingredient.
def ingredient_label(ingredient, column = :value)
label_tag ingredient.form_field_id(column) do
def ingredient_label(ingredient, column = :value, html_options = {})
label_tag ingredient.form_field_id(column), html_options do
[render_ingredient_role(ingredient), render_hint_for(ingredient)].compact.join(" ").html_safe
end
end
Expand Down
4 changes: 2 additions & 2 deletions app/views/alchemy/ingredients/_boolean_editor.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
class: boolean_editor.css_classes,
data: boolean_editor.data_attributes do %>
<%= element_form.fields_for(:ingredients, boolean_editor.ingredient) do |f| %>
<%= f.label :value, style: "display: inline-block" do %>
<%= f.check_box :value, id: boolean_editor.form_field_id %>
<%= f.label :value, style: "display: inline-block", for: nil do %>
<%= f.check_box :value, id: nil %>
<%= render_ingredient_role(boolean_editor) %>
<% end %>
<%= render_hint_for(boolean_editor) %>
Expand Down
2 changes: 1 addition & 1 deletion app/views/alchemy/ingredients/_picture_editor.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
class: picture_editor.css_classes,
data: picture_editor.data_attributes do %>
<%= element_form.fields_for(:ingredients, picture_editor.ingredient) do |f| %>
<%= ingredient_label(picture_editor, :picture_id) %>
<%= ingredient_label(picture_editor, :picture_id, for: nil) %>
<%= content_tag :div,
data: {
target_size: picture_editor.settings[:size] || [
Expand Down
6 changes: 6 additions & 0 deletions spec/helpers/alchemy/admin/ingredients_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@
is_expected.to have_selector("label > .hint-with-icon", text: "This is a hint")
end
end

context "with html_options given" do
it "adds them to the label tag" do
expect(helper.ingredient_label(ingredient_editor, :value, for: "foo")).to have_selector('label[for="foo"]')
end
end
end

describe "#render_ingredient_role" do
Expand Down
5 changes: 5 additions & 0 deletions spec/views/alchemy/ingredients/boolean_editor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,9 @@
end
end
end

it "does not add a for attribute to the label tag" do
is_expected.to have_selector("label", text: "Boolean")
is_expected.to_not have_selector("label[for]", text: "Boolean")
end
end
5 changes: 5 additions & 0 deletions spec/views/alchemy/ingredients/picture_editor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,9 @@
is_expected.to have_selector("a.disabled .icon.fa-crop")
end
end

it "does not add a for attribute to the label tag" do
is_expected.to have_selector("label", text: "Image")
is_expected.to_not have_selector("label[for]", text: "Image")
end
end

0 comments on commit a37545b

Please sign in to comment.