simple_form icon indicating copy to clipboard operation
simple_form copied to clipboard

Unable to add html options for collection labels other than class

Open JoeWoodward opened this issue 3 years ago • 0 comments

Environment

  • Ruby 3
  • Rails 6
  • Simple Form 5.1.0

Current behavior

This isn't a bug, it's a limitation in how the code is written (only passes class: label_class to the builder which accepts any options).

I'm using radio button collections in my app and using the common practice of hiding the radios and styling the labels. To make this work with tab navigation it is necessary to add tabindex to the labels. I did some digging and found that the only option that is respected when building the labels is :item_label_class. ActionView allows us to pass any options we want to the builder so not sure why we're restricted to just the class.

I've monkey patched locally and it works great.

I changed this method to also respect @options[:item_label_html] with the following change

    def render_component(builder)
      label_class = "#{@options[:item_label_class]} #{@options.dig(:item_label_html, :class)} collection_radio_buttons".strip
      label_options = (@options[:item_label_html] || {}).merge(class: label_class)

      builder.radio_button + builder.label(label_options)
    end

Now it's possible to config the inputs with

  config.wrappers :horizontal_collection, item_wrapper_class: '', item_label_html: { tabindex: '0' }, item_label_class: 'form-collection-label', tag: 'div', class: 'input-wrapper' do |b|
    b.use :html5
    b.optional :readonly
    b.wrapper :legend_tag, tag: 'legend', class: 'form-label', error_class: 'has-error' do |ba|
      ba.use :label_text
    end
    b.use :hint, wrap_with: { tag: 'p', class: 'form-hint' }
    b.wrapper tag: 'div', class: 'input-group' do |bb|
      bb.use :input, class: 'form-collection-input', error_class: 'has-error'
    end
    b.use :full_error, wrap_with: { tag: 'p', class: 'form-error' }
  end

or

f.input :radio, item_label_html: { tabindex: '0' }

If this is a change you would allow I'd be happy to create a PR and add test coverage.

JoeWoodward avatar Jul 19 '21 10:07 JoeWoodward