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

Add 'append and prepend' wrapper #531

Closed
wants to merge 1 commit into from
Closed

Add 'append and prepend' wrapper #531

wants to merge 1 commit into from

Conversation

mfo
Copy link

@mfo mfo commented Apr 5, 2012

Hi ;

at sharypic we needed the 'append and prepend' style of the bootstrap (see : http://twitter.github.com/bootstrap/base-css.html#forms or a small screen shot http://cl.ly/1B451f1v3x311k193u1i)

So i updated your generator, hope it helps.
Best, Martin.

PS: if something is messing let me know ;-)

@carlosantoniodasilva
Copy link
Member

@mfo sounds great, thanks! I'm just wondering if we should stick with append_and_prepend, prepend_and_append, prepend_append, or append_prepend.. Prepend first seems more logical to me, dunno :D. @rafaelfranca thoughts?

@josevalim
Copy link
Contributor

Since those are all the same, I think instead we should make the internal div a named parameter and get rid of those different wrappers:

input.wrapper :input_wrapper, :tag => 'div', :class => 'input-prepend input-append' do

And then in the view:

f.input :foo, input_wrapper_html: { class: "input-append" }

Another option is to have a wrapper_input component. But I think that having 4 components exactly the same is a bit not-dry. :)

(Alternatively we could wrap those in a lambda to dry up the config file)

@rafaelfranca
Copy link
Collaborator

I like the José idea. Can we try it?

@mfo
Copy link
Author

mfo commented Apr 5, 2012

@carlosantoniodasilva : right :-D, but jose submitted a great idea :-)
@josevalim / @rafaelfranca : i like the the idea of giving the className. Have to check in depth how simple_form works (to stay consistent with your conventions). So feel free to push a new idea/pull-request, can't wait to use it as a "native" stuff instead of a "hack".

Thx for your feedbacks guys

@zeppelin
Copy link

zeppelin commented May 8, 2012

I came up with the following quick solution. (the classname is not configurable, as I don't need really need it to be)

module AroundInput
  include ActionView::Helpers::TagHelper

  # Example:
  #
  # class StringInput < SimpleForm::Inputs::StringInput
  #   include AroundInput
  # end
  #
  # ...
  #
  # <%= f.input :twitter, prepend: '@' %>
  # 
  def input
    prepend_html = if input_options[:prepend]
      html_classes << 'input-prepend'
      content_tag :span, input_options[:prepend], class: 'add-on'
    end

    append_html = if input_options[:append]
      html_classes << 'input-append'
      content_tag :span, input_options[:append], class: 'add-on'
    end

    "#{prepend_html}#{super}#{append_html}".html_safe
  end
end

I'd love to learn how could I improve this :)

@dlee
Copy link
Contributor

dlee commented May 10, 2012

We should just allow all inputs to accept :prepend and :append that allows you inject HTML before and after the input. This is useful not only for the .addon, but for checkboxes with clickable text on the right. For examples, search for "Check me out" and "Remember me" in http://twitter.github.com/bootstrap/base-css.html#forms.

We can't use :hint for this, because we need the text to be placed right next to the checkbox so that it's still within the label.

Providing a general :prepend and :append option seems like a good way to solve this and other issues.

@dlee
Copy link
Contributor

dlee commented May 10, 2012

Actually, I found that there's an undocumented :inline_label option for checkboxes that appends text to the right of the checkbox.

@zeppelin
Copy link

@dlee Ah, you just saved me (possibly) hours of work with that :inline_label option! Working exactly on this atm. :)

Thanks! ;)

@josevalim
Copy link
Contributor

A :prepend/:append option seems useful. @carlosantoniodasilva, @rafaelfranca wdyt?

@zeppelin
Copy link

@josevalim @dlee is it reasonable to add both :prepend and :prepend_html option? it'd be nice to have to append just the content, when I have no interest in customizing the surrounding HTML.

(:append, :append_html as well, of course)

@josevalim
Copy link
Contributor

@zeppelin it seems reasonable. I also think you can easily do that today by passing a block to simple form:

<%= f.input :sample do |i| %>
  <%= i.input_field :sample %>
<% end %>

But it has the downside of forcing you to pass twice the field twice.

PS: not sure if it is called input_field the second method. :P

@dlee documentation for that option would be welcome :)

@dlee
Copy link
Contributor

