[TYPO3-core] RFC #13379: Sprite Icon API

Oliver Hader oliver at typo3.org
Fri Feb 19 19:27:24 CET 2010


Hi everybody,

Am 11.02.10 23:58, schrieb Steffen Gebert:
> Am 31.01.2010, 18:03 Uhr, schrieb Thomas Allmer <at at delusionworld.com>:
> 
>> On 31.01.2010 17:37, Steffen Ritter wrote:
>>> Thomas Allmer schrieb:
>>>> Hi!
>>>>
>>>> This is a SVN patch request.
>>>>
>>>> Type: Feature
>>>>
>>>> Bugtracker references:
>>>> http://bugs.typo3.org/view.php?id=13379
>>>>
>>>> Branches: Trunk
>>>>
>>>> Problem:
>>>> Currently the icons are single image files which results in a lot of
>>>> requests and can't be changed by css only.
>>>>
>>>> Solution:
>>>> The attached patch introduces an API for the use of sprites with icons.
> 
> Hi Thomas,
> 
> I overworked your patch and removed most of the CGL glitches.
> 
> What I don't like is this:
>> iconname like 'actions-document-new' || filepath like 'file:<path to
>> file>' || database like 'row:<tablename>' (give row in options)
> 
> Are you sure, we need this? Can't the calling function do this job and
> set the name accordingly?

These are very impressive features but still some more work has to be
done if we want to talk about an API. So, here are my remarks:

Review:
* t3lib_iconWorks
  + getSpriteIcon: I don't know why there is a third parameter $pass
    I think that it would be possible to use the functionality without
    passing the whole stuff in levels recursively
  + getSpriteIconClasses: The mode 'row:' renders record specific stuff,
    however it should be checked whether a $row was passed to that
    method and should throw an exception otherwise
  + getRowIconStatus: I could not determine why the third parameter
    $count is defined here and passed along to other methods - I think,
    that this parameter could be ommitted
* general
  + Passing mixed values (string or array) to a method should not be
    used, since the method has to check everytime what type was used.
    It's better to always use an array and require that type by type
    hints, e.g. "methodName(array $options)"
  + Type hints should also be used if it is sure that an array is
    expected - e.g. for all occurrences

Testing:
Most stuff seems to be working, cool! However, I got an "unknown" icon
for the frontend user tables and frontend user groups. Without that
patch the icons were displayed properly for these tables.

In general I'd like to see an simple API that does not do too much
internal magic. Besides that the input parameters should be self
explaining... I need to think a bit more about the "file:", "row:" etc.
settings and will give my feedback concerning the settings part later on.

olly
-- 
Oliver Hader
TYPO3 Release Manager 4.3


More information about the TYPO3-team-core mailing list