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

Deprecate Base#with_variant #1041

Merged
merged 4 commits into from
Aug 17, 2021
Merged

Deprecate Base#with_variant #1041

merged 4 commits into from
Aug 17, 2021

Conversation

camertron
Copy link
Contributor

@camertron camertron commented Aug 16, 2021

Summary

Deprecates Base#with_variant. Variants are an often misunderstood feature that use the Action Pack variant information present in the request (i.e. phone, tablet, etc) to render a component differently depending on the requesting device. Normally one passes in a list of acceptable variants in an Action View template, eg:

render(MyComponent.new, variants: [:phone, :tablet])

The variant can be overridden by calling #with_variant on the component, eg:

class MyComponent < ViewComponent::Base
  def call
    render(OtherComponent.new.with_variant(:tablet))
  end
end

It's important to note that these are two distinct render functions (the second does not accept a :variants option). Since components don't have access to the request object, it doesn't really make sense for them to decide which variant to render (that's probably why :variants isn't available in the first place). The consensus on the team was that overriding the variant is something that should really only be done in a test environment and not supported otherwise.

@camertron camertron requested a review from joelhawksley August 16, 2021 22:42
@camertron camertron linked an issue Aug 16, 2021 that may be closed by this pull request
@camertron camertron requested a review from joelhawksley August 17, 2021 18:39
lib/view_component/base.rb Outdated Show resolved Hide resolved
@camertron camertron merged commit 0fd7cbf into main Aug 17, 2021
@camertron camertron deleted the deprecate_with_variant branch August 17, 2021 22:02
@coorasse
Copy link

coorasse commented May 17, 2022

We are using variants, right now, to define two different templates for the same component.
Noticing the Deprecation Warning we realised that we are abusing the variant concept.
The idea is simple: the same component renders something in a page, and something completely different in another page.
We keep the same component, and just define two variants. How do we achieve that otherwise?

Reading the suggestions above:

https://guides.rubyonrails.org/layouts_and_rendering.html#the-variants-option is what powers the variant functionality in view component. (The only change we made is disabling explicitly setting variants on a component.)

How can we still render a specific variant explicitly? This does not seem possible otherwise.

You can pass an argument to the component so it renders different content.

Passing each time the template name, to the component will for sure work, so, the idea would be that I pass a template_name to the component and how can I conditionally render a template or another?

You can conditionally render a different component based on some condition in your code.

This is not an option for us since we have about 10 components all inheriting from the same parent.

@joelhawksley
Copy link
Member

@coorasse I'm not sure how complicated your templates are, but perhaps you could combine them together into a single template for the component and use conditional logic to switch between each mode, say with a mode parameter?

@coorasse
Copy link

Thanks for the input.
Complexity does not matter too much in my opinion: the point is that we need to pass an additional parameter mode, which basically decides the template to render. We then have to use a if...else somewhere to decide which template to render. All of this was already possible with with_variant which was very convenient and allowing a component to have multiple templates. That's not possible anymore, but needs to be implemented "manually". It would be nice to have an alternative 😄

@joelhawksley
Copy link
Member

@coorasse I think that's a good idea for a feature proposal then! #1096 has some overlap with what you're looking to do, perhaps you could check that out and provide commentary there?

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.

Deprecate Base#with_variant
5 participants