I started working on D8 port of drupal.org/project/imagecrop and was informed about efforts to consolidate the modules. It would be great if maintainer of drupal.org/project/manualcrop could check the original issue and provide feedback.

#2368337: Drupal 8 port (duplicate)

Comments

slashrsm’s picture

slashrsm commented

Let's continue the discussion here. I checked http://drupal.org/project/manualcrop, http://drupal.org/project/imagecrop and http://drupal.org/project/focal_point. It looks like the first two take more or less the same approach. Main difference IMO is in UI, which IMO looks much nicer in manualcrop. focal_point takes a bit different approach when it comes to storage of crop information. Instead of defining a rectangle it defines a single point of interest.

I thought about this and based on what I've seen and what I've already written in #2368337: Drupal 8 port (duplicate) I'm proposing the following approach:

- Implement crop information as a full-featured entity. It comes with many benefits: we don't need to define our own storage model, it is very extensible (people can add fields to it), it supports revisions, it is storage-swappable, ...
- Let's store crop information as WxH and center point (X,Y) of the crop area (both manualcrop and imagecrop currently use top-left corner for x,y). With this storage model we're able to implement approach that focal_point takes too. Same storage structure for two different approaches. Consolidation and re-use of code. Yay!
- We implement same image effects as we currently have in manualcrop and imagecrop (they basically overlap already). Effects that handle "point of interest" approach can live in a separate module that would depend on base module.
- Integration with storage components (file_entity, media_entity, ...). Instead of tying crop information to fid (assumes file entity) we tie it to entity_type/entity_id pair. This way we're able to have crops for any entity type, which gives us a lot of flexibility and allows us to clearly implement integration with both storage components that we have ATM (+ any other thing that might appear in the future). We would then implement a plugin for each storage component that would be responsible for providing URI of image that needs to be cropped. This way we abstract storage from UI as UI doesn't need to know which way the image is stored.
- If we separate storage and basic API from UI we allow people to have different UIs and still rely on same code for other things. The only reason we have so many cropping modules is need for diff UIs. It is much easier to only implement a UI part than implementing the entire stack over and over again. It results in redundant code and is waste of resources that we don't have. Default UI could be what manualcrop offers today. It could live as a sub-module in the same project. We could also decouple base API in a separate project which manualcrop (and other modules) would depend on.

The entire media ecosystem in D8 is taking decoupled approach. It also tries to existing tools as much as possible. What I'm proposing here follows both patterns (decoupling into focused components and using Entity API). This approach will prevent redundant code and make the entire ecosystem easier to maintain.

Thoughts?

ianthomas_uk’s picture

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
commented

Sounds like a good, well thought out plan. I'd probably go with a totally separate project for the shared code, so all UI modules would be equal.

cthshabel’s picture

cthshabel commented

Posted this on the original issue you all started, but it's more fitting here.

After using both of the modules extensively, I can say manual crop is superior, with very few exceptions.

Pros of Manual Crop:

- S3FS compatible (stream wrappers work well with cloud architecture)
- UI cleaner in general
- Media Desk compatible (multi image upload sites benefit from this)
- Plupload compatible (client side resize)
- Mobile Ready (OR VERY CLOSE):
- imgareaselect.js plugin (newest version at least supports mobile touch devices now!)
- Image Cache Autorotate (still not working, but if image javascript can get this working, manual crop should?)

Cons of Manual Crop:

- Image Cache Autorotate not compatible (makes the last item on Pros irrelevant for now)

Matthijs, manual crop is absolutely amazing. If we were able to get autorotate with this module, it would truly be the ultimate solution for the majority of people I think.

matthijs’s picture

matthijs
Primary language Dutch
commented

Thanks for your time and effort on this slashrsm! I'm still thinking this through, but so far I don't have any remarks on your ideas (yay).

Unlike ianthomas_uk I don't think it would be a good idea to split the shared code and UI in two separate Drupal.org projects. This will only complicate things and make the module less user friendly for new users. I'd just add a separate UI module to the project, which can disabled and replaced (or extended) at any time...

slashrsm’s picture

slashrsm commented

I am glad we agree on that. :)

WRT the way we structure modules/dependencies I'm slightly inclined towards separate projects. Main reason for that is clear separation of responsibilities. It also follows the approach that we're taking for entire Media ecosystem. See https://groups.drupal.org/node/418803 for more details.

