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

Jump to bottom

[ActiveStorage] Allow variant method to be called by TIFF images #34824

Merged
merged 1 commit into from Dec 30, 2018
Merged

[ActiveStorage] Allow variant method to be called by TIFF images #34824

merged 1 commit into from Dec 30, 2018

Conversation

lucianosousa
Copy link
Contributor

Summary

Allow a new image extension(tiff) to call the variant method and generate custom images

Other Information

Fixes #34698
TIFF images are supported by IE and Safari browsers, but not by Chrome or Firefox

sikachu
sikachu reviewed Dec 30, 2018
View reviewed changes
Copy link
Member

@sikachu sikachu left a comment

Choose a reason for hiding this comment

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

I think the PR looks fine. I have one minor styling comment.

activestorage/app/models/active_storage/variant.rb Outdated Show resolved Hide resolved
sikachu
sikachu approved these changes Dec 30, 2018
View reviewed changes
Copy link
Member

@sikachu sikachu left a comment

Choose a reason for hiding this comment

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

Thank you!

@georgeclaghorn I think this is ready to merge.

georgeclaghorn
georgeclaghorn suggested changes Dec 30, 2018
View reviewed changes
activestorage/app/models/active_storage/variant.rb Outdated Show resolved Hide resolved
activestorage/CHANGELOG.md Outdated Show resolved Hide resolved
activestorage/test/models/variant_test.rb Outdated Show resolved Hide resolved
@georgeclaghorn
Copy link
Contributor

georgeclaghorn commented Dec 30, 2018 •
edited

Last thing: let’s convert the existing racecar.jpg fixture to TIFF instead of introducing a new document of unknown provenance. You can generate racecar.tif with the following ImageMagick command:

$ convert -compress none activestorage/test/fixtures/racecar.jpg activestorage/test/fixtures/racecar.tif

I’d just give you the resulting TIFF file, but GitHub doesn’t allow attaching it and I’m too lazy to upload it elsewhere.

@lucianosousa
Copy link
Contributor Author

@georgeclaghorn well done sir! let me know if looks good!

@georgeclaghorn georgeclaghorn merged commit c329d32 into rails:master Dec 30, 2018
@lucianosousa lucianosousa deleted the active-storage-tiff-support branch December 31, 2018 06:51
@gmcgibbon gmcgibbon mentioned this pull request Dec 31, 2018
Merged
@ConnorSheremeta ConnorSheremeta mentioned this pull request Jun 21, 2019
Closed
suketa added a commit to suketa/rails_sandbox that referenced this pull request Jul 6, 2019
@suketa
try053
fe2ff8f 
[ActiveStorage] Allow `variant` method to be called by `TIFF`
images
rails/rails#34824
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sikachu sikachu sikachu approved these changes

@georgeclaghorn georgeclaghorn georgeclaghorn approved these changes

Assignees
No one assigned
Labels
activestorage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lucianosousa @georgeclaghorn @sikachu