ctessier / nova-advanced-image-field

🌄 📐 A Laravel Nova advanced image field with cropping and resizing using Vue Advanced Cropper and Intervention Image
https://novapackages.com/packages/ctessier/nova-advanced-image-field
MIT License
100 stars 26 forks source link

Added new method for intervention WebP support #108

Closed Ruitjes closed 1 year ago

Ruitjes commented 1 year ago

Hi @ctessier

Hope you can find the time to check my PR and hopefully approve it.

I hope I've implemented it the right way, if not please let me know so I can change it accordingly. Tested it locally worked as intended.

Its currently only limited to use webp and not for all the other possibly encodings see.

Fixes #106.

ctessier commented 1 year ago

Hi @Ruitjes

Thank you for your contribution!

If the need is to be able to convert the output image to WebP, it could totally be another format as well. Therefore, I would rather make a more generic method (convert for instance) to save the image in a given format.

What do you think? We could do something like below, where the output format would also be used as the file extension is specified.

AdvancedImage::make('Photo')->convert('webp'); 

public function convert($format) 
{
    $this->outputFormat = $format;

    return $this;
}

if ($this->outputFormat) {
    $this->image->encode($this->outputFormat);
}

See https://image.intervention.io/v2/api/encode for the available formats.

Ruitjes commented 1 year ago

Thanks for your quick response @ctessier!

I totally agree with your comment, thus I have made the needed changes to support all the encoding formats by Intervention. Please have a look again :).

Ruitjes commented 1 year ago

@ctessier Don't want to come across as impatient, but I was wondering if you could review my pull request sometime :)

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

ctessier commented 1 year ago

Hi @Ruitjes

I am merging after a few adjustments. I am on holidays so I am doing my best to do some final testing and publish a new release.

Thank you so much for your contribution!