[Framework-Team] Re: PLIP 178 review

Florian Schulze florian.schulze at gmx.net
Fri Nov 24 16:35:25 UTC 2006


On Mon, 06 Nov 2006 00:20:35 +0100, Wichert Akkerman  
<wichert at wiggy.net> wrote:

> I did a quick review of PLIP 178. This is based purely on the output
> from:
>   svn diff -r 11240:HEAD \
>   https://svn.plone.org/svn/plone/CMFPlone/branches/plip178-back-to-icons-as-images
>
> which gives you a list of all changes from the trunk. I have not tried
> to run the code.
>
> PLIP 178 modifies the icon handling in templates to use img elements
> instead of CSS classes. The icon is determined by calling a new getIcon
> method on IPlone. This will try to adapt the object IContentIcon, a
> simple adapter which returns all icon information. This approach makes
> icon handling very flexible.
>
> I have a few remarks on the implementation, most of which are trivially
> fixe:
>
> * folder_factories hardcodes the icon size; it should use the icon size
>   as provided by the IContentIcon interface
>
> * livesearch_reply does not set the width and height attributes on the
>   img element; it probably should. For AAA reasons it probably should
>   set an (empty?) alt attribute as well.
>
> * I am wondering if there are there possible use-cases for still keeping
>   the contenttype-<type> CSS classes?
>
> * The interface for getIcon describes the return value as a
>   dictionary, which is not true: it is either a dictionary or a
>   IContentIcon instance. It should describe the the mandatory and
>   optional attributes in the returned object, as well as what will
>   be returned if there is no icon.
>
> Finally something to think about: if we're extending things anyway,
> would it be useful to add an option to request the icon in different
> sizes? I can image that at some places in the UI you want to have a
> large icon instead of a small one.
>
> Wichert.
>


This PLIP should be finished now.

Regards,
Florian Schulze





More information about the Framework-Team mailing list