dlee commented May 10, 2012

@zeppelin Good to hear. I just created an Issue to add docs about :inline_label to the README: #568

@dlee
Copy link
Contributor

dlee commented May 10, 2012

Wow, quick responses.

@josevalim I saw your comment after I posted the Issue. Didn't mean to do a judo chop on you.

@zeppelin I think the :prepend / :append options you propose in your comment should be named :prepend_addon, :append_addon to be explicit. My proposed :prepend should not be renamed :prepend_html, but instead allow the user to supply a string or a string.html_safe depending on whether HTML is needed or not.

@zeppelin
Copy link

@dlee I think what @josevalim demonstrated takes care of the case where you want to provide the prepend/append HTML as-is. I was thinking about the :prepend_html that it should accept a hash, similar to :input_html.

To illustrate my point:
f.input :twitter, :prepend => "@", :prepend_html => { :class => "awesome" }

It does not, however, stops us from adding some polymorphism to the mix:
f.input :twitter, :prepend_html => "<span class='add-on'>@</span>"
(passing the whole HTML instead of a config hash)

But I guess the latter is a question of API-consistency.

@rafaelfranca
Copy link
Collaborator

I don't like the idea of add HTML tags in an option of the input method. As @josevalim said, we can use the block syntax to do such of thing.

PS: input_field is correct

@carlosantoniodasilva
Copy link
Member

I'm with @rafaelfranca on this one, I think there's no need for new extra options, but I do think we can make our little block input friend a little bit smarter to workaround the problem of having to give the input name twice.

@josevalim's example would look something like:

<%# ps: there's no variable passed to the block %>
<%= f.input :sample do %>
  Prepend stuff
  <%= f.input_field :sample %>
  Append stuff
<% end %>

We can make it a bit smarter by truly yielding a builder to the block:

<%= f.input :sample do |b| %>
  Prepend stuff
  <%= b.input_field %>
  Append stuff
<% end %>

This would avoid you from having to provide the input name twice, and would nicely solve the prepend/append problem in my opinion. Thoughts?

@rafaelfranca
Copy link
Collaborator

I like this idea.

@dlee
Copy link
Contributor

dlee commented May 10, 2012

👍

@zeppelin
Copy link

@carlosantoniodasilva You mean that f.input :sample, :prepend => "Prepend stuff" is rejected in favor of blocks? It's unclear to me if @rafaelfranca were only against passing HTML content as params, and you answered to this particular question, or does it apply to plain text too?

@rafaelfranca
Copy link
Collaborator

@zeppelin option was rejected in favor of blocks. I'll work on this issue

@ghost ghost assigned rafaelfranca May 10, 2012
@carlosantoniodasilva
Copy link
Member

@zeppelin I think that if the block change solves the problem of prepending / appending any text / html, then I'd go with it instead of adding new options to the api.

@zeppelin
Copy link

@carlosantoniodasilva @rafaelfranca I have to agree on the API argument.

@mckramer
Copy link

I like @carlosantoniodasilva's idea as far as that making sense, but I thought the main purpose of this was to simplify the markup for using prepend and append. See #525.

# From this (used proposed syntax)
= f.input :cost, wrapper => :prepend_and_append do |n|
      = content_tag :span, "$", class: "add-on"
      = n.number_field
      = content_tag :span, ".00", class: "add-on"

# To this
= f.input :cost, prepend: "$", append: ".00"

The wrapper class would only add the prepend class if the prepend option was used and vice versa. Or it would add both if both options were used.

@rafaelfranca
Copy link
Collaborator

The problem with this approach is that we are coupling SimpleForm with twitter bootstrap. I definitely don't want to do this.

Our intention, since the beginning, was to provide a easy way to configure SimpleForm with Third-party CSS libraries, not couple SimpleForm with they. If the library change we don't want to change SimpleForm.

For example, when we release SimpleForm 2.0 we didn't have the prepend-append input type. I don't want to change SimpleForm if the Twitter bootstrap add a new component called foo-bar.

Right now you can create easily a custom wrapper, input or maybe a helper in your application do to this.

I really want to add suport to prepend-append inputs, but not adding new options to the inputs.

@mckramer
Copy link

