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 <[email protected]> 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 : [email protected] > Unsubscribe : https://launchpad.net/~schooltool-developers > More help : https://help.launchpad.net/ListHelp _______________________________________________ Mailing list: https://launchpad.net/~schooltool-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~schooltool-developers More help : https://help.launchpad.net/ListHelp
