These are chat archives for symfony2admingenerator/GeneratorBundle

9th
Apr 2015
Matej Velikonja
@matejvelikonja
Apr 09 2015 08:00
@loostro morning :) can you send me a screenshot of how s2a_collection_fieldset should look? I think it got broken with updating to adminlte2. Would like to fix it, just that I need to know how it should work. you can also send me an example of s2a_collection_table
ioleo
@ioleo
Apr 09 2015 12:57
let me see
it looks like this on my app
this is two embeded forms
so it goes like this, store has gallery and gallery has collection of GaleryImages which are s2a_single_upload
maybe I did somethign wrong, not sure
ioleo
@ioleo
Apr 09 2015 13:03
fieldset.png
But this is on a production project, which is locked on previous admingenerator version
Stéphane
@sescandell
Apr 09 2015 13:04
Hi everyone
ioleo
@ioleo
Apr 09 2015 13:04
I don't have a project on current master that uses s2a_collection_fieldset
hi Stephane :)
well, the problem is i think, that the previous admingenerator (and formextensions) were based on boostrap2
Stéphane
@sescandell
Apr 09 2015 13:06

@matejvelikonja take care that there is no "direct link" (call it dependency if you prefer) between FormExtensions and GeneratorBundle.
It might be effectively broken on AdminLTE, but I'm just wondering how are you going to "fix it"?

We cannot assume FormExtension is used with AdminLTE

Matej Velikonja
@matejvelikonja
Apr 09 2015 13:06
I found out what has to be changed, but I have the problem is that one class cannot be chnaged easily
ioleo
@ioleo
Apr 09 2015 13:06
and probably, your problem is, the bundle got updated to Bootstrap3, but FormExtensions didn't
the problem is here with this col-md-4
if I change this to col-md-12 and add some CSS it works cool
but I cannot easily chnage it only for collection
or I don’t know how :)
ioleo
@ioleo
Apr 09 2015 13:08
@matejvelikonja can you inspect your page and copy-paste source code on pastebin ?
or at least the relevant part -> the "col-md-4" item and everything "inside"
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:09
sure
only html?
ioleo
@ioleo
Apr 09 2015 13:09
yes i want to see the structure and classes
only the relevant part if you can :)
Stéphane
@sescandell
Apr 09 2015 13:11

the problem is here with this col-md-4
if I change this to col-md-12 and add some CSS it works cool
but I cannot easily chnage it only for collection

You should be able to override the template through the block named form_col_YOU_COLUMN_HERE but I agree this is sporadic and is more like a workaround than a real solution

Matej Velikonja
@matejvelikonja
Apr 09 2015 13:11
yeah, would like to solve it on bundle levle
Stéphane
@sescandell
Apr 09 2015 13:11
you can use the class: form-model-type-{{ column.formType|lower }} .... (EDIT: no this is wrong... you cannot)
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:12
Stéphane
@sescandell
Apr 09 2015 13:12
and in CSS force it to behaves like col-md-12
form-model-type-\modissa\adminbundle\form\type\gallery\edittype ouch
this is an error :D
a filter is missing here
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:13
what filter?
Stéphane
@sescandell
Apr 09 2015 13:14
Here: form-model-type-{{ column.formType|lower }} I think classify|replace({'.': '-'}) is missing to remove \ caracters
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:16
but this still does not solve the real problem
Stéphane
@sescandell
Apr 09 2015 13:16
no no
just thinking out loud
ioleo
@ioleo
Apr 09 2015 13:17
ok i've cleaned it up a bit
removed the stuff inside data-prototype tag
Stéphane
@sescandell
Apr 09 2015 13:18

For your situation: we don't provide anything for now. For now you can have workaround:

  • by Twig block customization
  • using the col-md-12 as a key for your row field set definition in your generator

(IMO)

