Emerson / ember-form-master-2000

A simple form builder for Ember built as an Ember-CLI addon.
MIT License
40 stars 18 forks source link

make application-wide configuration (service) overrideable per field #36

Closed jelhan closed 8 years ago

jelhan commented 8 years ago

make application-wide configuration (service) overrideable per field; also observes changes in application-wide configuration

Example for http://getbootstrap.com/css/#forms-inline

{{#fm-form formClass='form-inline'}}
  {{fm-field labelClass='sr-only' label='Email address'}}
  {{fm-field labelClass='sr-only' label='Passwort'}}
  {{#fm-submit submitButtonClasses=null}}
    Sing in
  {{/fm-submit}}
{{/fm-form}}

Also fixes #26 / #35.

jelhan commented 8 years ago

@Emerson would you mind to change submitButtonClasses (https://github.com/Emerson/ember-form-master-2000/blob/master/addon/services/fm-config.js#L4) also to a string submitButtonClass? Would make it much more easier to override and would unify things. Beside it's only property having to classes in default configuration I can't seen what makes it special. In non default configuration people might need only one submit button class and multiple label classes...

Emerson commented 8 years ago

Hey @jelhan - thanks for looking at this! I think it makes good sense and want to merge it.

Feel free to update the submitButtonClasses to be more inline with the rest of the code. The reason it initially had two classes was to match bootstrap, which is kinda the default markup I used for this library (just because it's so popular). Happy to use a string instead of an array - but can we make it btn btn-primary by default so that it doesn't break the demo page?

Did you want me to wait on merging until you've updated the submit button stuff?

Emerson commented 8 years ago

@g-cassie - if you have any thoughts on this let me know

jelhan commented 8 years ago

@Emerson changed to submitButtonClass.

Emerson commented 8 years ago

Cool - thanks :hamster:

Going to let this percolate for a few hours in case anyone else wants to chime in, but I feel good about it.

g-cassie commented 8 years ago

I think this makes sense. I would like to make use of the hasBlock property introduced in 1.13 to provide full customization on a per field basis eventually but I think that will require a more substantial refactor to make it possible.

Emerson commented 8 years ago

Cool - merging now. I should probably cut a release on NPM...