backbone-forms icon indicating copy to clipboard operation
backbone-forms copied to clipboard

Add Form.NestForm template to allow different theming for nested forms used in Object editor

Open rjmackay opened this issue 11 years ago • 13 comments

Adds Form.NestedForm and uses it in editors/Object. NestedForm extends Form but uses a different template which doesn't have a form tag. This generates valid HTML, but leaves the flexibility to tweak the nested form template if needed.

rjmackay avatar Jun 26 '14 03:06 rjmackay

+1 this is a great way to do it - I've done similar things in 2 different projects

However you need to make the same logic in src/editors/nestedmodel.js

Doing this also makes it super easy to have a nice InlineNestedModel List Item editor -- rather than having it use a Modal.

philfreo avatar Jun 26 '14 04:06 philfreo

+1 I need that!

fonji avatar Jun 26 '14 07:06 fonji

Looks like a much needed improvement however I've got several tests failing with these changes; is are you able to update these?

After the merge would be great to simplify the list editor so modals aren't required as @philfreo says

powmedia avatar Jun 26 '14 14:06 powmedia

I believe this implies a small modification of the bootstrap3 templates. For example,

Form.editors.Base.prototype.className = 'form-control';

should be replaced by something similar to this:

Form.editors.Base.prototype.className = function() {
  if (this.hasNestedForm)
    return '';
  return 'form-control';
};

fonji avatar Jun 26 '14 14:06 fonji

@fonji except NestedModel doesn't set hasNestedForm so I'm not sure that works? Maybe I should just unset className for specific editor types?

rjmackay avatar Jun 27 '14 08:06 rjmackay

Updated to fix unit tests, and simplified NestedModel while at it. I did run into some issues running tests in Firefox, so maybe someone else could double check that?

rjmackay avatar Jun 27 '14 09:06 rjmackay

@rjmackay Will do.

fonji avatar Jun 27 '14 10:06 fonji

I had errors with every browser I used for the tests (firefox on linux and windows, chromium on linux, safari on windows and IE). I don't have the time to debug now, sorry.

fonji avatar Jun 27 '14 11:06 fonji

Here's an example of using this with a List editor now

http://jsfiddle.net/dW2Qu/484/

Either we should get rid of the Modal adapters or we should add an InlineNestedModel like in the jsfiddle above

philfreo avatar Jun 28 '14 01:06 philfreo

I tried testing once more and have good news: tests actually works on most tested browsers. I still have trouble with... IE, for a change. I'll keep you guys up to date when I found what's bothering it. Edit: IE tests failing aren't related to this merge. See my next comment.

Bad news is: I'm an idiot. I git-cloned an old version of @rjmackay's repository and that's why it didn't work. I guess I forgot how to git remote. Shame on me. Edit: I know why I am an idiot. I cloned the master branch. Very usefull...

fonji avatar Jul 07 '14 14:07 fonji

The problems with IE are focus and blur-related and have nothing to do with these changes. They also happen with powmedia/backbone-forms/master.

fonji avatar Jul 07 '14 15:07 fonji

I made a small change to allow each instance and subclass to set their own NestedForm and NestedField class. I'm not sure it's usefull so I didn't write any tests yet. Please let me know if you're interested, I'll write some and create a pull request to @rjmackay's version.

Edit: usage example:

Backbone.Form.editors.List.InlineObject = Backbone.Form.editors.Object.extend({});
Backbone.Form.editors.List.InlineObject.NestedField = Backbone.Form.Field;

So the applied template is the same.

fonji avatar Jul 09 '14 09:07 fonji

+1. I could put this utility to use right away - but how specify the template for the nestedmodel? I need an inline list of objects, like in @philfreo's fiddle, but I need to style it with a template.

slackjaw avatar Aug 01 '14 17:08 slackjaw