-
Notifications
You must be signed in to change notification settings - Fork 355
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Allow to configure default form attributes #562
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,8 @@ Gem::Specification.new do |s| | |
"easy to create beautiful-looking forms using Bootstrap 4" | ||
s.license = "MIT" | ||
|
||
s.post_install_message = "Default form attribute role=\"form\" will be dropped in 5.0.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
s.files = `git ls-files -z`.split("\x0").reject do |f| | ||
f.match(%r{^(test)/}) | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
# frozen_string_literal: true | ||
|
||
module BootstrapForm | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. |
||
class Configuration | ||
def default_form_attributes=(attributes) | ||
case attributes | ||
when nil | ||
@default_form_attributes = {} | ||
when Hash | ||
@default_form_attributes = attributes | ||
else | ||
raise ArgumentError, "Unsupported default_form_attributes #{attributes.inspect}" | ||
end | ||
end | ||
|
||
def default_form_attributes | ||
return @default_form_attributes if defined? @default_form_attributes | ||
|
||
# TODO: Return blank hash ({}) in 5.0.0. Role "form" for form tags is redundant and makes W3C to raise a warning. | ||
{ role: "form" } | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,14 +58,14 @@ def initialize(object_name, object, template, options) | |
options[:inline_errors] != false | ||
end | ||
@acts_like_form_tag = options[:acts_like_form_tag] | ||
add_form_role_and_form_inline options | ||
add_default_form_attributes_and_form_inline options | ||
super | ||
end | ||
# rubocop:enable Metrics/AbcSize | ||
|
||
def add_form_role_and_form_inline(options) | ||
def add_default_form_attributes_and_form_inline(options) | ||
options[:html] ||= {} | ||
options[:html][:role] ||= "form" | ||
options[:html].reverse_merge!(BootstrapForm.config.default_form_attributes) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be nice to make this work reasonably if the user sets |
||
|
||
return unless options[:layout] == :inline | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
require_relative "./test_helper" | ||
|
||
class BootstrapConfigurationTest < ActionView::TestCase | ||
test "has default form attributes" do | ||
config = BootstrapForm::Configuration.new | ||
|
||
assert_equal({ role: "form" }, config.default_form_attributes) | ||
end | ||
|
||
test "allows to set default_form_attributes with custom value" do | ||
config = BootstrapForm::Configuration.new | ||
config.default_form_attributes = { foo: "bar" } | ||
|
||
assert_equal({ foo: "bar" }, config.default_form_attributes) | ||
end | ||
|
||
test "allows to set default_form_attributes with nil" do | ||
config = BootstrapForm::Configuration.new | ||
config.default_form_attributes = nil | ||
|
||
assert_equal({ }, config.default_form_attributes) | ||
end | ||
|
||
test "does not allow to set default_form_attributes with unsupported value" do | ||
config = BootstrapForm::Configuration.new | ||
|
||
exception = assert_raises ArgumentError do | ||
config.default_form_attributes = [1,2,3] | ||
end | ||
assert_equal('Unsupported default_form_attributes [1, 2, 3]', exception.message) | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to give a little more information to the reader around configuration. I suggest something like a first paragraph that says:
The actual example will depend on the path we choose for maintaining compatibility for version 4, and enabling the new default for version 5 (which will be the Bootstrap v5 version). See my comments on the pull request.