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

support bootstrap horizontal form #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jelhan
Copy link
Contributor

@jelhan jelhan commented Nov 20, 2015

Introduces inputWrapperClass, an element which wraps input fields, errors and help texts.
This is needed for columns in bootstrap horizontal from.

Example usage:

// routes/application.js
import Ember from 'ember';

export default Ember.Route.extend({
  beforeModel() {
    this.set('inputWrapperClass', 'col-sm-10');
    this.set('labelClass', 'col-sm-2');
    this.set('formClass', 'form-horizontal');
  },
  fmConfig: Ember.inject.service('fm-config')
});

You will have a bootstrap horizontal form.

Column offset are not handeled automatically for fields without a label
(fm-checkbox, fm-submit) or if label property is empty. As this seems
to be to much bootstrap specific for this general form-builder libary.

Together with #36
we could make a nice horizontal bootstrap form like this one:
http://getbootstrap.com/css/#forms-horizontal

{{#fm-form}}
  {{fm-field label='Email'}}
  {{fm-field label='Password'}}
  {{fm-checkbox label='Remember me' inputWrapperClass='col-sm-10 col-sm-offset-2'}}
  {{#fm-submit inputWrapperClass='col-sm-10 col-sm-offset-2'}}
     Sign in
  {{/fm-submit}}
{{/fm-form}}

Of course you should have service configuration set like above.

@g-cassie
Copy link
Collaborator

I think that this is pretty custom behaviour which can easily be added by extending the component and using a custom template.

// horizontal-field.js
import FmField from  'ember-form-master-2000/components/fm-field';
import layout from '../templates/components/fm-field';
export default FmField.extend({
  layout: layout
});
// horizontal-field.hbs
{{#if label}}
  <label for="{{forAttribute}}" class="{{labelClass}}">{{{label}}}</label>
{{/if}}

<div class="horizontal-classes">
{{#if isBasicInput}}
  {{fm-input
    type=type
    value=value
    classNameBindings='errorClass inputClass'
    maxlength=maxlength
    placeholder=placeholder}}
{{/if}}

{{#if isSelect}}
  {{fm-select
    content=content
    optionValuePath=optionValuePath
    optionLabelPath=optionLabelPath
    prompt=prompt
    value=value
    action=(action 'selectAction')
  }}
{{/if}}

{{#if isTextarea}}
  {{fm-textarea
    value=value
    classNameBindings='errorClass textareaClass'
    placeholder=placeholder
    rows=rows
    cols=cols
    maxlength=maxlength
    spellcheck=spellcheck
    disabled=disabled
  }}
{{/if}}

{{#if errors}}
  {{fm-errortext errors=errors}}
{{/if}}

{{#if helptext}}
  {{fm-helptext helptext=helptext}}
{{/if}}
</div>

If we could do something to make this extension process easier - like making it so you do not have to copy as much template from the default form-master template I think that would be a better way to proceed.

@jelhan
Copy link
Contributor Author

jelhan commented Nov 20, 2015

@g-cassie It's of course also possible by overrides and reopen. This was the way I had this one implemented before. But I think a wrapper around these elements is required by many form markup that's why I introduced it as a PR. Also this addon is configured to work with bootstrap as default. But if you come to a decision not to include this one, it's also fine for me.

@jelhan jelhan force-pushed the bootstrap-horizontal branch 2 times, most recently from 380adb2 to 87fdf86 Compare November 20, 2015 18:41
@jelhan
Copy link
Contributor Author

jelhan commented Nov 20, 2015

Rebased after #36 was merged.

errors and help texts.
This is needed for columns in bootstrap horizontal from.

Example usage:

```
// routes/application.js
import Ember from 'ember';

export default Ember.Route.extend({
  beforeModel() {
    this.set('inputWrapperClass', 'col-sm-10');
    this.set('labelClass', 'col-sm-2');
    this.set('formClass', 'form-horizontal');
  },
  fmConfig: Ember.inject.service('fm-config')
});
```

You will have a bootstrap horizontal form.

Column offset are not handeled automatically for fields without a label
(fm-checkbox, fm-submit) or if label property is empty. As this seems
to be to much bootstrap specific for this general form-builder libary.

Together with Emerson#36
we could make a nice horizontal bootstrap form like this one:
http://getbootstrap.com/css/#forms-horizontal

```
{{#fm-form}}
  {{fm-field label='Email'}}
  {{fm-field label='Password'}}
  {{fm-checkbox label='Remember me' inputWrapperClass='col-sm-10 col-sm-offset-2'}}
  {{#fm-submit inputWrapperClass='col-sm-10 col-sm-offset-2'}}
     Sign in
  {{/fm-submit}}
{{/fm-form}}
```

Of course you should have service configuration set like above.
@jelhan jelhan force-pushed the bootstrap-horizontal branch from 87fdf86 to 35eea3a Compare November 26, 2015 10:47
@jelhan
Copy link
Contributor Author

jelhan commented Nov 26, 2015

Rebased after #38 was merged.

@g-cassie
Copy link
Collaborator

@jelhan

How about the following API?

{{fm-field
   fieldLayout='bs-horizontal'
   ...
}}

Then we can provide some predefined layouts.

// ember-form-master-2000/templates/field-layouts/bs-horizontal.hbs
{{#if label}}
  <label for="{{forAttribute}}" class="{{labelClass}}">{{{label}}}</label>
{{/if}}

<div class="horizontal-classes">
{{component widgetName
   value=(mut value)
   widgetAttrs=widgetAttrs
}}

{{#if errors}}
  {{fm-errortext errors=errors}}
{{/if}}

{{#if helptext}}
  {{fm-helptext helptext=helptext}}
{{/if}}
</div>

@g-cassie g-cassie closed this Jan 18, 2016
@g-cassie g-cassie reopened this Jan 18, 2016
@jelhan
Copy link
Contributor Author

jelhan commented Jan 18, 2016

Thanks for digging into this.

I'm not quite sure about the api. Default template and horizontal template would be very similar for my case - just two lines would be different. If bootstrap grid columns should be supported, there would be some more changes but template specific parameters would be needed. Feels like the api would not solve the DRY issue but make fm-field much more complex. Extending fm-field seems to be a cleaner solution. Perhaps it's all about splitting up default field template so that it would be easier to customize?

Also it's a little bit confusing that basic inputs, textarea and select using fm-field while radio groups, submit and checkboxes aren't. So to at a wrapper class around input fields (as neeeded here) you have to customize 4 components. It's also a source of inconsistency as introduced by 53f3430.

@g-cassie
Copy link
Collaborator

I would like to refactor so that we have a concept of a "widget". A widget is a generic term for input, textarea, select, radio group, jquery datepicker etc. It should be extensible so you can make any widget you want and still get standard label, error and help text from the fm-field class. If we had this refactor in place, I think it would be trivial to add new layouts and a lot less DRY than my example above. Also I think we could have layouts for popular css frameworks available in the form-master repo so in most cases it would be zero work.

In terms of making this "widget" class, I think we really need the hash helper introduced in Ember 2.3. So the api would look something like this:

{{fm-field
   fieldLayout='bs-horizontal'
   widgetName='my-autocomplete-input'
   widgetParams=(hash 
      my-autocomplete-choices=someArray
      my-autocomplete-labelField='name'
      my-autocomplete-valueField='id'
  )
  label='Country'
  value=(mut country)
}} 

So I agree the layout option above is not ideal now, but I think if we unified all the field types so there is less repetition in the code base it could have a nice syntax and be very extensible.

@jelhan
Copy link
Contributor Author

jelhan commented Jan 18, 2016

That widget api looks really good. @g-cassie thanks for your work on that! Let me know if I could help you.

@Emerson
Copy link
Owner

Emerson commented Jan 18, 2016

Love the API as well as the notion of an extensible widget. Just need to wrap my mind around the new params and mut syntax.

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

Successfully merging this pull request may close these issues.

3 participants