[discussion] Adding property for all elements of a builder

[blelump]

Hi @fran-worley ,

consider such use case: you know that form is a validable form and you'd like to add required: required for all elements ot this form and perhaps even to have a possibility to exclude an element from validation. I know this is messy, but sounds like a real use case, where you'd like to add require attribute to all form elements.

[fran-worley]

I understand your required use case, but can you explain about the validation one?

We could add an element_options argument to the builder initializer. That in turn could pass the element_options hash to your elements when they are called.

Such builder options would override any 'defaults' on the Element class, but would be overridden by options passed directly to the element.

[blelump]

Let me provide an example first:

<form> 
  <input required='required' name='...' >
  <input required='required' name='...' >
  <input required='required' name='...' >
  <select required='required' ></select>
  <textarea required='required' ></textarea>
</form>

All these elements above have one thing in common, all have required attribute. The whole idea is to inject such property into all elements. However, I imagine another use case, eg:

<form> 
  <input required='required' name='...' >
  <input name='...' >
  <input required='required' name='...' >
  <select required='required' ></select>
  <textarea ></textarea>
</form>

And that's why it becomes messy :-) Ideas?

[fran-worley]

If you were able to provide 'element_options' to the builder you could achieve the above like so:

builder = FancyBuilder.new(model: my_model, element_options: { required: 'required' })
builder.form(action: '/some_path') do |f|
  # every element you add via this builder would have the attribute 'required'
  # you could override this on specific elements by passing `required: nil`  into the options hash.

  # e.g.
  f.input :name # will have required attribute
  f.input :gender, required: nil # won't have required attribute
end

[blelump]

@fran-worley , sounds great! How about having it on a class level and defining like column_classes? Since I imagine, we're going type based builders, instead of having bunch of configuration, then each builder type provides some context, eg:

class FancyBuilder
  element_options { required: 'required' }
end 

class FancierBuilder < FancyBuilder
  element_options { class: %w(blah) } 
end 

The question is: should the opts from FancierBuilder be merged into FancyBuilder hash? Since it's inherited I'd expect such behaviour.

What do you think?

ps. I think these element_options should not apply to the form element itself.

[fran-worley]

Ah of course I'd not thought about form. I think we might have to whitelist/ blacklist elements which the options should/ shouldn't apply to. For example, we aren't going to want it to apply to divs or spans either.

# this would add required to all elements except those with form, div, hidden, or span keys
set_element_default(:required, 'required', except: [:form, :div, :hidden, :span]) 

# We are then of course going to start looking for ways to group elements so you do things like:
set_element_default(:required, 'required', only: :controls)

WRT inheritance, I agree that they should by but we will run into the same issue (#10) with classes.

[blelump]

Uhm, I am not sure about such solution. Imagine builder with bootstrap decorator, where we have :error, :hint etc unless they don't apply there.

Perhaps defining common attributes isn't a good idea at all, because from one hand it solves some problem, but on another introduces additional complexity. Nice to have, but with limited cost.

[fran-worley]

You could always subclass your controls add the required attribute and access them via required_input, required_select etc.

[fran-worley]

I think I agree. The benefits of saving a few option calls don't currently outweigh the additional complexity.

The only way that you could do it would be to group your elements and define defaults for a group e.g. all form controls rather than all elements.

Either way, it's a thought for another day 😄

[blelump]

Hi @fran-worley ,

Sorry for being unresponsive, but my mind was focused on other project. Anyway, I think such change would introduce too much complexity, especially if you'd consider to support :only or :except. With such approach, we're going to introduce additional options instead of trying to build type oriented forms. Lets consider client side form validation where you usually define some properties that define validation process. Now, when we have elements inherited, we're able to override particular form element and hence, provide specialized form definition for any use case. I am not sure yet, how useful/reliable is such approach, but let's test it in practice, because there're many many possible use cases for sure.

[fran-worley]

@blelump No worries, I'd decided to abandon this largely for the reasons you have stated.

Because Fomular builders render more than just controls, being able to specify default options that apply to all rendered elements is unlikely to ever be useful and certainly not for enough cases to justify expanding the API at this stage.

Are you happy for me to close this to allow us to focus on other things? We can always re-open/ re-raise if needed?

[blelump]

Sure, I'm closing now

[blelump]

@fran-worley ,

so far i've ended up with smth like:

    class Builder < Formular::Builders::Bootstrap3Horizontal

      ValidableElements = [:input, :select, :radio].freeze

      ValidableElements.each do |element|
        self.elements[element] = Class.new(self.elements[element]) do
          set_default :'required', 'required'
        end
      end

      element_set({
                    form: Validable::Form,
                    radio: Isa::Formular::Radio
      })
    end

[fran-worley]

@blelump this is essentially what I had intended on doing.

A builder could have multiple element_sets and you could define default options for a specified element set. I'd probably make an element_set an object which you could initialize with default options.

Just got to think about how it would work with method names where you have the same key name in multiple element sets...

# this would achieve the same thing as your code above
add_element_set :validable, 
                elements: { input: ValidableInput, select: ValidableSelect, radio: ValidableRadio }, 
                default_options: { required: 'required' } 

# our defined methods could come out as: 
# validable_input => ValidableInput
# validable_select => ValidableSelect
# validable_radio => ValidavbleRadio

# the name argument of the add_element_set method would be optional and if undefined, your keys would be defined as methods without a prefix.

[blelump]

Morning @fran-worley ,

to be honest, I am not sure about such approach :smile: . I get the idea and it sounds really good, especially if you'd consider a form with validable and 'default' form elements, because it easily lets you manage many kind of form elements within one builder. So for sure, it is realy flexible. On the other hand it only groups a certain elements by given properties...

Well, let's try it!

[fran-worley]

This won't make 0.1.0 as I need to consider it further. Rest assured though, I will find a cleaner solution.

[blelump]

@fran-worley ,

well, having separated ElementSet and Builder definitely sounds like a great idea and I'd implement it anyway. Not sure though how to inject default_options into elements of ElementSet, because with current design, we operate more on types than on objects and hence it would be diffcult to add concrete options into element type under condition that such options occur only in given ElementSet.