schooltool-developers team mailing list archive

[Tom Hoffman <tom.hoffman@xxxxxxxxx>]

Nice work Douglas and I agree with Justas's suggestions.  Seems like
just converting everything to PNG might avert strange bugs down the
road.  We can recommend PNG's.

--Tom

On Thu, Feb 16, 2012 at 6:38 AM, Justas <justas@xxxxxx> wrote:
> 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     : schooltool-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~schooltool-developers
> More help   : https://help.launchpad.net/ListHelp