search

Raptus / raptus.article.core

Provides a configurable article content type (replaces the default Page content type).
0 stars 4 forks source link

add link to cropping editor to manageable views #6

Closed frisi closed 10 years ago

frisi commented 10 years ago

this pull request adds a cropping action to the list of manageable items if a component adds a crop key to all items in the list.

this is how it looks like: cropping

example usage: https://github.com/webmeisterei/raptus.article.imageslider/commit/42239479d232a7c0dc35e56d96ddd089050a875f

i first wanted to add these links in raptus.article.core.manageable directly (so components need not add the crop key) but ran into a problem basically this is what i tried to do:

try:
    import plone.app.imagecropping
    CROPPING_AVAILABLE = True
except ImportError:
    CROPPING_AVAILABLE = False

from raptus.article.core.interfaces import IComponents   
component = getAdapter(self.context, interfaces.IComponent, name='imageslider.teaser')

#add a crop key to the item dict if it's an image, cropping is available and we're using a scale
#pseudo-code
'crop': IImage.providedBy(obj) and modify and CROPPING_AVAILABLE and safe_hasattr(component, 'scale') and '%s/@@croppingeditor?scalename=%s' % (brain.getURL(), component.scale)

the scale property however, is available on the viewlet, and not the component. (since it's configurable via a property) https://github.com/webmeisterei/raptus.article.imageslider/blob/master/raptus/article/imageslider/browser/slider.py#L59

frisi commented 10 years ago

@skaeser before merging i'd of course add a upgrade step for the javascript registration. before that i'd like to hear your general feedback and whether you tend to add the crop key in manageble or in components that provide manageble lists of croppable images

frisi commented 10 years ago

@skaeser @wag i now added the the upgrade step for the javascript registration.

i also integrated the getScale method in https://github.com/webmeisterei/raptus.article.teaser/commit/a7d0150e3e6113e514ac2a2157639a50c5a48e5c to be able to decide whether to show the cropping link or not.

example usage in raptus.article.listings see: https://github.com/webmeisterei/raptus.article.listings/commit/ec33867461b1fcf6552d2dcabc7abaedd60ff5a0

skaeser commented 10 years ago

Looks good to me. For future additions of features to manageable items we should probably consider a plugin architecture (named adapter lookups) but this is ok for now.

skaeser commented 10 years ago

Are the changes in webmeisterei/raptus.article.teaser@a7d0150e3e6113e514ac2a2157639a50c5a48e5c required for all this to work? if so, could you issue pull request for those too?

Thanks for your work!

frisi commented 10 years ago

thanks for merging @skaeser! of course, the changes in r.a.teaser are necessary as well. there was already an older pull request i added my more recent changes to: https://github.com/Raptus/raptus.article.teaser/pull/3

just noticed that the changes in javascript that require a slightly different structure for the component (https://github.com/Raptus/raptus.article.core/commit/17d17ad9f27e7f80c487015d429d51bf0ab8f795) will require adaptions in raptus.article.listings as well to make the images refresh after defining a crop. i'll submit a pull for these in a few weeks when i'll be updating the project where i'm using this component.