The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
In the CROP entity code, the use of the ‘crop_crop’ effect plugin is hard-coded at several key points in the process. Introducing complexity into the code, it takes the form of :
if(is_crop_crop) {
do something
}
else {
do something, but almost the same
}The main problem this creates is that for retrieving the Crop Type in the ‘getCropFromImageStyleId’ method, only the ‘crop_crop’ plugin is allowed to have a selection of the Crop type, in all other cases the ‘Crop type’ must correspond to the Provider (the module) declaring the effect plugin.
Steps to reproduce
NA
Proposed resolution
It would be preferable to do away with these special cases and use the crop_crop plugin as the basis/template for implementing other CROP-based effect plugins.
Remaining tasks
Documentation :
- Specify that any effect plugin wishing to allow selection of the crop type must store this value in a storage named ‘crop_type’.
User interface changes
NA
API changes
All
Data model changes
NA
| Comment | File | Size | Author |
|---|
Issue fork crop-3464677
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
drdam commentedComment #3
drdam commentedComment #5
drdam commentedComment #6
nikolay shapovalov commentedThanks for your MR, but I believe this issue still need review.
I hide patch file, because MR approach used.
There is 2 issue related on this topic.
And at first sight it looks like methods: getCropFromImageStyle, getCropFromImageStyleId, cropExists, findCrop should be moved away from CropInterface. Maybe we need to make bigger refactor to impove developers expirience.
Comment #7
drdam commentedI totaly agree, but without knowing the history of the implementation, it's not clear how far the refactor can go.