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

Slots V2 #503

Merged
merged 41 commits into from Dec 9, 2020
Merged

Slots V2 #503

merged 41 commits into from Dec 9, 2020

Conversation

BlakeWilliams
Copy link
Contributor

@BlakeWilliams BlakeWilliams commented Oct 13, 2020

Summary

This PR introduces the second iteration of slots, Slotable::V2. This takes
feedback we've received about the original iteration of slots as well as the
experiences we've had with the original iteration of slots and builds on them.

Here's an overview of the changes:

  • with_slot becomes renders_one
  • with_slot collections become renders_many
  • Slots no longer accept a class_name argument, and instead accept a component or a lambda that allow you to extend the slot.
  • Slots now accept positional arguments in addition to keyword arguments
  • Slots no longer use content in templates to render their content, they instead use to_s. e.g. <%= item.content %> becomes <%= item %>
  • Components no longer set slot values with the #slot method, instead they use the slot method directly. e.g. c.slot(:row, theme: :yellow)becomes c.row(theme: yellow)

V1 example

Defining a component

class MyComponent < ViewComponent::Base
  with_slot :row, collection: true, class_name: "Row"

  class Row < ViewComponent::Slot
    def initialize(theme: :gray)
      @theme = theme
    end

    def theme_class_name
      case @theme
      when :gray
        "Box-row--gray"
      when :hover_gray
        "Box-row--hover-gray"
      when :yellow
        "Box-row--yellow"
      when :blue
        "Box-row--blue"
      when :hover_blue
        "Box-row--hover-blue"
      else
        "Box-row--gray"
      end
    end
  end
end

Defining a component's template

<%= if rows.any? %>
  <% rows.each do |row| %>
    <%= row.content %>
  <% end %>
<% end %>

Using a component

<% component.slot(:row) do %>
  Row one
<% end %>
<% component.slot(:row, theme: :yellow) do %>
  Yellow row
<% end %>

V2 equivalent of the above example

Defining a component

class MyComponent < ViewComponent::Base
  renders_many :rows, -> (theme: :gray) { Row.new(theme: theme) }

  class RowComponent < ViewComponent::Base
    def initialize(theme:)
      @theme = theme
    end

    def theme_class_name
      case @theme
      when :gray
        "Box-row--gray"
      when :hover_gray
        "Box-row--hover-gray"
      when :yellow
        "Box-row--yellow"
      when :blue
        "Box-row--blue"
      when :hover_blue
        "Box-row--hover-blue"
      else
        "Box-row--gray"
      end
    end
  end
end

Defining a component's template

<%= if rows.any? %>
  <% rows.each do |row| %>
    <%= row %>
  <% end %>
<% end %>

Defining the Row component's template:

<div class="<%= theme_class_name %>">
  <%= content %>
</div>

Using a component

<% component.row do %>
  Row one
<% end %>
<% component.row(theme: :yellow) do %>
  Yellow row
<% end %>

@swanson
Copy link
Contributor

swanson commented Oct 17, 2020

I really like the <% component.row do %> short-hand, but I prefer the old with_slot DSL for defining slots. I can see how the new language is more Rails-y, but my sense is that "slots" term is winning the mindshare (with the two closest alternatives to view_component to me being Vue and the headless-ui work from the Tailwind team that use "Slots").

@bradgessler
Copy link

I like this API much better, including the terminology "renders" over "slots". It actually reminds me quite a bit of ActiveRecord has_many blocks.

I am curious, what would it look like if somebody wanted to reverse the order of what's rendered from the components before_render methods? There's a lot of other use cases that may want to programmatically alter the state of the view component or collections from component code (not the view).

@bbugh
Copy link
Contributor

bbugh commented Oct 21, 2020

Thanks for all of your hard work on this! The slot less usage looks great.

One of the things I am most excited about with view_component are libraries like bootstrap-view-component and simple-form-view-component equivalents. I was disappointed to see the DSL come back after all of the discussion in the first thread, but I don't have anything new to say and won't rehash it.

How will the DSL behave when a subclassed component needs to override the slot?

class ThirdPartyGemComponent < ViewComponent::Base
  renders_one :thing do
    # ...
  end
end
class MyAppComponent < ThirdPartyGemComponent
  renders_one :thing do
    # Does this replace the other slot? Extend it? 
  end
end

What about templates? If MyAppComponent defines a template, will it override the ThirdPartyGemComponent one?

