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

[Thomas Allmer]

On 19.02.2010 19:27, Oliver Hader wrote:
> 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

see answer for bennis post.

>    + 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

good point will add that

>    + 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

it's used to define how many overlays you want to use as we now have a 
priority list and only want to show x (=$count) overlays with the 
highest status. [by default it's one and that's what jens wanted]

> * 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

I thought it's just nicer to use...
t3lib_iconWorks('apps-...', 'My power app');
is nicer to write than
t3lib_iconWorks('apps-...', array('title' => 'My power app') );

however can of course be changed easily

> 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.

"frontend user tables and frontend user groups" currently don't have an 
icon with the new naming scheme... we have to add them and regenerate 
the sprites

one small thing I just found is that if you for example a shortcut and 
you set "hide in menu" it get's a wrong icon... already have a solution 
and will post a patch tomorrow

> 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.

I wanted ONE wrapper function for all icon stuff so it needs "a little 
magic"...

you can of course make different functions something like
simpleIconWithOverlay($name, $overlay)
simpleIcon($name)
simpleIconOverlayRow($table, $row)

it's a little late now; but if you need more info on that I can write 
some more tomorrow...

-- 
+---------------------------------+-----------------------------------+
| Thomas Allmer                   |   http://www.delusionworld.com    |
| E-mail: at at delusionworld.com    |   phone: +43 699 16217064         |
+---------------------------------+-----------------------------------+