🇫🇷France @DrDam

Ok, nice catch.

But the point I doesn't understood is how you generate derivatives from the images in the external DAM ?
The derivativ exist in the DAM ? ou it be generated "on the flow" when delivered to client ?

Reroll #10

@norman.lol :

In which class do you propose adding this method?

Nice catch

Thanks

the unlink in createContextualizedDerivativePath() are present to prevent non replacement of derivativ in 1.x versions.

Nice catch

drdam → created an issue.

It seems it was corrected in ImageWidgetCrop 3.x

On a fresh drupal install
- I have create 2 crop type
- On a unique image style, I add 2 effect "manual crop" (one for each crop)
- On the article mange display interface, I choose the image style which use the both crop
- On the article manage form display, I choose "ImageWidget Crop" widget. It only propose the "last crop type" for selection.

see 3259571-IWC.png & 3259571-imageStyles.png

I propose to close this issue

Thanks for this catch !

I'm just realize that it's not fully compatible with CK5 ...

- I am using patches so I'm not sure why it didn't patch, but explicitly decorating a service or altering a route is more reliable.

I don't want altering any route, native controller work fine, and I need it continue making is job has he do. I juste want limit code duplication.

If you prefer not using this patch, the last release without it is the 2.1.4

That's not the goal ... I don't want alter it, I want be able to extends (herite from it) without duplicate 40% of it's code...

see #2685905 ✨ Refactor ImageStyleDownloadController so derivatives can be generated by contrib modules Needs work for more informations

I just add the interface, but I have a doubt about making all methods public ... if some one can check

Did you check if this patch have been applied ?

"drupal/core": {
                "Refactor ImageStyleDownloadController": "https://git.drupalcode.org/project/drupal/-/merge_requests/10766.patch"
            }

This patch change things in core ImageStyleDownloadController in order to extract some technical elements, used in the ContextualImageStyleDownloadController::process method

did you check bout media_contextual_crop collection, and more precisly focal_point adapter module → ?

@nikolay shapovalov : sorry for that, after nearly 2 month, I have presume it be ok for you ...

@nikolay shapovalov :
"Do you think we can add interface for these new methods?"

I don't know ...
The purpose of the RM is simply to separate the processing/generation of the derivatives from the various validation/authorisation operations linked to this generation.

@oily : I don't know either what @deviantintegral want to say, when it create the issue.
I just done my best when @smustgrave ask me to update summary to the "good template"
I have my use case "with mediaContextualCrop, we want to create a new image delivering methodebased on 80% of the ImageStyleDownloadController" for the others I don't know their motivations.

With the good patch, it works better

Reroll patch for 10.0.x

Can you check the MR, if changing parameter type is enought

drdam → made their first commit to this issue’s fork.

The "Null" label is created by the "->label()" method because at this stage (node creation) the "title" ( targeted by the method) is null. I.E. there is no "saved title" for the current entity.

fixed in 1.0.0@alpha2

fixed in 1.0.0@alpha2

drdam → made their first commit to this issue’s fork.

Create MR for 11.x and reroll #16 on it

drdam → made their first commit to this issue’s fork.

+1 for closing

Released in 2.1.4

For test coverage, I don't what it need to be tested, it just code management, no new functionnality.

I have rework the "association loop" for trying to manage mixte between not-upscaling / upsaling.

The association between size and imageStyle are "not optimal" because of #10 and #11, but it doesn't really matter because the derivative will be the same, just not in the "best folder" on the disk

Exemple as a native src-set image :

The "tiniest" size ( 100px) better fit for the thumbnail image style are associate with the "wide" image style because it's the "last" of the list.

The problem is that the imageStyle stack does not provide a simple way of finding the ‘expected size of the derivative if the source image is large enough not to be upscaled’ for an image style.
In fact, it is not possible to ‘classify’ image styles by this ‘expected size’ and therefore to find the ‘smallest’ image style that is most suitable.

The anomaly is present in native ResponsiveImages too.

In fact, this case is not natively covered by nativ responsiveImage because of the possible mixe between "allow/no-allow" upscaling.

But if you editing the media, you change it on all its usage

OK for RTBC

drdam → changed the visibility of the branch 3496234-better-handling-of to active.

@deepali sardana : thanks for the merge Request, but I'll close it, because there is a lighter way to acheive that.

Merge Request available

drdam → changed the visibility of the branch 2685905-refactor-imagestyledownloadcontroller-so to hidden.

drdam → changed the visibility of the branch 3228376-cant-delete-a to hidden.

grimreaper → credited drdam → .

It's the case

Can you make the MR for the 2.x branch ?

the 1.x branch are minimaly maintained now

thanks

released in 1.0.0-alpha1 →

grimreaper → credited drdam → .

RTBC

diag : the point here is the same as many (many) other Crop/Image anomalies, Drupal Core don't know how handle multiple "version" of the same derivative ( I.E. couple ImageStyle x ImageFile ).

There is no real solution base of maintening the current implementation of the "focal point preview" feature.

2 solutions are possibles :
- Simulate the rendering (by CSS/html style manipulation) instead using the full derivative generation
- Add some "context" to the derivativ generation (I.E. hack some core process in order add this feature)

may check https://www.drupal.org/project/image_widget_crop/issues/2865396 →

MR created

drdam → made their first commit to this issue’s fork.

drdam → changed the visibility of the branch 3214798-crop-effect-can to hidden.

drdam → made their first commit to this issue’s fork.

You need to install the 2.3 and make the db update first, and after update to 2.4.

If it's the case, maybe a better "message/communication my be better"

When using media, you need some configurations :

1. in the "host content type" (node)
   • in the form mode : media library as widget
   • in the view mode : display media with a specific render mode "foo"
2. in the "media image"
   • in the form mode : the image widget crop widget
   • in the view mode "foo" : render image with a specific image style using a crop

Did you check about media_contextual_crop → ?

This module provide a way to wire crop and medias

Waiting completion of #3293782

drdam → made their first commit to this issue’s fork.

I have split the doTestFileUriAlter method in order to have "clean" code separation

Merge in 3.0.2

Fix reviews & add test

I think this two issues (phpstan & phpcs) may be merge BEFORE other merge resquests

Ok for me

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.

I totaly agree, but without knowing the history of the implementation, it's not clear how far the refactor can go.

The question is whether the current behavior is “intentional” or a case that was not anticipated.

Thanks @hartsak, I pass the ticket to RTBC

We are hosting on Pantheon which sits behind the Fastly CDN. Sounds like disabling flush_derivative_images is recommended with this setup?

that's what I would recommend. The "derivative cache" are supported by the CDN, so no need to "force drupal" to handle it.

I've read this a couple of times and I think that you are missing the word "no" in the above sentence, that "... there is no way to detect...". Can you clarify ?

You are right, I fix my message. There is no way

Fix in release 2.0.7

I have create a second merge-request for 2.x branch