Ok, I understand that reasoning, but I think it could be designed so that initializers could set some of the options. Maybe if some of the other CSS libraries add prepend/append functionality, then I can pester you into adding support for it, haha. Although, the use cases for prepend/append can get complicated (using checkboxes or buttons instead of the simple text example I have given), so it might be best to leave it out.

However, I don't see how this proposed change handles the initial problem for this issue. There still isn't a prepend_and_append wrapper. (Only the singular prepend and append wrappers.) Maybe I missed something?

@yemartin
Copy link

yemartin commented Jul 3, 2012

I tried to block method to prepend an add-on, but it breaks the placeholder.
Before:

= f.input :username

After:

= f.input :username do
  .input-prepend
    %i.add-on.icon-envelope
    = f.input_field :username

Before, I get an automatic placeholder from I18n. After, the placeholder is gone.

@carlosantoniodasilva
Copy link
Member

@yemartin that's weird, using f.input_field should give you the placeholder as usual. Can you open a new issue please? Make sure you post the same you posted here, plus the output html of both cases if possible, and the SimpleForm wrapper you're using from the initializer. Thanks!

@richardkmichael
Copy link

Another use-case, as I've not seen this discussed (with replies) on the mailing-list or wiki/Nested-Models.

I would like to append (or prepend) arbitrary HTML by calling a helper, rather than "append" implying a specific tag (span) or class (add-on) as suggested by @mckramer.

When used with simple_form, nested_form provides a nice integration: simple_nested_form_for. In nested forms, it is common to want a "Remove" link beside each associated model.

I do this now as discussed above by @josevalim, @carlosantoniodasilva and @rafaelfranca:

simple_nested_form_for @book do |book|
  book.simple_fields_for :authors do |author|
    author.input :name do
      author.input_field :name
      author.link_to_remove "Remove"
    end

I don't think I can do this with a wrapper, because wrap_with: ... would "wrap", and even then it's with a tag: ..., and not Ruby code (helper, Proc, etc.). As alternatives, I'll investigate a custom input such as author.input_with_remove :name, or perhaps a custom component.

Bottom line, I would like a less-verbose usage. For example:

author.input :name, removable: true

Or perhaps it belongs as a option at the collection level:

book.simple_fields_for :authors, removable: true do |author|
  author.input :name
end

@jacob-carlborg
Copy link

Here's a quick and easy solution that is DRY:

  wrapper_class = "input-prepend"
  options = { tag: "div", class: "control-group", error_class: "error" }

  block = lambda do |b|
    b.use :html5
    b.use :placeholder
    b.use :label
    b.wrapper :tag => 'div', :class => 'controls' do |input|
      input.wrapper :tag => 'div', :class => wrapper_class do |prepend|
        prepend.use :input
      end
      input.use :hint,  :wrap_with => { :tag => 'span', :class => 'help-block' }
      input.use :error, :wrap_with => { :tag => 'span', :class => 'help-inline' }
    end
  end

  config.wrappers :prepend, options, &block

  wrapper_class = "input-append"
  config.wrappers :append, options, &block

  wrapper_class = "input-prepend input-append"
  config.wrappers :"prepend append", options, &block
  config.wrappers :"append prepend", options, &block

Use like this:

<%= f.input :foo, wrapper: "append prepend"  do %>
<% end %>

Or

<%= f.input :foo, wrapper: "prepend append"  do %>
<% end %>

This makes it look like you can specify multiple wrappers. I don't know if you think that's good or bad.

@Veejay
Copy link

Veejay commented Sep 2, 2013

Late to the party here, but after reading this whole page, I'm still not clear about whether or not this is actually easily doable to have a prepend-append kind of style around an input. I've found solution involving modifications to Rails initializers, none of them are really satisfactory.

We need to present domain name reservations on our website and it'd be extremely helpful. Thanks.

@bcackerman
Copy link

Easiest way to do it now is this FYI

<%= f.input :field, :wrapper_html => {class: "input-prepend input-append"} do %>
  <span class="add-on">$</span>
    <%= f.input_field :field %>
  <span class="add-on">%</span>
<% end %>

@Veejay
Copy link

Veejay commented Sep 5, 2013

@bcackerman Thank you. Much appreciated.

@kwerle
Copy link

kwerle commented Sep 17, 2013

Thank you! This information should really be added to the homepage.

@rafaelfranca
Copy link
Collaborator

This is not need to bootstrap 3 so I'm closing.

Thank you for the PR

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.