Fragmentation of modules won't be real UX problem if we do it right. We need to communicate full-featured modules as the way to go (manualcrop in this case). If people install those with Drush all dependencies get installed automatically. So yes, it can be a bit problematic if people go and do that manually. But I also believe clear separation of responsibilities ensures cleaner architecture, which is IMO an investment for the future.

ianthomas_uk’s picture

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
commented

To me the main reason to have separate projects is to encourage other modules to make use of this shared library. If you were the maintainer of focal_point, for example, would you be prepared to depend on manual crop? Would you feel that the concerns of your module would be addressed by the maintainer of manual crop if they weren't relevant for the manual crop UI?

There is a small cost in the complexity of installation, but I think if you're capable of installing one module, you should be capable of installing two.

slashrsm’s picture

slashrsm commented

@ianthomas_uk: Good point. 100% agree.

1kenthomas’s picture

1kenthomas commented

FWIW, perhaps redundant: are separate UIs really a sufficient reason for separate modules in this case (as opposed to some settings)? If so, how about something like an API module and UI modules?

UPDATE: && what Ian said :)

slashrsm’s picture

slashrsm commented

I created new project for Crop API (https://www.drupal.org/project/crop) with a repo on Drupal media GitHub (http://github.com/drupal-media/crop). Media initiative uses GitHub for all development and d.o. issues for management of projects. We sync all repos to d.o. to make it easier for people to test. I've also set up sync job.

I created few initial issues for initial implementation. Feel free to grab any of those: https://www.drupal.org/project/issues/crop.

I really want to make this open and accessible to everyone. I will give commit access to anyone interested (specially maintainers of D7 modules). If you want commit access to Crop API just ping me and we'll make it happen.

danielnolde’s picture

danielnolde commented

Hey guys,

what's the current status on this?
Has any usable results been achieved yet in porting Manual Crop to Drupal 8?
There's not even a 8.x branch yet, so i assume there are no official results?
Maybe some sandbox or github experiment?

If you would need to build a rather D8 site within days that would need image cropping (even different crops for different image styles), what would you do? What choices are there currently?

Cheers,

Daniel

slashrsm’s picture

slashrsm commented

@danielnolde: See #1 and #9. Crop API should be ready to act as a storage component for image crops in D8. Now we're more or less at the point where we need to port UI part.

woprrr’s picture

woprrr commented

I do not understand the mechanics to trigger a crop is upload an image. I have managed to put in place the types of crop and add pictures to my style but I think that is one thing I do not understand this link is not there. If there are no I like to know if you have tracks on which was to decide because I would love to help you for this module !

I followed the decisions and I still do not understand really how we link the image and the crop? Via the Picture Style that does seem to work for the moment we can well apply the effect (The application of the effect goes true but $ this-> getCrop ($ image) is never satisfied in \ Drupal \ crop \ Plugin \ ImageEffect \ CropEffect :: applyEffect).

If my analysis is correct it will be \ Drupal \ Crop \ Plugin \ EntityProvider \ File which will be responsible directly to the crop or upload \ Drupal \ crop_media_entity \ Plugin \ EntityProvider \ MediaEntity to be a provider for crop entity?

the library will use http://odyniec.net/projects/imgareaselect/ well ??

Thanks for this good job :)

Anonymous’s picture

Anonymous (not verified) commented

Hi All

What is the status on the Drupal 8 Port?

Thanks for the Great Module!

slashrsm’s picture

slashrsm commented

There is Crop API, which handles storage and Image widget crop, which is first UI on top of it. Hopefully we'll see more UI modules adopting Crop API soon.

balsama’s picture

balsama
he/him
Primary language English
Location boston
commented

Looks like u/devin-carlson has a working version of this in his sandbox:
drupal.org/sandbox/DevinCarlson/2572651

Can we get that code moved here as an alpha perhaps?

slashrsm’s picture

slashrsm commented

+1 That would be great!

dodozhang21’s picture

dodozhang21 commented

It's been over a year. Is there any updates on the d8 port of this module?

woprrr’s picture

woprrr commented

In d8 the standard adopted by ligthning and mire stable are image_widget_crop to make manual_crop. To make manual crop focal point based we can use focal_point module too. The Both modules are based on crop api