ioleo
@ioleo
Apr 09 2015 13:18
oh and i see you're from Germany :)
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:18
no :)
I’m from Slovenia, but working for Swiss company and clients ;)
ioleo
@ioleo
Apr 09 2015 13:19
ahh :D
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:20
otherwise the table collection works better, so for the moment I can use this. But I would really like to prefer the fieldset one, so that each images is floated and they would create some kind of grind. It would look really nice
you are polish, right?
ioleo
@ioleo
Apr 09 2015 13:21
http://pastebin.com/SSkZ0yqQ more shortened version
yes
OK, so I see the fieldsets have col-md-4 class (which should make a 3-item-in-a-row grid) and they are inside a "row-fluid" div
these class names are valid for bootstrap3, so this should work
can you inspect one fieldset item and see if there are any CSS properties overriden by adminlte CSS file?
in my (working) version I don't have the control-group classes too
maybe it's the control-group overriding some key CSS property?
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:26
as far as I see, the adminLte ovverides only fonts
BTW: are forms or generator using some kind of WYSIWYG editor?
so it can be easil integrated
about fieldsets: I can send you saved pages in HTML, so you can easily look at it. Just give me email :)
ioleo
@ioleo
Apr 09 2015 13:33
you can use TrsteelCKEditorBundle
i use it in a project on current master
so i know it works well
if you need file manager for CKEditor you can use ElFinderBundle
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:36
I’m also using this one, so it’s familiar to me
ioleo
@ioleo
Apr 09 2015 13:36
the s2a_collection_fieldset and s2a_collection_table have special hacks for CKEditor to allow "sortable" functionality
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:36
will try it out, thx
ioleo
@ioleo
Apr 09 2015 13:39
about fieldsets: i'll setup a demo project later today and see myself
feel free to open an issue on github
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:43
ok. great, because I would be great that I could finish it by tomorrow afternoon :)
ioleo
@ioleo
Apr 09 2015 13:45
@sescandell are you OK with merging #117 or is there some reason I shouldn't do it?
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:53
I have taken a look at #117, and it’s basically so funny what you did. It actually does the same thing, just differently and sension lab is than happy :)
I thin kthe idea behind not using DIR
is that you use kernel’s resource locator
i thin kusing reflection for locating files it’s guite heavy on CPU power
Bob van de Vijver
@bobvandevijver
Apr 09 2015 13:54
It makes it easier for people to overwrite stuff
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:55
you should use something like this
$kernel = $container->getService('kernel');
$path = $kernel->locateResource('@AdmeDemoBundle/path/to/file/Foo.txt');
at least for files
for directory something else
ioleo
@ioleo
Apr 09 2015 13:59
actually useing the reflection class is not so heavy and it's the recommended (by SensioInsight!) way to do this
Matej Velikonja
@matejvelikonja
Apr 09 2015 13:59
really? :)
ioleo
@ioleo
Apr 09 2015 14:00
yeah
and yes, It does the same thing, it's supposed to, otherwise it wouldn't work :P
i'm not sure we actually need to use the resource locator
it's not like we actually need to change the current behaviour
Matej Velikonja
@matejvelikonja
Apr 09 2015 14:01
well, if sension lab recommends reflection, that we should go with this way, I think they know better than us :)
ioleo
@ioleo
Apr 09 2015 14:01
the only motivation behind this change was to make sensio insight happy
but since Stephane was sceptical, I want to make sure im not missing some drawbacks of this, before i merge this
the performance shouldn't be a problem, becouse it's not like this part of code is called thousand times per request
@bobvandevijver what do you mean with easier to overwrite? as far as I know, there should be no diffrence in useing __DIR__ and ReflectionClass in terms of overriding
Bob van de Vijver
@bobvandevijver
Apr 09 2015 14:17
There is
Matej Velikonja
@matejvelikonja
Apr 09 2015 14:17
tell us :)
Bob van de Vijver
@bobvandevijver
Apr 09 2015 14:18
The __DIR__ would return the location in your App/Resources/AdminGeneratorBundle/FileYouAreOverwriting.php instead of the expected one in the vendor directory
While the ReflectionClass should always return the original one
ioleo
@ioleo
Apr 09 2015 14:19
no.. that cant be true
i've got to test this :D
Bob van de Vijver
@bobvandevijver
Apr 09 2015 14:20
Oh please do, this is just an theory ;)
But I've already problems with this in the templates directory (https://github.com/symfony2admingenerator/GeneratorBundle/blob/master/Resources/templates/Doctrine/ActionsBuilderAction.php.twig), as they also use an relative path
ioleo
@ioleo
Apr 09 2015 14:22
no wait actually that may be true
the __DIR__ does not change if you EXTEND files
but the symfony2 resource overriding system does not extend
i dont actually know how it works internally, interesting question :)
if its like you say, that would explain WHY SensioInsight red flags all __FILE__ and __DIR__ uses
Matej Velikonja
@matejvelikonja
Apr 09 2015 14:25
I’ve also tried this. and actually in my opinion DIR should work better than reflection
for example if you extend this class, that all template would stoped working, because it would look for them in the different directory, so you would have to copy them manually
not good in my opinion
or you would need to ovveride also the logic for finding files
I think the best solution would be, to get via kernel the location of bundle and than work from there
ioleo
@ioleo
Apr 09 2015 14:27
actually we probably should change all extend tags like {% extends '../CommonAdmin/ActionsAction/ActionsBuilderAction.php.twig' %}
to use full paths
then, overriding only one file should still work
Matej Velikonja
@matejvelikonja
Apr 09 2015 14:31
extends should be used with ‘@'NameOfTheBundle
@loostro do you still thing that using reflection is good idea?
Stéphane
@sescandell
Apr 09 2015 14:32
SOrry was on something else
ioleo
@ioleo
Apr 09 2015 14:32
we've had quite a discussion on #117
Stéphane
@sescandell
Apr 09 2015 14:33

