ryanhowdy / fcms

Family Connections - Create a private family website.
http://www.familycms.com
GNU General Public License v2.0
84 stars 41 forks source link

Cropped images #621

Closed GregLeonhardt closed 1 year ago

GregLeonhardt commented 3 years ago

It has bothered me from back in 2013 when I first started using FCMS that all uploaded pictures were cropped. This is mitigated to some extent with the ability to upload unedited imaged but looking at these images is a pain and it has to be done one by one. Anyhow I drilled through the code and found that if i change "up.settings.resize.crop" from full in inc/Upload/PhotoGalery/Plupload.php lines 135 and 147 from true to false, everything looks good. For me this is a bug but in the end I suppose that it's only a suggestion.

GregLeonhardt commented 3 years ago

Make that: inc/Upload/PhotoGalery/Form/Plupload.php

ryanhowdy commented 3 years ago

You mean the thumbnail, right? Because the actual pictures should not be cropped.

GregLeonhardt commented 3 years ago

There are up to three images copied: thumbnail (tb), regular (), and full (full). [I realize that you obviously know this I only included it so my context is clear.] Of these three both thumbnail and regular are cropped. When full is enables it is the only image that is not cropped.

ryanhowdy commented 3 years ago

Thanks for reminding me. I guess I forgot we crop the regular size as well.

In a perfect world, you would never not have full sized photos on. The only reason that is an option is because some people would run out of server disk space way too soon with the size of photos these days. So I think that made more sense to crop the regular when it was assumed that you would have 3 sizes. But it was requested to have full size optional.

It seems like a good opportunity to describe how full sized photos works on the front-end better. And maybe switch them on by default, instead of off.

GregLeonhardt commented 3 years ago

The only reason I turned on full size photos was because all other photos were cropped. I would rather not store the full size photos but the way things are now it's the only way to see the whole image. P.S. Though I wouldn't mind bumping the photo maximum size from 600x600 to something like 1200x1200 for a little better detail in the stored images.

ryanhowdy commented 3 years ago

I know we keep saying cropped, but to be perfectly clear and precise. I think only the thumbnail is cropped. The regular photo is resized to be no wider than 600, but should retain the aspect ratio. So if the photo is 1200 wide by 300 tall, it would get resized to 600x150.

This can be verified here.

I think the 600x600 size was chosen because that is the amount of pixels to show by default in the default theme. So making it larger seemed unnecessary. I'm not sure if maybe we should give more options so the user could decide what size to use as regular or maybe just create a 4th size. Thumbnail, medium, large and full. Where regular becomes medium. Maybe even have them all configurable.

GregLeonhardt commented 3 years ago

Agreed but when the images are uploaded by inc/Upload/PhotoGalery/Form/Plupload.php:

                    BeforeUpload: function(up, file) {
                        if ("thumb" in file) {
                            up.settings.resize         = { width: 150, height: 150, quality: 80, crop: true };
                            up.settings.resize.enabled = true;
                            up.settings.resize.width   = 150;
                            up.settings.resize.height  = 150;
                            up.settings.resize.quality = 90;
                            up.settings.resize.crop    = true;
                            up.settings.file_data_name = "thumb";

                            if ("full" in file) {
                                up.settings.resize.enabled = false;
                                up.settings.file_data_name = "full";
                            }
                        }
                        else {
                            up.settings.resize.enabled = true;
                            up.settings.resize.width   = 600;
                            up.settings.resize.height  = 600;
                            up.settings.resize.crop    = true;     <<<<
                            up.settings.file_data_name = "main";
                        }
                    },

Changing 'true' to 'false' here makes the software behave as you describe.

ryanhowdy commented 3 years ago

Well that is inconsistent.

I'd consider this a bug and needs fixed.

This should be consistent across the different upload types.

thumbnail

main/regular

full