Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fileupload thumbnail preview #985

Merged
merged 2 commits into from
Oct 10, 2023
Merged

Conversation

AIC-BV
Copy link
Contributor

@AIC-BV AIC-BV commented Oct 10, 2023

No description provided.

@AIC-BV AIC-BV changed the title Make sure image is not stretched Prevent image from being stretched Oct 10, 2023
@LukeTowers
Copy link
Member

@AIC-BV can you post a before / after screenshot so I can visualize the change and why it was required?

@LukeTowers LukeTowers added needs response Issues/PRs where a maintainer is awaiting a response from the submitter maintenance PRs that fix bugs, are translation changes or make only minor changes labels Oct 10, 2023
@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 10, 2023

@LukeTowers

Before:
image
image

After:
image
image

@LukeTowers LukeTowers added Status: Completed and removed needs response Issues/PRs where a maintainer is awaiting a response from the submitter labels Oct 10, 2023
@LukeTowers LukeTowers changed the title Prevent image from being stretched Fix fileupload thumbnail preview Oct 10, 2023
@LukeTowers LukeTowers merged commit 751545f into wintercms:develop Oct 10, 2023
10 checks passed
@LukeTowers LukeTowers added this to the v1.2.4 milestone Oct 10, 2023
@WebVPF
Copy link
Contributor

WebVPF commented Oct 10, 2023

@LukeTowers @bennothommo
This is the wrong fix. Preview works correctly. The default mode is auto, which makes the thumbnail in portrait or landscape orientation.
Cancel this change.

@AIC-BV If you need thumbnails of the same size, you need to set the field settings mode to exact or crop .
https://wintercms.com/docs/v1.2/docs/services/image-resizing#available-parameters

@LukeTowers
Copy link
Member

@AIC-BV can you post the configuration of the field where you were running into the issue?

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 11, 2023

@AIC-BV can you post the configuration of the field where you were running into the issue?

photos:
            context: ['create', 'update']
            tab: aic.account::lang.general.photos
            label: Foto's
            type: fileupload
            mode: image
            useCaption: false
            thumbOptions:
                mode: crop
                offset:
                    - 0
                    - 0
                quality: 90
                sharpen: 0
                interlace: false
                extension: auto
            span: auto

@WebVPF
Copy link
Contributor

WebVPF commented Oct 11, 2023

@AIC-BV if you need strict sizes, please set

imageHeight: 100
imageWidth: 100

Forcing the width and height to stretch via CSS is not a solution. We then leave no way for the user to visually distinguish portrait images from landscape images.
The user may want to sort images based on their orientation.

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 11, 2023

In that case, to avoid stretching, it should be this code?

&.image img {
    .border-left-radius(3px);
    width: auto;
    height: auto;
    max-height: 100%;
}

(max-width: 100% is already set by something else)
image

Because if you leave it as is, images get stretched?
image

I don't want to destroy your method of working, but there is something wrong with the css 😅
I don't mind this getting roll backed, I can just add it to my own override.css file

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 12, 2023

How about a PDF icon? Should be centered?
Should all images be centered then?
image

.icon-container {
    border-right: 1px solid #f6f8f9;
    float: left;
    // display: inline-block;
    overflow: hidden;
    width: 75px;
    height: 75px;
    
    display: flex;
    justify-content: center;
    align-items: center;
}

@WebVPF
Copy link
Contributor

WebVPF commented Oct 12, 2023

I like it when it's centered

AIC-BV added a commit to AIC-BV/winter that referenced this pull request Oct 12, 2023
@bennothommo
Copy link
Member

Yeah I'd be happy with it being resized and centered, but still hinting the actual orientation of the image. As long as the result is that all preview images are square in terms of their container. I'm a stickler for consistency myself.

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 16, 2023

image

Right now it should look like this @bennothommo.
If you prefer the 1st draft, where I used object-fit: cover, we will have to add something to display the orientation of the image for @WebVPF's case

@bennothommo
Copy link
Member

@AIC-BV that example you posted in your latest post is what I prefer. :)

@AIC-BV
Copy link
Contributor Author

AIC-BV commented Oct 17, 2023

@AIC-BV that example you posted in your latest post is what I prefer. :)

That one is merged 😇
#991

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance PRs that fix bugs, are translation changes or make only minor changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants