This needs #2306683: Misc small fixes applied first.

So... I decided that I was going to use one of these 'focus' modules on an existing site, to make image styles more usable for content editors. And I discovered focal_point was definitely the best fit out there.

Then while I was editing my 8 existing styles I thought

    "Why do I actually need to change all my 'Scale and Crop' effects to 'Focal Point Scale and Crop' effects? Can't there just be a setting for the existing 'Scale and Crop' effect?"

and the next thought was

    "If we have that, there should also be a default setting to enable/disable Focal Point for all 'Scale and Crop' effects, because having to enable this for every individual effect (and having to export that extra setting into features before it even works) is just too tedious."

That's what this patch implements: per-style-per-effect settings, and global-per-style setting defaults. The effects:

  • Enabling this module will have no immediate changing effect on any behavior
  • but changing the crosshair in image widgets will now immediately have effect on derived images, for any styles which have the 'Scale & Crop' effect. No configuration changes necessary.
  • This is not the case for the 'Crop' effect, because that would have immediate changing effect when the module gets enabled. So you'd have to enable that yourself if you wanted. (See code/comments for why, and the resulting complications.)
  • "Focal Point (Scale &) Crop" is exactly equivalent to "(Scale &) Crop" with the focal_point setting enabled, so the Focal Point specific effects could be migrated and deleted in a 'version 2', if we wanted to do that.
  • If you do not use the Focal Point specific effects, and you disable Focal Point, your image styles won't totally break. They'll just keep working, while ignoring the crosshair settings. (Is that an advantage?)
CommentFileSizeAuthor
#1 interdiff-2306809-0-1.txt8.33 KBroderik
#1 focal_point-alter-effects-2306809-1.patch17.4 KBroderik
focal_point-alter-effects.patch14.32 KBroderik

Comments

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
StatusFileSize
new focal_point-alter-effects-2306809-1.patch17.4 KB
new interdiff-2306809-0-1.txt8.33 KB

Well.

Just after uploading the above, I thought "but if we are implementing hook_image_effect_info_alter()... then we should probably assume that someone else could have implemented that hook too!"

That means we don't blindly call the original callbacks, but we check if they're altered - and call those.

Not a problem, but unfortunately it makes the code harder to read.

roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Status: Needs review » Postponed
Related issues: +#2308801: Export less data for image effects (within image styles)

...and then I discovered some really confusing info. Some of my altered image styles were suddenly not taking the Focal Point into account anymore!

After some debugging/thinking, it seems to me that this is a Features bug that should be solved first, before hook_image_effect_info_alter() can be used reliably. (By any module, not only this one.)