Elderjs / plugins

Elder.js plugins and community plugins.
https://elderguide.com/tech/elderjs/
MIT License
80 stars 20 forks source link

Plugin: Images - maxWidth not being respected in largest() function #138

Open TotalLag opened 2 years ago

TotalLag commented 2 years ago

I have the following widths configured:

widths: [1800, 1200, 900, 600, 350]

On an image with original width 1500 and using the following code for src: helpers.images.largest('/images/photo-2.jpg',{maxWidth:400}).relative

Expected: return value of photo-2-ejs-350.jpg

Actual: return value of photo-2-ejs-1200.jpg

I have deleted my manifest and regenerated to no avail.

dmitrysmagin commented 2 years ago

Here's the fix for the issue:

--- a/node_modules/@elderjs/plugin-images/utils/helpers.js
+++ b/node_modules/@elderjs/plugin-images/utils/helpers.js
@@ -58,6 +58,7 @@ function getLargest(fileSizes, orgFormat, maxWidth) {
     .filter((p) => p.format === orgFormat)
     .find((p, i, arr) => {
       let largest = true;
+      if (p.width > maxWidth) return false;
       arr.forEach((a) => {
         if (a.width > maxWidth) return;
         if (a.width > p.width) largest = false;
TotalLag commented 2 years ago

TY this is great. I'll test this out and if a PR doesn't already exist, can create one.

EDIT: Looks like you already did! 😃

nickreese commented 2 years ago

This will be picked up on the typescript branch release. I'm trying to release beta 1.8.0-beta.3 for Elder.js and betas for all of the plugins tonight.

We've got ElderGuide.com migrated over, we'll get FindEnergy.com and the others moved then release 1.8.0 and a migration guide.

Included here: https://github.com/Elderjs/plugins/commit/52354e21194cb5e8595714c355f49f0054699687