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

Handle resize_to_limit and saver.quality to be compatible with MiniMagick and Vips #7

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

SebouChu
Copy link
Member

@SebouChu SebouChu commented Apr 3, 2022

Modifications apportées

  • Dans la vue Kamifusen, passer sur les options compatibles avec les 2 processeurs (documentation dans l'issue Bug: Incompatible avec Vips #6)
  • Dans le helper keycdn_url processeur :
    • Ajout de la gestion des options compatibles
    • Deprecation notice à l'utilisation de variant(resize: "400>") et variant(quality: 80) au profit de variant(resize_to_limit: [400, nil]) et variant(saver: { quality: 80 })

@SebouChu SebouChu requested a review from arnaudlevy April 3, 2022 08:29
url = "#{Kamifusen.keycdn}/#{variant.blob.key}?"
url += "format=#{transformations[:format]}&" if transformations.has_key? :format

width = get_resize_width
Copy link
Member

Choose a reason for hiding this comment

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

Il vaudrait mieux un lazy load

width = get_resize_width
url += "width=#{width}" unless width.nil?

quality = get_quality
Copy link
Member

Choose a reason for hiding this comment

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

idem lazy load

transformations.dig(:saver, :quality)
end

def get_resize_width
Copy link
Member

Choose a reason for hiding this comment

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

resize_width, lazy

@transformations ||= variant.variation.transformations
end

def get_quality
Copy link
Member

Choose a reason for hiding this comment

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

quality, lazy

Copy link
Member

@arnaudlevy arnaudlevy left a comment

Choose a reason for hiding this comment

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

On peut rendre le code encore plus élégant avec des lazy loaders sans get

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.

2 participants