[TYPO3-core] RFC: #9591: Integrate sys_actions into toolbar
Oliver Hader
oliver at typo3.org
Mon Oct 20 16:43:21 CEST 2008
Hi Ingo,
Ingo Renner schrieb:
> Ingo Renner wrote:
>> Hi Steffen,
>>
>> as dicussed via PM here's a cleaned up version, this also represents a
>> +1 on reading and testing
>
> ok, Steffen and I found a small JS issue with window/load
> document/dom:loaded events. This caused some misbehaviour in Safari....
> solved now.
Here's my review:
Line 10: please use single quotes
Line 71: should be protected instead of private
Line 87: missing @return
Line 88: missing type for TYPO3backend
Line 102: isn't "if ($actionEntries)" enough?
Line 107: I could not find anything about concatenation over multiple
lines - but I think the "." should be at the end of each line
Line 123: would it be possible to use a string instead of an array?
Line 162: should be "is_resource($queryResource)"
Line 174: after the while the TYPO3_DB->sql_free_result(...) is missing
Line 218: what is this method for if it always returns true?
Line 265ff: a dimension of "0px" can be written "0" in CSS without a unit
Line 331: Please use "SysactionMenu" instead of "sysactionMenu"
Line 398: Hm... is it possible to have more instances of this toolbar
item? But I'm fine with it - just a question...
olly
--
Oliver Hader
TYPO3 4.3 Release Manager
More information about the TYPO3-team-core
mailing list