Adding a new crop to existing role causes issues

[IllyaMoskvin]

Description

I'm attempting to add a new crop to an existing image role.

https://twill.io/docs/#models

This is causing an error on the frontend in the CMS.

When I try to edit an existing media and select the new crop in the crop modal, I get an undefined error here:

twill/frontend/js/components/Cropper.vue

Line 157 in 377e5e0

this.currentRatioName = this.crop.name

Steps to reproduce

1. Add a media definition like this to some model:

   public $mediasParams = [
       'hero' => [
           'default' => [
               [
                   'name' => 'default',
                   'ratio' => 16 / 9,
               ],
           ],
       ],
   ];
   

2. Create a new instance of that model via the CMS, fill in the media, set a crop, and save it.
3. Add a new crop:

   public $mediasParams = [
       'hero' => [
           'default' => [
               [
                   'name' => 'default',
                   'ratio' => 16 / 9,
               ],
           ],
           'mobile' => [
               [
                   'name' => 'mobile',
                   'ratio' => 9 / 16,
               ],
           ],
       ],
   ];
   

4. Open that media in the CMS for cropping, click on the new crop.
5. You'll see the following error in the console:

   TypeError: Cannot read properties of undefined (reading 'name')
   

This is happening because this.crop resolves to undefined.

Expected result

Adding a new crop to an existing role does not cause an error.

Actual result

As described above.

Versions

Twill: 2.4.0
Laravel: 6.20.34
PHP: 7.2
DB: Postgres 13

[pboivin]

Hi @IllyaMoskvin, I can confirm this issue but it's a bit different on my end (Twill 2.5.2)

Following your steps, I do not get the error in the console. The medias field and the cropper display correctly, but I don't see my new crop in the cropper options, only the first crop that was available when the image was saved.

Thanks for reporting!

[IllyaMoskvin]

Thanks @pboivin! As a temporary work-around, we are "deleting" the selected media from the field, and re-selecting it from the media library. This allows us to then select the new crop.

[ifox]

Hi guys,

The inability of Twill to pick up changes to the crops configuration, forcing users to remove the associated image to allow a new set of crops, is a known issue that we should prioritize. It can be a critical issue if a crop position has been manually tailored by an editor.

I am intrigued by the difference of behaviors. What @pboivin experiences is what I'm used to, not the error in console. We'll investigate.

Thanks for reporting @IllyaMoskvin!

[mattrabe]

I am also seeing the error in the console, and can confirm that this is related to changing the list of mediaParams after saving a crop. e.g. if you have "set A" of mediaParams, save crops on an image, then change to "set B" of mediaParams and attempt to crop that same image, the error occurs. In my case, TypeError: Cannot read properties of undefined (reading 'name') plus the fact that the crop window will not match the ratio spec'd in your mediaParams.

I can also confirm that if you delete the image with saved crops, and reupload the same image, the cropping feature will work again as spec'd.

It would appear, then, that crop information is likely stored to the database in a rigid manner, wherein if the set of mediaParams changes later, things break.

Agree with this being a high priority fix needed.

[pboivin]

Hi @mattrabe, have a look at the 2 PRs linked above: #1258 and #1289. This will be released soon with Twill 2.6 but is already available in the 2.x branch if you want to test it out.

[mattrabe]

thanks @pboivin!