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

thumbnails: Add portrait resolutions to default config #5656

Merged
merged 2 commits into from
Feb 27, 2023
Merged

thumbnails: Add portrait resolutions to default config #5656

merged 2 commits into from
Feb 27, 2023

Conversation

jacobgkau
Copy link
Contributor

Description

This adds corresponding portrait-orientation resolutions for each existing landscape-orientation resolution to the thumbnailer service's default config.

Related Issue

This improves, but does not actually fix, the following bug (affecting web):

Motivation and Context

Cropping portrait-orientation photos to landscape greatly reduces the usefulness of the resulting thumbnails (see the screenshots section below.)

How Has This Been Tested?

I added these three portrait-orientation resolutions to the config on my own server, and it improved the behavior by allowing a portrait-orientation photo to have a portrait-orientation preview. (The behavior is still not ideal because the preview should not be cropped at all, but this at least allows me to see most of the image in the preview, instead of a small fraction of it.)

Screenshots (if appropriate):

I have been uploading scanned documents to my OCIS server, some of which are in image format. Below is what a document looked like with the current default config (left), and what it looks like after adding the portrait orientations (right):

image

Much more of the image is now visible in the web view.

Although this isn't a proper fix for the web view bug, it seems to make sense that anyone with a portrait-orientation photo and using it in a cropped/thumbnailed context would prefer to have more of their photo appear in the thumbnail. Adding the portrait orientations to the default config was suggested by an ownCloud developer here.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

@update-docs
Copy link

update-docs bot commented Feb 26, 2023

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

Signed-off-by: jkoberg <[email protected]>
@sonarcloud
Copy link

sonarcloud bot commented Feb 27, 2023

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 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@mmattel mmattel merged commit 473cc7d into owncloud:master Feb 27, 2023
ownclouders pushed a commit that referenced this pull request Feb 27, 2023
Merge: 05644e3 f5fbd8a
Author: Martin <[email protected]>
Date:   Mon Feb 27 09:43:21 2023 +0100

    Merge pull request #5656 from jacobgkau/portrait-thumbnails

    thumbnails: Add portrait resolutions to default config
@jacobgkau jacobgkau deleted the portrait-thumbnails branch February 27, 2023 16:16
@micbar micbar mentioned this pull request May 3, 2023
89 tasks
@ScharfViktor ScharfViktor mentioned this pull request May 4, 2023
86 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants