On 02/16/2012 02:53 AM, Douglas Cerna wrote:
Gentlemen:
I've implemented most of the functionality Justas and I discussed during the
sprint for the photo work:
* Use of blobs for storing the images (using zope.file's File objects)
* Data converter that accepts only image mime types (JPEG, PNG, GIF and BMP).
If you try to upload a PDF, you should get a validation error. The converter
also resizes the image, using 99px for maximum width or 128 for maximum height.
This is the ratio of the passport photo. Of course, we can modify these values
to store larger images, I just chose those because it's the area of the image
in the person index view
* The person index view adapts the general information table width if there's a
photo, so the photo doesn't overlap the table
* There's a new @@photo view on person objects that returns the image data
setting the appropriate headers, such as content type and length. It also sets
a If-Modified-Since header according to the last time the person object was
modified. This allows the browser to use its cache instead of requesting the
image on every request
* The edit person view allows the user to delete the existing image or upload a
new one
Nice!
You can see it in action:
http://69.164.203.135:6662/persons/douglas (vertical JPEG)
http://69.164.203.135:6662/persons/camila (horizontal PNG)
http://69.164.203.135:6662/persons/teacher001 (GIF)
http://69.164.203.135:6662/persons/teacher002 (BMP)
Very nice :)
The branch is at:
lp:~replaceafill/schooltool/flourish_photos
I'd appreciate your feedback on functionality and code issues.
PhotoView.__call___: # XXX: what to do if photo is None? redirect?
- Maybe throw NotFound?
We should get rid of PhotoDataConverter.getImageInfo, and use
image.size() and image.format instead.
Nitpicking
- do not hide defaults like DESIRED_SIZE inside methods. Make
those class attributes or something.
- PhotoFieldFormAdapter: it's slightly nicer to use
object.__setattr__ instead of self.__dict__['foo'] = bar -- that said, I
failed to do this in many many places.
- image.save(f, image.format.upper(), **kw). Is there a point of
doing .upper() there?
flourish.css: "border-radius: 4px;". Is it worth adding
-moz-border-radius?.. Maybe not anymore.
May be worth considering:
- store all images as PNG.
- convert all images to RGB or RGBA mode. Paletted (GIF) ->
convert to RGB -> pretty scaling with resampling and all.
- do not accept large data. No 1GB image transforms to thumbnails
killing the server.
- do not accept large images. No 10000x10000 px image transforms -
it's a photo, not a map of Washington D.C.
- save image.copy() (after the thumbnail transform) to keep only
raster data. Do we really need app headers like image comments, GPS
coordinates and other waste?
- scale images up, if they're smaller than desired size?
- when editing person info, it would be nice to see the current photo.
Well, that's my two cents.
Nice work!
Justas
_______________________________________________
Mailing list: https://launchpad.net/~schooltool-developers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~schooltool-developers
More help : https://help.launchpad.net/ListHelp
