Skip to content

Commit

Permalink
add required option on form_group_builder (#488)
Browse files Browse the repository at this point in the history
* add required option on form_group_builder

* add doc, tests and changelog entry

* deprecate :skip_required in favor to :required

* Fix RuboCop offences and message.

* Fix typo.

* Fix a few more RuboCop offences.

* Fix the offence without breaking the tests.
  • Loading branch information
ThomasSevestre authored and lcreid committed Nov 7, 2018
1 parent 508b278 commit c953bc6
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ In addition to these necessary markup changes, the bootstrap_form API itself has
* [#476] Give a way to pass classes to the `div.form-check` wrapper for check boxes and radio buttons - [@lcreid](https://github.com/lcreid).
* [461](https://github.com/bootstrap-ruby/bootstrap_form/pull/461): default form-inline class applied to parent content div on date select helpers. Can override with a :skip_inline option on the field helper - [@lancecarlson](https://github.com/lancecarlson).
* The `button`, `submit`, and `primary` helpers can now receive an additional option, `extra_class`. This option allows us to specify additional CSS classes to be added to the corresponding button/input, _while_ maintaining the original default ones. E.g., a primary button with an `extra_class` 'test-button' will have its final CSS classes declaration as 'btn btn-primary test-button'.
* [#488](https://github.com/bootstrap-ruby/bootstrap_form/pull/488): add required option on form_group_builder - [@ThomasSevestre](https://github.com/ThomasSevestre).

### Bugfixes

Expand Down
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,11 @@ validator with the associated model attribute. Presently this is one of:
ActiveRecord::Validations::PresenceValidator or
ActiveModel::Validations::PresenceValidator.

In cases where this behavior is undesirable, use the `skip_required` option:
In cases where this behavior is undesirable, use the `required` option to force the class to be present or absent:

```erb
<%= f.password_field :password, label: "New Password", skip_required: true %>
<%= f.password_field :login, label: "New Username", required: true %>
<%= f.password_field :password, label: "New Password", required: false %>
```

### Input Elements / Controls
Expand Down
12 changes: 10 additions & 2 deletions lib/bootstrap_form/form_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -431,10 +431,15 @@ def form_group_builder(method, options, html_options=nil)

label_text ||= options.delete(:label) if options[:label].is_a?(String)

if options.key?(:skip_required)
warn "`:skip_required` is deprecated, use `:required: false` instead"
options[:required] = options.delete(:skip_required) ? false : :default
end

form_group_options[:label] = {
text: label_text,
class: label_class,
skip_required: options.delete(:skip_required)
required: options.delete(:required)
}.merge(css_options[:id].present? ? { for: css_options[:id] } : {})

css_options[:placeholder] = label_text || object.class.human_attribute_name(method) if options.delete(:label_as_placeholder)
Expand Down Expand Up @@ -468,7 +473,10 @@ def generate_label(id, name, options, custom_label_col, group_layout)
classes << "mr-sm-2"
end

unless options.delete(:skip_required)
case options.delete(:required)
when true
classes << "required"
when nil, :default
classes << "required" if required_attribute?(object, name)
end

Expand Down
22 changes: 21 additions & 1 deletion test/bootstrap_form_group_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class BootstrapFormGroupTest < ActionView::TestCase
assert_equivalent_xml expected, @builder.text_field(:email, skip_label: true)
end

test "preventing a label from having the required class" do
test "preventing a label from having the required class with :skip_required" do
expected = <<-HTML.strip_heredoc
<div class="form-group">
<label for="user_email">Email</label>
Expand All @@ -84,6 +84,26 @@ class BootstrapFormGroupTest < ActionView::TestCase
assert_equivalent_xml expected, @builder.text_field(:email, skip_required: true)
end

test "preventing a label from having the required class" do
expected = <<-HTML.strip_heredoc
<div class="form-group">
<label for="user_email">Email</label>
<input class="form-control" id="user_email" name="user[email]" type="text" value="steve@example.com" />
</div>
HTML
assert_equivalent_xml expected, @builder.text_field(:email, required: false)
end

test "forcing a label to have the required class" do
expected = <<-HTML.strip_heredoc
<div class="form-group">
<label class="required" for="user_comments">Comments</label>
<input class="form-control" id="user_comments" name="user[comments]" type="text" value="my comment" />
</div>
HTML
assert_equivalent_xml expected, @builder.text_field(:comments, required: true)
end

test "label as placeholder" do
expected = <<-HTML.strip_heredoc
<div class="form-group">
Expand Down

0 comments on commit c953bc6

Please sign in to comment.