collective / plone.app.imagecropping

Crops Images in Plone manually using cropper JS library
https://pypi.python.org/pypi/plone.app.imagecropping
9 stars 23 forks source link

UI Team Review #9

Closed vangheem closed 8 years ago

vangheem commented 11 years ago

Image cropping is a feature that it would make sense for plone core to support eventually so the UI team supports the efforts!

Here are my thoughts.

Overall, it's a great start! Keep the UI team informed on anymore progress and we'll get this to a point where it's good to be merged into plone core!

frisi commented 11 years ago
Instead of providing a "Cropping" button, can we customize the image
widget to include a link to "crop" the image. Then, it should be able
to handle multi-image content types.

handling multiple image fields on an archetypes and (potentially) dexterity content type is supported since version 0.1-rc1 see https://github.com/collective/plone.app.imagecropping/blob/0.1rc1/src/plone/app/imagecropping/browser/editor.py#L99

this is how the ui looks like on an archetype with multiple image fields multiple-fields

including a link to crop an image in the imagewidget makes sense, too. it would provide users an additional way to access the cropping editor.

we think it's more important to link to the cropping view out of the tinymce image plugin. (we already planning this in https://github.com/collective/plone.app.imagecropping/issues/2)

the reason: if you see your image in use in the text and want it cropped, you'll hit edit on the document, mark the image and open the tiny image dialog.

another possible - and as we think most intuitive - way to access the croppingeditor would be a cropping button displayed over images. eg on images inserted via tiny and also on images added in the page template (eg newsitem_view) the button would open the croppingeditor with the given image and the used scale.

Are we hitting the correct use case here with our scale selection? Since
plone already provides support for image scales, being able to crop,
only to those scales seems prohibiting. 

the way plone.app.imagecropping handles scales is intended to be the plone way. plone already forces editors to use the available image scales integrators/siteadmins have defined via the imaging controlpanel.

The way I imagine people
wanting to use this is for cropping parts of the image out or using a
custom size that they'll reference the original image in a page or news
item (and using a custom size is not possible right now).

to support the custom sizes you're talking about, the plone way would be:

All the image scales are actually the same aspect ratio so they end up
not being very helpful--they're all just different sizes of the same
aspect ratio.

agreed

If we are going to provide a pre-set list of cropping
sizes and ratios, I think we should provide often used ratios here like
4x6, 3x5, etc.

which scales (and therefore ratios) are used is a matter of the project.

on a typical project we add some custom sizes with the scales needed to implement the given design.

maybe it's a good idea to have additional settings to constrain which scales are available in tiny and the cropping editor

This functionality should probably merge with the "Transform" button.

transform operate on the original image (not just it's scales) if one needs the functionality to crop the original image, he/she can install http://plone.org/products/products-imageeditor

I think your UI is mostly fine. It's good to keep something like this as
simple as possible.

I'm not sure what the point of the delete button is. I think that can
probably be removed.

if you save a cropped version of the image for a given scale (eg. preview) and you don't want to see the cropped version any longer all over your portal you can remove the crop.

the next time your-image/image_preview is accessed, the image gets scaled by plone as usual (without any cropping)

The cancel button should probably be offset of the save button so there
is less of a chance for accidental clicks.

good point, we added ticket #10

A little more styling could help spiff up the UI a bit. Once the
functionality gets to where we want it, we should have a designer touch
it up a bit.

ideas very welcome. the ui becomes bloated, especially if you have multiple image fields on one contenttype. we could reduce the display size for images in the field and scale selection. maybe you guys do have other ideas?

it makes sense to hide the field selection (open @@croppingeditor?fieldname=foo) when linking the view from the imagewidget. or even the scale selection too, when linking via the crop-button displayed over an image

Overall, it's a great start! Keep the UI team informed on anymore progress and we'll get this to a point where it's good to be merged into plone core!

we'll keep the ui-team up to date by adding comments to https://github.com/collective/uiteam/issues/2

vangheem commented 11 years ago

Wow, I think I misunderstood one major aspect of this product. You're modifying existing scales, leaving the original in tact. I thought the cropping was being done on the original from the selected scale.

The way you describe the workflow of adding desired scales and how it works makes a lot of sense now; however, we need to figure out a way to make sure that's understood by the user properly and that they know they're modifying the actual scale, not the original.

Good work!

jensens commented 8 years ago

I think this one can be closed after all the work done meanwhile.