[TYPO3-core] RFC: Feature/Bugfix #7427: Add docheaders to Filelist module

[Christoph Koehler]

Benjamin,

Thanks for the early feedback.
Attached is a new patch with comments below your comments.

Benjamin Mack wrote:
> Hey Christoph,
> 
> this one bugged me as well, I'm glad the file list is getting a bit more 
> attention.
> 
> One side note: please add your patch to the initial email for future 
> patch requests.

Attached now, sorry about that!

> I'm glad we have you coding here as well :)
> 
> After I had a quick look over the code, this is what I got:
> 
> 1) stylesheet.css
> 
> +BODY#typo3-file-list-php { margin: 0;}
> +BODY#typo3-file-edit-php { margin: 0;}
> +BODY#typo3-file-rename-php { margin: 0;}
> +BODY#typo3-file-upload-php { margin: 0;}
> 
> Put it to the rest of the BODY statements, and make it like this
> 
> +BODY#typo3-file-list-php,
> +BODY#typo3-file-edit-php,
> +BODY#typo3-file-rename-php,
> +BODY#typo3-file-upload-php { margin: 0;}
>

Changed in new patch.

> 2) typo3/file_edit.php
> 
> Does it hurt to create an extra function getButtons() here as well? 
> Would be good.
> 

I left it as it is because there is only one button to add. file_list 
has so many buttons that a separate method made sense, this one doesn't. 
If you guys insist I will modify it though.

> 3) typo3/file_newfolder.php
> 
> Same as 2), and: don't comment out stuff, either remove it (because you 
> have an extremely good reason) or comment it out and write a comment 
> above why you removed a line.

See above for comment about the addButtons method.
Removed the commented out code, sorry about that, just missed it.

> 4) Removing functions
> I'm not sure how Patrick handled it with his code (haven't had a chance 
> to do an extensive code review yet), but I always tried to keep 
> backwardscompatibility by adding getButtons() and keeping existing 
> functions that make use of getButtons() to do their stuff, setting the 
> old functions to deprecated and using getButtons() in the functions 
> where we need the buttons. Possible? (Patrick?) Maybe I'm too concerned 
> here...
> 

I am holding off on that one for more feedback. I did a global search in 
the code for the method I removed and it wasn't used anywhere else. And 
since it wasn't a public API method but just an internal helper I 
removed it. Like I said, waiting for more feedback.

Christoph
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: filelist-1.1.diff
Url: http://lists.netfielders.de/pipermail/typo3-team-core/attachments/20080209/96503678/attachment-0001.txt