Re: [Rails-core] [Feature][ActiveStorage] Pre-defined Variants

2018-11-27 Thread Abhishek Chandrasekhar
@kasper - Allowing the one-off options on a given variant is an even better 
approach and just as easily implementable. Thanks for the suggestion.

> I’m not looking to see this through though, so you’d have to get George 
Claghorn or someone else on board for this ride.

Totally understood :)
@George - would love to hear your thoughts on this. If you agree, I don't 
mind picking up the development and submitting a patch. 

Thanks.

On Tuesday, November 27, 2018 at 10:02:39 AM UTC-5, Kasper Timm Hansen 
wrote:
>
> I do remember proposing this at one point internally:
>
> has_one_attached :avatar do |attachable|
>   attachable.variant :small, resize: ’100x100>’
>   …
> end
>
> I’m also fine with exposing it as `variant(:small)` or for one offs 
> `variant(:small, caption: ’foo’)`.
>
> I’m not looking to see this through though, so you’d have to get George 
> Claghorn or someone else on board for this ride.
>
> Appreciate the extensive write up with reasoning!
>
> Den 27. nov. 2018 kl. 14.28 skrev Abhishek Chandrasekhar <
> abhishek.ch...@gmail.com >:
>
> Hello all -
>
> Firstly, huge thanks to those who have worked on ActiveStorage so far. The 
> library seems to be coming along nicely. 
>
>
> ActiveStorage currently allows you to define variants of an attachment as 
> follows:
>
> ```ruby
> class User < ActiveRecord::Base
>   has_one_attached :avatar
>
>   # ...
> end
>
> user.avatar.variant(resize: "100x100>")
> user.avatar.variant(resize: "100x100>", caption: "foo")
> user.avatar.variant(resize: "200x200", rotate: "-90")
> ```
>
>
> I'd like to propose the following functionality that lets users configure 
> and pre-define variants. 
>
> ```ruby
> class User < ActiveRecord::Base
>   has_one_attached(
> :avatar, 
> variants: { 
>   small: { resize: "100x100>" }
>   small_captioned: { resize: "100x100>", caption: "foo" }
>   medium_rotated: { resize: "200x200", rotate: "-90" }
> }
>   )
>
>   # ...
> end
>
> user.avatar.variant(:small)
> user.avatar.variant(:small_captioned)
> user.avatar.variant(:medium_rotated)
>
> # Something not pre-definied
> user.avatar.variant(rotate: "120")
> ```
>
> This is similar in concept to how existing attachment libraries 
> (paperclip, carrierwave, etc...) have allowed definition and configuration 
> of variants.
>
> It is true that this functionality can be mimicked outside of 
> activestorage by having the developer maintain a manual mapping of key 
> names to variant configurations. However, I believe this should be part of 
> ActiveStorage directly because -
>
>
> 1. It leads to cleaner/more readable code (e.g. `
> user.avatar.variant(:small)` is easy to understand)
> 2. It keeps configuration consolidated inline with `has_one_attached`, 
> which is similar to how options are already defined inline with `has_one`, 
> `has_many`, etc...
> 3. It's fully backward compatible with how variants are invoked right now 
> and doesn't force you to use a particular approach.
>
> Would such a feature be accepted if I were to submit a pull request for 
> it? 
>
> Thank you!
>
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to rubyonrails-co...@googlegroups.com .
> To post to this group, send email to rubyonra...@googlegroups.com 
> .
> Visit this group at https://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> Kasper
>
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.


Re: [Rails-core] [Feature][ActiveStorage] Pre-defined Variants

2018-11-27 Thread Kasper Timm Hansen
I do remember proposing this at one point internally:

has_one_attached :avatar do |attachable|
  attachable.variant :small, resize: ’100x100>’
  …
end

I’m also fine with exposing it as `variant(:small)` or for one offs 
`variant(:small, caption: ’foo’)`.

I’m not looking to see this through though, so you’d have to get George 
Claghorn or someone else on board for this ride.

Appreciate the extensive write up with reasoning!

> Den 27. nov. 2018 kl. 14.28 skrev Abhishek Chandrasekhar 
> :
> 
> Hello all -
> 
> Firstly, huge thanks to those who have worked on ActiveStorage so far. The 
> library seems to be coming along nicely. 
> 
> 
> ActiveStorage currently allows you to define variants of an attachment as 
> follows:
> 
> ```ruby
> class User < ActiveRecord::Base
>   has_one_attached :avatar
> 
>   # ...
> end
> 
> user.avatar.variant(resize: "100x100>")
> user.avatar.variant(resize: "100x100>", caption: "foo")
> user.avatar.variant(resize: "200x200", rotate: "-90")
> ```
> 
> 
> I'd like to propose the following functionality that lets users configure and 
> pre-define variants. 
> 
> ```ruby
> class User < ActiveRecord::Base
>   has_one_attached(
> :avatar, 
> variants: { 
>   small: { resize: "100x100>" }
>   small_captioned: { resize: "100x100>", caption: "foo" }
>   medium_rotated: { resize: "200x200", rotate: "-90" }
> }
>   )
> 
>   # ...
> end
> 
> user.avatar.variant(:small)
> user.avatar.variant(:small_captioned)
> user.avatar.variant(:medium_rotated)
> 
> # Something not pre-definied
> user.avatar.variant(rotate: "120")
> ```
> 
> This is similar in concept to how existing attachment libraries (paperclip, 
> carrierwave, etc...) have allowed definition and configuration of variants.
> 
> It is true that this functionality can be mimicked outside of activestorage 
> by having the developer maintain a manual mapping of key names to variant 
> configurations. However, I believe this should be part of ActiveStorage 
> directly because -
> 
> 
> 1. It leads to cleaner/more readable code (e.g. `user.avatar.variant(:small)` 
> is easy to understand)
> 2. It keeps configuration consolidated inline with `has_one_attached`, which 
> is similar to how options are already defined inline with `has_one`, 
> `has_many`, etc...
> 3. It's fully backward compatible with how variants are invoked right now and 
> doesn't force you to use a particular approach.
> 
> Would such a feature be accepted if I were to submit a pull request for it? 
> 
> Thank you!
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to rubyonrails-core+unsubscr...@googlegroups.com 
> .
> To post to this group, send email to rubyonrails-core@googlegroups.com 
> .
> Visit this group at https://groups.google.com/group/rubyonrails-core 
> .
> For more options, visit https://groups.google.com/d/optout 
> .

--
Kasper

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.