[Framework-Team] PLIP 178 review

Alec Mitchell apm13 at columbia.edu
Mon Nov 6 03:26:23 UTC 2006


On 11/5/06, 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?

At least for BBB these should be kept (though how do you deprecate
something like this), it's quite likely people are using these classes
for some purpose.

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

+1 we might as well gain some flexibility while revamping things.

Alec




More information about the Framework-Team mailing list