I'm SO THRILLED y'all are working so hard on making this an important part of Rails. I didn't like the Rails view/helper system ten years and I like it even less now. Component-based setups are really awesome way to shore up some of the issues with a viewmodel-less MVC scenario.

@BlakeWilliams
Copy link
Contributor Author

How will the DSL behave when a subclassed component needs to override the slot?

That's a great question. Right now you would you would call renders_one or renders_many and the slot would be overwritten. There is no inheriting slots, but I think the changes in v2 will help promote composition. React has some really great documentation on it that I feel applies to view components as well: https://reactjs.org/docs/composition-vs-inheritance.html

That being said, this PR is trending away from slots being their own user defined classes, and more towards being functions that can return HTML or a component instance that will be rendered on the page. I think being able to delegate to an existing component provides an escape hatch for inheritance if it's necessary.

class ThirdPartyGemComponent < ViewComponent::Base
  renders_one :thing do
    # ...
  end
end
class MyAppComponent < ThirdPartyGemComponent
  renders_one :thing do
    # Does this replace the other slot? Extend it? 
  end
end

In this example, renders_one :thing would replace the parent implementation. We should definitely add test coverage though.

What about templates? If MyAppComponent defines a template, will it override the ThirdPartyGemComponent one?

Yep! Exactly. I think that should be the existing behavior too.

I'm SO THRILLED y'all are working so hard on making this an important part of Rails. I didn't like the Rails view/helper system ten years and I like it even less now. Component-based setups are really awesome way to shore up some of the issues with a viewmodel-less MVC scenario.

Thank you! That means a lot. ❤️

@BlakeWilliams BlakeWilliams marked this pull request as ready for review October 30, 2020 18:15
view_component.gemspec Outdated Show resolved Hide resolved
lib/view_component/base.rb Outdated Show resolved Hide resolved
lib/view_component/slot.rb Outdated Show resolved Hide resolved
lib/view_component/slot.rb Outdated Show resolved Hide resolved
lib/view_component/slot.rb Outdated Show resolved Hide resolved
test/app/components/slots_v2_component.html.erb Outdated Show resolved Hide resolved
test/app/components/slots_v2_component.rb Outdated Show resolved Hide resolved
test/app/components/slots_v2_component.rb Outdated Show resolved Hide resolved
test/app/components/slots_v2_component.rb Outdated Show resolved Hide resolved
test/view_component/slotable_v2_test.rb Outdated Show resolved Hide resolved
Copy link
Collaborator

@jonspalmer jonspalmer left a comment

Choose a reason for hiding this comment

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

great work a couple of questions about details

@joelhawksley
Copy link
Member

TODO: collection slot rendering:

component.items(@items...)

@joelhawksley
Copy link
Member

TODO: Add to docs/index.md

@joelhawksley
Copy link
Member

TODO: Add test for subcomponent with call method instead of template.

@joelhawksley
Copy link
Member

TODO: Note that slots V1 is deprecated.

Gemfile.lock Outdated Show resolved Hide resolved
Gemfile.lock Outdated Show resolved Hide resolved
lib/view_component/base.rb Outdated Show resolved Hide resolved
lib/view_component/compiler.rb Outdated Show resolved Hide resolved
lib/view_component/sub_component_wrapper.rb Outdated Show resolved Hide resolved
test/app/components/sub_component_component.rb Outdated Show resolved Hide resolved
test/app/components/sub_component_delegate_component.rb Outdated Show resolved Hide resolved
test/view_component/sub_components_test.rb Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
Copy link
Contributor

@halo halo 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 for your hard work on this. This is great.

docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/slots_v1.md Outdated Show resolved Hide resolved
@BlakeWilliams
Copy link
Contributor Author

Just pushed up a PR that should resolve the last bit of docs feedback.

docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
docs/index.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Joel Hawksley <joelhawksley@github.com>
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@joelhawksley joelhawksley merged commit 33dcfd9 into master Dec 9, 2020
@joelhawksley joelhawksley deleted the slots-v2 branch December 9, 2020 17:55
@cheshire137
Copy link
Contributor

Is this available in the view_component gem yet? The latest version I'm seeing is 2.22.1 from Nov 10. 🙇‍♀️

@cheshire137
Copy link
Contributor

Ah, 2.23.0 has it, thanks @BlakeWilliams!

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.

None yet

8 participants