[Framework-Team] PLIP 178 review

Wichert Akkerman wichert at wiggy.net
Sun Nov 5 23:20:35 UTC 2006


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.

-- 
Wichert Akkerman <wichert at wiggy.net>    It is simple to make things.
http://www.wiggy.net/                   It is hard to make things simple.




More information about the Framework-Team mailing list