extends should be used with ‘@'NameOfTheBundle

Except that this is not handle by the GeneratorBundle.... This is managed through an independant library not based on symfony so not base on Bundle

For all your others questions.... Reflection or not... Overriding or not... I don't have any idea now, I'm on something else
Bob van de Vijver
@bobvandevijver
Apr 09 2015 14:45
I might be abled to check it tonight, otherwise it would be tomorrow evening
ioleo
@ioleo
Apr 09 2015 16:45
im off for today, cya
Bob van de Vijver
@bobvandevijver
Apr 09 2015 18:11
Okay, so it is like the following:
__DIR__ always returns the directory of the current file
And that is the file in which the __DIR__ is actually placed
Matej Velikonja
@matejvelikonja
Apr 09 2015 18:16
@bobvandevijver yes, and the reflection returns the path to the initialzed class, so if you would extend it, it would return the path to this child class
Bob van de Vijver
@bobvandevijver
Apr 09 2015 18:16
Ho ho, I'm not ready ;)
And indeed, that is true
I was really hoping that it would return the location of the original bundle as per symfony inheritance
But it appears to do no such thing :(
Matej Velikonja
@matejvelikonja
Apr 09 2015 18:22
yeah, and this is really not good
Bob van de Vijver
@bobvandevijver
Apr 09 2015 18:22
Well, the result is the same, and I think I like the reflectionclass better
Matej Velikonja
@matejvelikonja
Apr 09 2015 18:23
no, the result is not the same, that’s the problem
it will work ok, until you want to ovveride the class in your bundle, and than you cannot access the template files anymore (well not at least without also changing the logic of where this templates are or copy the actual templates to your bundle)
Bob van de Vijver
@bobvandevijver
Apr 09 2015 18:25
I now have the difference indeed
However, by using the __DIR__ you can never easily overwrite them
And it is more logical to have the location of the really instantiated class than the location of same extended pparent
Matej Velikonja
@matejvelikonja
Apr 09 2015 18:28
but this means more work when you are extending class
Bob van de Vijver
@bobvandevijver
Apr 09 2015 18:29
Yes, it does indeed
Matej Velikonja
@matejvelikonja
Apr 09 2015 18:30
In my opinion the current behaviour is more appropriate
Bob van de Vijver
@bobvandevijver
Apr 09 2015 18:32
However, then it is impossible to easily overwrite the files accessed with those functions
But, I think other methods might be even better
Because there should be a better way to retrieve the Resources directory without doing $this->getContainer()->get('filesystem'), __DIR__.'/../Resources/skeleton/bundle' or dirname($reflClass->getFileName()).'/../Resources/skeleton/bundle'
@matejvelikonja Did you not post that earlier?
Yeah, you did indeed
Matej Velikonja
@matejvelikonja
Apr 09 2015 18:38
yes, this would be a good solution
ioleo
@ioleo
Apr 09 2015 21:59
thanks for valuable discussion, I'll consider this tomorrow and update the PR
Bob van de Vijver
@bobvandevijver
Apr 09 2015 21:59
:+1: