Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make possible to use the Wrappers API to define attributes for the components #997

Merged
merged 35 commits into from
Mar 17, 2014

Conversation

rafaelfranca
Copy link
Collaborator

(This is a alternative solution to #622)

Motivation

To integrate Simple Form with Bootstrap 3 we need to make possible to define classes to input only in some wrappers.

We added input_class some time ago but that broke some components like checkboxes (see heartcombo/simple_form-bootstrap#28).

Instead of changing a global class to be applicated in every input we allow users to choose in which wrappers they want to define a class.

How to use

Now users can define attributes for the components in the component definition of the Wrappers API.

config.wrappers tag: 'div', class: 'control-group', error_class: 'error' do |b|
    b.use :label, class: 'form-label'
    b.use :input, class: 'form-input'
end

The class: 'form-label' option will be used every time a label component is rendered.

Implementation

To make this possible I had to change who the Wrappers API is defined and how components are rendered.

Wrapper API storage change

Before the work on this branch, the API components were stored in a mix of Simple Form classes and symbols.

simple_form_wrapper_before

This added some conditionals in our rendering code and made harder to store state in our components. This create a lot of code in Simple Form that rely in the fact that Ruby hashes are mutables and we have a global state. The implementation in #622 had this problem were the changes in a global hash make the hint and errors classes to getting duplicated.

I choose to avoid completely this problem storing the attributes in a class and making it implement the same protocol as SimpleForm::Wrappers::Many and SimpleForm::Wrapper::Single. This also make possible to remove some conditionals to deal with Symbols in the components tree.

simple_form_wrapper_after

Input rendering

Before this work, when rendering the input, there was no way to pass information about the context where it is being rendered. I changed the implementation to always pass the wrapper options for the component rendering method itself.

To understand better lets look how a custom input was implemented before:

class CustomInput < SimpleForm::Inputs::Base
  def input
    @builder.text_field(attribute_name, input_html_options)
  end
end

So when rendering a wrapper Simple Form will call the method input of that class. As you can see we don't have a clean way to get information about the wrapper that is being rendered inside that method.

Now, Simple Form will pass an argument wrapper_options that hold the options defined in the Wrappers API. So, for our previous example, wrapper_options will contain class: 'form-input'.

So, our custom input will look like:

class CustomInput < SimpleForm::Inputs::Base
  def input(wrapper_options)
    @builder.text_field(attribute_name, input_html_options)
  end
end

Now we can use a new helper merge_wrapper_options to merge the options present in the form.input call and the options defined in the wrapper.

Finally, our custom input will look like:

class CustomInput < SimpleForm::Inputs::Base
  def input(wrapper_options)
    @builder.text_field(attribute_name, merge_wrapper_options(input_html_options, wrapper_options))
  end
end

Deprecations

The custom input/custom components methods now have to accept one argument with the wrapper options. The inputs/components defined using the old API will work but with a deprecation message.

These examples are deprecated:

class CustomInput < SimpleForm::Inputs::Base
  def input
  end

  def label
  end

  def hint
  end

  def label_input
  end

  def error
  end

  def placeholder
  end
end

To fix the deprecation message use:

class CustomInput < SimpleForm::Inputs::Base
  def input(wrapper_options)
  end

  def label(wrapper_options)
  end

  def hint(wrapper_options)
  end

  def label_input(wrapper_options)
  end

  def error(wrapper_options)
  end

  def placeholder(wrapper_options)
  end
end

Next steps

  • Add proper documentation
  • Test this new feature writing the wrappers for Bootstrap 3

Closes #622

@@ -1,7 +1,7 @@
module SimpleForm
module Components
module Errors
def error
def error(context=nil)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This argument=values was the guideline in Simple Form for a while but I prefer to change it now since it is not consistent anymore

Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like spaces here too

@josevalim
Copy link
Contributor

❤️ 💚 💙 💛 💜

@@ -0,0 +1,28 @@
module SimpleForm
module Wrappers
class Leaf
Copy link
Contributor

Choose a reason for hiding this comment

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

It took me a while to figure out that the context object that will be passed to the components was an instance of this class - dunno if matching names or something like that could make this more straightforward.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it would be best to simply pass the options?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought that too. I was more thinking about the future if we need to pass more things, but I think we will not need it in the near feature.

@josevalim
Copy link
Contributor

There are some docs here that need to be updated: https://github.com/plataformatec/simple_form/blob/rm-wrapper-classes/lib/simple_form/wrappers/many.rb#L3-L4

I saw them when checking if we could rename Many to Node but I think Many is a better name?

module WrapperOptions
private

def merge_wrapper_options(options, context)
Copy link
Contributor

Choose a reason for hiding this comment

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

dunno if we need a module just for this method. Pushing it to SimpleForm::Inputs::Base is an option?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is, but I don't see problem to extract to a module.

I'm removing because helpers are related to the HTML attributes and this one is not related to any attribute at all.

Helpers are related to HTML attributes. merge_wrapper_options is not
related to HTML
Right now we only care about the options so lets make the code simpler
@rafaelfranca
Copy link
Collaborator Author

@josevalim Simple and Many are both Nodes, so I think we can't change that. Actually the name Leaf is what I'd choose to change, but I could not came with another name.

ened added a commit to ened/bootstrap-colorpicker-rails that referenced this pull request Jan 9, 2015
DEPRECATION WARNING: input method now accepts a `wrapper_options` argument. The method definition without the argument is deprecated and will be removed in the next Simple Form version. Change your code from:

    def input

to

    def input(wrapper_options)

See heartcombo/simple_form#997 for more information.
panmari added a commit to panmari/rails3-jquery-autocomplete that referenced this pull request Jan 23, 2015
JasonBarnabe added a commit to JasonBarnabe/simple-form-datepicker that referenced this pull request Jan 28, 2015
thiagoa added a commit to thiagoa/rails4-autocomplete that referenced this pull request Feb 8, 2015
c-lliope added a commit to sfbrigade/ohana-api that referenced this pull request Feb 21, 2015
Since heartcombo/simple_form#997,
custom input adapters must accept a hash of wrapper options
c-lliope added a commit to sfbrigade/ohana-api that referenced this pull request Feb 21, 2015
Since heartcombo/simple_form#997,
custom input adapters must accept a hash of wrapper options
c-lliope added a commit to sfbrigade/ohana-api that referenced this pull request Feb 21, 2015
Since heartcombo/simple_form#997,
custom input adapters must accept a hash of wrapper options
ewr added a commit to SCPR/outpost that referenced this pull request Apr 30, 2015
whatcould added a commit to whatcould/rails-jquery-autocomplete that referenced this pull request May 11, 2015
eturino pushed a commit to artirix/browsercms that referenced this pull request Jun 22, 2015
eturino pushed a commit to artirix/browsercms that referenced this pull request Jun 22, 2015
mitio added a commit to skanev/evans that referenced this pull request Aug 28, 2015
DEPRECATION WARNING: input method now accepts a `wrapper_options` argument. The method definition without the argument is deprecated and will be removed in the next Simple Form version. Change your code from:

    def input

to

    def input(wrapper_options)

See heartcombo/simple_form#997 for more information.
 (called from block in _app_views_tips_new_html_haml__844955797404246493_70326972009780 at /Users/dimitar/projects/fmi/evans/app/views/tips/new.html.haml:6)
@jmotis jmotis mentioned this pull request Mar 24, 2016
jhanggi added a commit to wyeworks/nucore-open that referenced this pull request May 24, 2016
These methods now accept a `wrappers_options` argument.

heartcombo/simple_form#997
drench pushed a commit to wyeworks/nucore-open that referenced this pull request May 25, 2016
drench pushed a commit to wyeworks/nucore-open that referenced this pull request Jun 1, 2016
@AndreLZGava
Copy link

Hello, can anyone help me I'm still learning rails and was trying to create a input to accept datepicker-bootstrap, I found this tutorial https://github.com/plataformatec/simple_form/wiki/Custom-inputs-examples, I createad as it was declared and worked for a form but when I load another don't, in my terminal I found the reason, that must be this issue and this deprecated, can you help me about this broblem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

9 participants