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

Non-semantic forms for editors #62

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
@becdetat
Contributor

becdetat commented Nov 9, 2013

pia7y

This lets you use Chameleon Forms within a partial, an editor template or a view template, by using BeginChameleonEditor, which just skips the <form> markup generation.

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Nov 9, 2013

Member

mind=blown

Member

robdmoore commented Nov 9, 2013

mind=blown

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Nov 9, 2013

Member

I've just had a long conversation with @bendetat about this and this is the first step in a series of steps to get nice "partial"-style support (as well as the ability to use Chameleon to generate portions of a form's HTML for things like SPAs).

The main thing to note here is this is the most basic possible implementation and relies on the developer to use it correctly - the documentation will need to reflect this.

Ben is going to make a couple of tweaks then I'll merge in.

Member

robdmoore commented Nov 9, 2013

I've just had a long conversation with @bendetat about this and this is the first step in a series of steps to get nice "partial"-style support (as well as the ability to use Chameleon to generate portions of a form's HTML for things like SPAs).

The main thing to note here is this is the most basic possible implementation and relies on the developer to use it correctly - the documentation will need to reflect this.

Ben is going to make a couple of tweaks then I'll merge in.

@becdetat

This comment has been minimized.

Show comment
Hide comment
@becdetat

becdetat Nov 11, 2013

Contributor

Yes, crap, missed that. will do

Contributor

becdetat commented Nov 11, 2013

Yes, crap, missed that. will do

@MRCollectiveCI

This comment has been minimized.

Show comment
Hide comment
@MRCollectiveCI

MRCollectiveCI Nov 13, 2013

Contributor

TeamCity ChameleonForms :: Continuous Integration Build 0.9.140 is now running

Contributor

MRCollectiveCI commented on d5d78f3 Nov 13, 2013

TeamCity ChameleonForms :: Continuous Integration Build 0.9.140 is now running

This comment has been minimized.

Show comment
Hide comment
@MRCollectiveCI

MRCollectiveCI Nov 13, 2013

Contributor

TeamCity ChameleonForms :: Continuous Integration Build 0.9.140 outcome was SUCCESS
Summary: Tests passed: 433 Build time: 0:0:0

Contributor

MRCollectiveCI replied Nov 13, 2013

TeamCity ChameleonForms :: Continuous Integration Build 0.9.140 outcome was SUCCESS
Summary: Tests passed: 433 Build time: 0:0:0

@becdetat

This comment has been minimized.

Show comment
Hide comment
@becdetat

becdetat Nov 13, 2013

Contributor

@robdmoore I finally renamed those tests 👊

Contributor

becdetat commented Nov 13, 2013

@robdmoore I finally renamed those tests 👊

@becdetat

This comment has been minimized.

Show comment
Hide comment
@becdetat

becdetat Nov 13, 2013

Contributor

So I've been pondering this and I'm not convinced this is a good solution. I've come up with an alternative that should be easy to implement but would be a breaking change if this PR is accepted so kill this please :-/

My thinking is that the partial or editor doesn't care about the form or section or whatever. All it cares about is getting the context that applies to it.

So what I've done with this PR is a hacky way to let the editor make something that looks like a context but is actually just a limited form.

I'm coming aroung to not using @Html.EditorFor directly. Rob 💖 . So the form would do something like:

@s.EditorFor(x => x.Foo) // or @s.PartialFor(x => x.Foo, "_FooPartial")

which creates a Context<Foo> (which subclasses Section for now - to cancel out the start/end markup - but would end up being Section's superclass), drops it into the view data, and calls Html.EditorFor.

Then the editor's entry point into CF is

@using (var context = Html.GetChameleonContext) {}

which pulls out the context.

Contributor

becdetat commented Nov 13, 2013

So I've been pondering this and I'm not convinced this is a good solution. I've come up with an alternative that should be easy to implement but would be a breaking change if this PR is accepted so kill this please :-/

My thinking is that the partial or editor doesn't care about the form or section or whatever. All it cares about is getting the context that applies to it.

So what I've done with this PR is a hacky way to let the editor make something that looks like a context but is actually just a limited form.

I'm coming aroung to not using @Html.EditorFor directly. Rob 💖 . So the form would do something like:

@s.EditorFor(x => x.Foo) // or @s.PartialFor(x => x.Foo, "_FooPartial")

which creates a Context<Foo> (which subclasses Section for now - to cancel out the start/end markup - but would end up being Section's superclass), drops it into the view data, and calls Html.EditorFor.

Then the editor's entry point into CF is

@using (var context = Html.GetChameleonContext) {}

which pulls out the context.

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Nov 15, 2013

Member

So as per my SMS - this is EXACTLY what I was thinking.

It's gonna be amazing.

I would love to remote pair with you on it.

You are my hero.

o/

Member

robdmoore commented Nov 15, 2013

So as per my SMS - this is EXACTLY what I was thinking.

It's gonna be amazing.

I would love to remote pair with you on it.

You are my hero.

o/

@robdmoore

This comment has been minimized.

Show comment
Hide comment
@robdmoore

robdmoore Nov 15, 2013

Member

I'm thinking that with editor templates you might want to specify it:

  • As an attribute for a particular field (there is UIHint of course, which I think we should respect but it would be nice to also just say "use editorfor" and leave it to the default editor template resolution stuff and avoid the magic string)
  • By convention for certain classes across the entire app (e.g. a global configuration)
  • Possibly on an ad hoc basis in a view

The other thing to consider is whether or not to use the editor template for the whole thing or just for generating the Field Element (or possibly allowing the configuration of both). re: partials - cool: perhaps that's the difference - if it's a partial it's the whole thing if it's an editor template then it's just generating the field html (dead easy to add a field generator handler that delegates to Html.EditorFor).

Then after we decide on all that (or a logical subset) we need to figure out the MVP so we can do a small thing first and build up from there.

Member

robdmoore commented Nov 15, 2013

I'm thinking that with editor templates you might want to specify it:

  • As an attribute for a particular field (there is UIHint of course, which I think we should respect but it would be nice to also just say "use editorfor" and leave it to the default editor template resolution stuff and avoid the magic string)
  • By convention for certain classes across the entire app (e.g. a global configuration)
  • Possibly on an ad hoc basis in a view

The other thing to consider is whether or not to use the editor template for the whole thing or just for generating the Field Element (or possibly allowing the configuration of both). re: partials - cool: perhaps that's the difference - if it's a partial it's the whole thing if it's an editor template then it's just generating the field html (dead easy to add a field generator handler that delegates to Html.EditorFor).

Then after we decide on all that (or a logical subset) we need to figure out the MVP so we can do a small thing first and build up from there.

@robdmoore robdmoore closed this Nov 15, 2013

@robdmoore robdmoore referenced this pull request Nov 22, 2014

Open

Adding proper extensibility to ChameleonForms #107

5 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment