[TYPO3-core] RFC: #16228: [Feature] TCA tree

[Steffen Kamper]

Hi Ernesto,

Ernesto Baschny [cron IT] schrieb:
> Hi Steffen,
> 
> 
> sorry, I reviewed v1 yet.. I've just seen your v2 on the list, too quick
> for my review... anyway:
>

np - thanks for the review!

> A)
> I fixed some CGL issues in v2 and also some documentation and other
> minor issues. For your convenience I attached a patch documenting the
> changes between your "v1" and my "v2" patch.
>

thx. v2 is only some js change, so easy to merge.

> B)
> Another note would be:
> t3lib/tree/renderer/class.t3lib_tree_renderer_extjs_json.php
> 
> should be called:
> t3lib/tree/renderer/class.t3lib_tree_renderer_extjsjson.php
> 
> and thus the class:
> t3lib_tree_Renderer_ExtJsJson
> 
> Else you would need another directory (because of the "_").
> 

ok, i hadn't in mind the directory logic behind.

> C)
> Question, why is t3lib/js/extjs/tree/tree.js a mix up of "generic" tree
> stuff and "Tca" stuff? Couldn't that be split up? Is tree.js the start
> of the ExtJS part which will later on also handle our "page tree" etc?
>

i will improve this in the next time also with more trees to come.
Only the toolbar and the changecheck handler are TCA related, the rest 
is general and can be used without TCA. I thought of splitting into more 
files, performance is better with one if not concatenated.

> D)
> Lots of methods are still not phpDocumented, if you can, consider adding
> those too. Also many are only phpdoc headers without any explanation of
> parameters and method functionality.
> 

yes, have to be done.

> 
> Other than that nitpicks: +1 by reading and testing
> 
>

great!


> I would split up the commits in these changesets, which would ease later
> reviewing of the changes. Instead of a "monster commit":
> 
> 1) fixed sprite generator (why is this needed?)
> M      t3lib/spritemanager/class.t3lib_spritemanager_spritegenerator.php
> 
> 2) fixed abstract tree classes:
> M      t3lib/tree/renderer/class.t3lib_tree_renderer_abstract.php
> M      t3lib/tree/renderer/class.t3lib_tree_renderer_unorderedlist.php
> M      t3lib/tree/extdirect/class.t3lib_tree_extdirect_abstractextjstree.php
> 
> 3) register new classes
> M      t3lib/core_autoload.php
> 
> 4) added a new ExtJS renderer for trees:
> A      t3lib/tree/renderer/class.t3lib_tree_renderer_extjs_json.php
> A      t3lib/js/extjs/tree/tree.js
> 
> 5) implemented TCA rendering in TCEforms "renderMode=tree":
> M      t3lib/class.t3lib_tceforms.php
> A      t3lib/tceforms/class.t3lib_tceforms_tree.php
> A      t3lib/tree/tca/class.t3lib_tree_tca_extjsarrayrenderer.php
> A      t3lib/tree/tca/class.t3lib_tree_tca_tcatree.php
> A      t3lib/tree/tca/class.t3lib_tree_tca_databasetreedataprovider.php
> A      t3lib/tree/tca/class.t3lib_tree_tca_databasenode.php
> A      t3lib/tree/tca/class.t3lib_tree_tca_dataproviderfactory.php
> A      t3lib/tree/tca/class.t3lib_tree_tca_abstracttcatreedataprovider.php
> 
> 6) sample styling for the new ExtJS trees
> A      typo3/sysext/t3skin/extjs/images/tree/expand-all.gif
> A      typo3/sysext/t3skin/extjs/images/tree/collapse-all.gif
> M      typo3/sysext/t3skin/extjs/xtheme-t3skin.css
> M      typo3/sysext/t3skin/stylesheets/ie6/z_t3-icons-gifSprites.css
> M      typo3/sysext/t3skin/stylesheets/sprites/t3skin.css
> 

good suggestion, will do this way.

vg Steffen