Problem/Motivation

The two effects share some code. OO allows us to refactor code in a way that removes that.

Proposed resolution

- Make FocalPointScaleAndCropImageEffect extend FocalPointCropImageEffect and rely on the latter to do the cropping part.
- Move functions from base image effect class to that image effect where code is needed.
- Since both effects now share common ground we don't need base class any more.

CommentFileSizeAuthor
#2 2657434_1.patch17.04 KBslashrsm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

slashrsm created an issue. See original summary.

slashrsm’s picture

slashrsm CreditAttribution: slashrsm at Examiner.com commented
Status: Active » Needs review
FileSize
2657434_1.patch17.04 KB

This will conflict with #2626950: Depend on Crop API for storage, but shouldn't be to hard to resolve. I can also merge patches.

marcingy’s picture

marcingy CreditAttribution: marcingy at Examiner.com commented
Status: Needs review » Reviewed & tested by the community

Looks good

bleen’s picture

bleen CreditAttribution: bleen at NBCUniversal commented

Lets wait until #2626950: Depend on Crop API for storage lands before we move forward here

slashrsm’s picture

slashrsm CreditAttribution: slashrsm at MD Systems GmbH commented
Status: Reviewed & tested by the community » Closed (won't fix)

It seems that this is not relevant any more. Thanks!