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

api: fix best fit for thumbnail, add background #54

Merged
merged 3 commits into from
May 23, 2018

Conversation

kpsherva
Copy link
Contributor

@kpsherva kpsherva commented May 4, 2018

  • fixes for best fit case (f.e. dimensions !400,200) -
    changed variable in the formula

  • expected behaviour for best fit case changed

    • now it resizes the image and keeps the ratio of
      an image and fills the background to requested size
      with black pixels - this helps to keep responsive
      images in the UI
  • adds the function to fill the pixels with
    black background if the best fit is smaller
    than requested thumbnail box size - preserves
    the thumbnail ratio

  • add the function to fill the background with
    black pixels for GIF if the best fit is
    smaller than requested window size

  • provides testing cases for portrait, landscape
    and fixed best fit case

  • related to requested fix: (closes ! parameter is not correctly calculated #47).

@egabancho egabancho requested a review from drjova May 4, 2018 12:22
(200, 100),
(1280, 720),
])
def setUp(self, width=1280, height=1024):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since you parametrize it, could you please move the imports outside :)

from PIL import Image
from flask_iiif.api import MultimediaImage

@drjova
Copy link
Member

drjova commented May 14, 2018

@kprzerwa thank you for your contribution, could you please rebase?

@drjova
Copy link
Member

drjova commented May 16, 2018

Could you please also add a release commit for version v0.5.0, example

* fixes for best fit case (f.e. dimensions !400,200) -
  changed variable in the formula

* expected behaviour for best fit case changed
  - now it resizes the image and keeps the ratio of
  an image and fills the background to requested size
  with black pixels - this helps to keep responsive
  images in the UI

* adds the function to fill the pixels with
  black background if the best fit is smaller
  than requested thumbnail box size - preserves
  the thumbnail ratio

* add the function to fill the background with
  black pixels for GIF if the best fit is
  smaller than requested window size

* provides testing cases for portrait, landscape
  and fixed best fit case
@kpsherva
Copy link
Contributor Author

@drjova thank you for the review, is it all ready to merge?

@lnielsen lnielsen merged commit 45e66f2 into inveniosoftware:master May 23, 2018
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.

! parameter is not correctly calculated
3 participants