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

Combine predefined activestorage attachment variants #46242

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mdh
Copy link
Contributor

@mdh mdh commented Oct 14, 2022

Motivation / Background

Sometimes you will want to combine predefined-variants, like rotate and thumb:

@user.avatar.variant(:thumb, :rotate_90)

Until now, you could only use 1 variant. If you would like to combine multiple variants, you would have to explicitly create all possible combinations, which becomes quite cluttered quick, e.g.:

attachable.variant :thumb, ...
attachable.variant :thumb_rotate_90, ...
attachable.variant :thumb_rotate_180, ...

This PR adds support for combining multiple variants.

This Pull Request has been created because we use activestorage a lot and sometimes want to combine predefined-variants.

Detail

This Pull Request changes the way that ActiveStorage::Attachment looks up transformations. It is backwards compatible, but allows users to define more than 1 predefined variant.

Before:

has_one_attached :avatar do |attachable|
  attachable.variant :thumb,            resize_to_limit: [100, 100]
  attachable.variant :thumb_rotate_90,  resize_to_limit: [100, 100], rotate: 90
  attachable.variant :thumb_rotate_180, resize_to_limit: [100, 100], rotate: 180
end

@user.avatar.variant(:thumb_rotate_90)

After:

has_one_attached :avatar do |attachable|
  attachable.variant :thumb, resize_to_limit: [100, 100]
end

@user.avatar.variant(:thumb, rotate: 90)

Additional information

I chose to use some recursion in the lookup process, it works just fine, but might be hard to read for some. Could write this out in a more explicit way.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • There are no typos in commit messages and comments.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Feature branch is up-to-date with main (if not - rebase it).
  • Pull request only contains one commit for bug fixes and small features. If it's a larger feature, multiple commits are permitted but must be descriptive.
  • Tests are added if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.
  • PR is not in a draft state.
  • CI is passing.

@mdh mdh changed the title Add support for multiple predefined activestorage attachment variations Add support for combining predefined activestorage attachment variations Oct 14, 2022
@mdh mdh changed the title Add support for combining predefined activestorage attachment variations Combine predefined activestorage attachment variations Oct 14, 2022
@mdh mdh changed the title Combine predefined activestorage attachment variations Combine predefined activestorage attachment variants Oct 14, 2022
@mdh mdh force-pushed the add-multiple-variant-support branch from a46180e to fb2a731 Compare October 14, 2022 07:44
@mdh mdh force-pushed the add-multiple-variant-support branch from f3176ca to 907e81d Compare June 23, 2023 13:14
@mdh mdh force-pushed the add-multiple-variant-support branch 3 times, most recently from 435288f to 1ddb0d6 Compare July 6, 2023 09:34
@mdh mdh force-pushed the add-multiple-variant-support branch 2 times, most recently from d94277e to a4be2f0 Compare August 3, 2023 21:13
@mdh mdh force-pushed the add-multiple-variant-support branch from a4be2f0 to ce2f785 Compare August 14, 2023 13:05
@mdh mdh force-pushed the add-multiple-variant-support branch 2 times, most recently from 5759252 to fdd17ec Compare September 14, 2023 09:51
@mdh mdh force-pushed the add-multiple-variant-support branch from fdd17ec to 3deae75 Compare September 28, 2023 08:25
@mdh mdh force-pushed the add-multiple-variant-support branch from 3deae75 to d5e720a Compare November 17, 2023 15:03
@mdh mdh force-pushed the add-multiple-variant-support branch from d5e720a to 562226d Compare March 25, 2024 18:04
ActiveStorage attachments can have variants.
You can also predefine these variants, e.g. "thumb".

Sometimes you will want to combine these predefined variants, like rotate a thumb:
  @user.avatar.variant(:thumb, :rotate_90)

This change makes that possible.
@mdh mdh force-pushed the add-multiple-variant-support branch from 562226d to 5911ff3 Compare March 25, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants