symfony2admingenerator / AvocodeFormExtensionsBundle

(old-legacy) Symfony2 form extensions for Admingenerator project (also working standalone!)
Other
48 stars 31 forks source link

[Patch] Change logic to add avocode twig form resources #9

Closed sescandell closed 11 years ago

sescandell commented 11 years ago

Add twig form resources at the beginning of available resources (so application can override default renders provided by avocode formextensions bundle).

ioleo commented 11 years ago

@sescandell These files must also be loaded after Admingenerator form resources:

sescandell commented 11 years ago

@loostro

I don't think to test for specific Admingenerator resource name is a good idea. Because of two things:

So, to be "independent" about Admingenerator and any other Bundle, I simply added the same logic I introduced in the Admingenerator:

So finally, it permits the developper to import them as he wants depending on its project and its context (by the way, because, I think, CompilerPass order execution is depending on how they are defined in AppKernel, I'm not sure I can force AvocodeFormExtensions resources to be added before Admingenerator ones)

Sounds better for you?

ioleo commented 11 years ago

If Admingenerators form_row block overwrites FormExtensions form_row then the help block will not be rendered and thus HelpMessageExtension will not work. Thats why our resource should be loaded after Admingenerator.

Why do we/I care about Admingenerator? Becouse the basic idea behind this bundle was to provide form types for Admingenerator project. Ofcourse, we may work to make it independent, but I think our first goal should be to make it as easy to use (with Admingenerator) as possible.

sescandell commented 11 years ago

If Admingenerators form_row block overwrites FormExtensions form_row then the help block will not be rendered and thus HelpMessageExtension will not work. Thats why our resource should be loaded after Admingenerator.

Oh yes, sorry, I didn't see this minor change,

Why do we/I care about Admingenerator? Becouse the basic idea behind this bundle was to provide form types for Admingenerator project. Ofcourse, we may work to make it independent, but I think our first goal should be to make it as easy to use (with Admingenerator) as possible.

Ok I see your point. But, as I said, I think we can't guarantee execution order of CompilerPass (I almost sure it is dependent on AppKernel declaration order). I can't be sure that Avocode FormCompiler will be executed after the admingenerator one. So, IMO, current solution is the more flexible:

If you don't think this is enough... I'll add a check on Admingenerator includes (but I'm not sure we can guarantee Avocode ones will be inserted after... maybe except if we precise that this bundle must be declared after Admingenerator in AppKernel... but that's not really a perfect solution IMO)

Just let me now,

sescandell commented 11 years ago

@loostro ,

If you don't mind, I would like to merge this PR (I need this flexibility for my project).

Could we so first merge as it, and then discuss on if we really need to check for Admingenerator inclusion (and handle order etc.)?

ioleo commented 11 years ago

@sescandell You can merge anything and anytime without asking me. If I (or anyone) have problems with something we'll just open an issue and discuss it :)