Skip to content

Commit

Permalink
Make boolean input tag generate valid html5 when using nested style
Browse files Browse the repository at this point in the history
This keeps Rails functionality by adding the required hidden checkbox
which will send an "unchecked value" when checkbox is not checked.
  • Loading branch information
carlosantoniodasilva committed Jan 30, 2012
1 parent c53398a commit fb0360d
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 10 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Expand Up @@ -28,7 +28,8 @@
to generate custom label and input structure. It is used internally with the :nested
option for `:boolean_style`, and is useful to allow some more customization if required.
* Do not generate hidden check box field when using nested boolean style, as it is considered
invalid markup in HTML5. This will only work in Rails > 3.2.1 (not released at this time).
invalid markup in HTML5. This will work by default in Rails > 3.2.1 (not released at this time),
and is backported inside SimpleForm builder extensions.
More info in [#215](https://github.com/plataformatec/simple_form/issues/215)
* Add `item_wrapper_class` configuration option for collection radio buttons / check boxes inputs.

Expand Down
Expand Up @@ -84,7 +84,6 @@ SimpleForm.setup do |config|
# to learn about the different styles for forms and inputs,
# buttons and other elements.
config.default_wrapper = :bootstrap

<% else %>
# The default wrapper to be used by the FormBuilder.
config.default_wrapper = :default
Expand Down
25 changes: 21 additions & 4 deletions lib/simple_form/inputs/boolean_input.rb
Expand Up @@ -3,7 +3,8 @@ module Inputs
class BooleanInput < Base
def input
if nested_boolean_style?
template.label_tag(nil, :class => "checkbox") { build_check_box(nil) }
build_hidden_field_for_checkbox +
template.label_tag(nil, :class => "checkbox") { build_check_box_without_hidden_field }
else
build_check_box
end
Expand All @@ -13,7 +14,8 @@ def label_input
if options[:label] == false
input
elsif nested_boolean_style?
@builder.label(label_target, label_html_options) { build_check_box(nil) + label_text }
build_hidden_field_for_checkbox +
@builder.label(label_target, label_html_options) { build_check_box_without_hidden_field + label_text }
else
input + label
end
Expand All @@ -22,12 +24,27 @@ def label_input
private

# Build a checkbox tag using default unchecked value. This allows us to
# reuse the method for nested boolean style, but with nil unchecked value,
# which won't generate the hidden checkbox (only in Rails > 3.2.1).
# reuse the method for nested boolean style, but with no unchecked value,
# which won't generate the hidden checkbox. This is the default functionality
# in Rails > 3.2.1, and is backported in SimpleForm AV helpers.
def build_check_box(unchecked_value='0')
@builder.check_box(attribute_name, input_html_options, '1', unchecked_value)
end

# Build a checkbox without generating the hidden field. See
# #build_hidden_field_for_checkbox for more info.
def build_check_box_without_hidden_field
build_check_box(nil)
end

# Create a hidden field for the current checkbox, so we can simulate Rails
# functionality with hidden + checkbox, but under a nested context, where
# we need the hidden field to be *outside* the label (otherwise it
# generates invalid html - html5 only).
def build_hidden_field_for_checkbox
@builder.hidden_field(attribute_name, :value => '0', :id => nil)
end

# Booleans are not required by default because in most of the cases
# it makes no sense marking them as required. The only exception is
# Terms of Use usually presented at most sites sign up screen.
Expand Down
17 changes: 13 additions & 4 deletions test/inputs/boolean_input_test.rb
Expand Up @@ -20,20 +20,29 @@ class BooleanInputTest < ActionView::TestCase
assert_no_select 'label > input'
end

test 'input allows changing default boolean style config to nested, generating a default label' do
test 'input allows changing default boolean style config to nested, generating a default label and a manual hidden field for checkbox' do
swap SimpleForm, :boolean_style => :nested do
with_input_for @user, :active, :boolean
assert_select 'label[for=user_active]', 'Active'
assert_select 'label.boolean > input.boolean'
assert_no_select 'input + label'
assert_no_select 'input[type=checkbox] + label'
end
end

test 'input boolean generates a manual hidden field for checkbox outside the label, to recreate Rails functionality with valid html5' do
swap SimpleForm, :boolean_style => :nested do
with_input_for @user, :active, :boolean

assert_select "input[type=hidden][name='user[active]'] + label.boolean > input.boolean"
assert_no_select 'input[type=checkbox] + label'
end
end

test 'input accepts changing boolean style to nested through given options' do
with_input_for @user, :active, :boolean, :boolean_style => :nested
assert_select 'label[for=user_active]', 'Active'
assert_select 'label.boolean > input.boolean'
assert_no_select 'input + label'
assert_no_select 'input[type=checkbox] + label'
end

test 'input accepts changing boolean style to inline through given options, when default is nested' do
Expand Down Expand Up @@ -67,7 +76,7 @@ class BooleanInputTest < ActionView::TestCase
swap SimpleForm, :boolean_style => :nested do
with_input_for @user, :active, :boolean

assert_select 'label.boolean + label.checkbox > input.boolean'
assert_select 'label.boolean + input[type=hidden] + label.checkbox > input.boolean'
end
end
end
Expand Down

3 comments on commit fb0360d

@dlee
Copy link
Contributor

@dlee dlee commented on fb0360d May 10, 2012

Choose a reason for hiding this comment

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

@carlosantoniodasilva How did you figure out that having the hidden input right before the checkbox generated invalid HTML? Unfortunately, this change is breaking bootstrap's rule for aligning checkboxes: https://github.com/twitter/bootstrap/blob/v2.0.3/less/forms.less#L185-189

@rafaelfranca
Copy link
Collaborator

Choose a reason for hiding this comment

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

<!DOCTYPE html>
<head>
<title>Test</title>
</head>
<body>
<label>
  <input type="hidden" />
  <input type="checkbox" />
</label>
</body>
</html>

Error:

Line 8, Column 27: The label element may contain at most one input, button, select, textarea, or keygen descendant.

@carlosantoniodasilva
Copy link
Member Author

Choose a reason for hiding this comment

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

@dlee yeah, we're aware of the problem with bootstrap, but it's preferable to have valid markup and sort out some other way of fixing it through css there I think.

@rafaelfranca thanks.

Please sign in to comment.