Automattic / regenerate-thumbnails

WordPress plugin for regenerating thumbnails of uploaded images. Over 1 million active users and counting.
https://alex.blog/wordpress-plugins/regenerate-thumbnails/
GNU General Public License v2.0
133 stars 54 forks source link

Added hook when plugin initialises post id. #133

Closed clementduncan closed 1 year ago

clementduncan commented 2 years ago

Added hook 'regenerate_thumbnails_regenerator' to allow external functions. Included .DS_Store in .gitigore for macOS developers. Updated node-sass package to support Node.js 16.

aaronfc commented 1 year ago

Hey, I am missing a bit of context: what's your use case?

The get_instace method doesn't look like a proper place to have any external functions run. Maybe it would make more sense to have the hook in the RegenerateThumbnails_Regenerator::regenerate method?

I know it's an old PR. If you don't need this anymore let me know and I will close it.

clementduncan commented 1 year ago

Hi, my use case is that I have attachment images that have custom metadata (selectable options in 'media edit') that can change the crop position of the image. This is processed on image update. However when regenerate thumbnails is used, this custom function is ignored.

The hook added is to allow devs to inject any custom function when using regenerate thumbnails.

It does make more sense to add the hook in RegenerateThumbnails_Regenerator::regenerate.

aaronfc commented 1 year ago

Looking better!

Still not completely sure I understand your use case. Is it enough to receive in the action the attachment ID for your process + regenerate_thumbnails to work together?

Just to confirm I understood:

Is that right? What I don't understand yet is why you need to apply the modifications every time that the thumbnails regeneration is run.

clementduncan commented 1 year ago

The action only receiving the attachment ID is enough for this use case. Perhaps passing the full attachment object would be more suitable for other use cases?

Yes to all three bullet points. I'll go into a bit more detail about the implementation to clarify the use case and why I run it on every regenerate.

The action allows image regenerate to process this logic again, otherwise the default crop position would be used.

clementduncan commented 1 year ago

Changes made as requested.

Thank you for working through this merge with me.