Skip to content

Commit

Permalink
Respond to feedback from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
joelhawksley committed Mar 3, 2020
1 parent 3f7e5e5 commit 151a53c
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 32 deletions.
28 changes: 1 addition & 27 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# ActionView::Component
# ViewComponent
A view component framework for Rails.

**Current Status**: Used in production at GitHub. Because of this, all changes will be thoroughly vetted, which could slow down the process of contributing. We will do our best to actively communicate status of pull requests with any contributors. If you have any substantial changes that you would like to make, it would be great to first [open an issue](http://github.com/github/actionview-component/issues/new) to discuss them with us.
Expand Down Expand Up @@ -107,8 +107,6 @@ Component class names end in -`Component`.

Component module names are plural, as they are for controllers. (`Users::AvatarComponent`)

Components support ActiveModel validations. Components are validated after initialization, but before rendering.

Content passed to a `ViewComponent` as a block is captured and assigned to the `content` accessor.

#### Quick start
Expand Down Expand Up @@ -144,8 +142,6 @@ A `ViewComponent` is a Ruby file and corresponding template file (in any format
`app/components/test_component.rb`:
```ruby
class TestComponent < ViewComponent::Base
validates :content, :title, presence: true

def initialize(title:)
@title = title
end
Expand Down Expand Up @@ -175,29 +171,13 @@ Which returns:
<span title="my title">Hello, World!</span>
```

#### Error case

If the component is rendered with a blank title:

```erb
<%= render(TestComponent.new(title: "")) do %>
Hello, World!
<% end %>
```

An error will be raised:

`ActiveModel::ValidationError: Validation failed: Title can't be blank`

#### Content Areas

A component can declare additional content areas to be rendered in the component. For example:

`app/components/modal_component.rb`:
```ruby
class ModalComponent < ViewComponent::Base
validates :user, :header, :body, presence: true

with_content_areas :header, :body

def initialize(user:)
Expand Down Expand Up @@ -246,8 +226,6 @@ This allows a few different combinations of ways to render the component:
`app/components/modal_component.rb`:
```ruby
class ModalComponent < ViewComponent::Base
validates :header, :body, presence: true

with_content_areas :header, :body

def initialize(header:)
Expand All @@ -272,8 +250,6 @@ end
`app/components/modal_component.rb`:
```ruby
class ModalComponent < ViewComponent::Base
validates :header, :body, presence: true

with_content_areas :header, :body

def initialize(header: nil)
Expand Down Expand Up @@ -308,8 +284,6 @@ end
`app/components/modal_component.rb`:
```ruby
class ModalComponent < ViewComponent::Base
validates :body, presence: true

with_content_areas :header, :body

def initialize(header: nil)
Expand Down
2 changes: 1 addition & 1 deletion lib/action_view/component.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# frozen_string_literal: true
# frozen_string_literal: true

require "active_model"
require "view_component"
6 changes: 3 additions & 3 deletions lib/view_component/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -160,17 +160,17 @@ def compiled?
end

def compile!
compile(validate: true)
compile(raise_template_errors: true)
end

# Compile templates to instance methods, assuming they haven't been compiled already.
# We could in theory do this on app boot, at least in production environments.
# Right now this just compiles the first time the component is rendered.
def compile(validate: false)
def compile(raise_template_errors: false)
return if compiled?

if template_errors.present?
raise ViewComponent::TemplateError.new(template_errors) if validate
raise ViewComponent::TemplateError.new(template_errors) if raise_template_errors
return false
end

Expand Down
2 changes: 1 addition & 1 deletion test/app/components/path_container_component.html.erb
Original file line number Diff line number Diff line change
@@ -1 +1 @@
<%= render PathComponent.new %>
<%= render PathComponent.new %>
1 change: 1 addition & 0 deletions test/app/components/validations_component.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<span><%= content %></span>
7 changes: 7 additions & 0 deletions test/app/components/validations_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# frozen_string_literal: true

class ValidationsComponent < ActionView::Component::Base
validates :content, presence: true

def initialize(*); end
end
8 changes: 8 additions & 0 deletions test/view_component/view_component_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,14 @@ def test_no_validations_component
assert_selector("div")
end

def test_validations_component
exception = assert_raises ActiveModel::ValidationError do
render_inline(ValidationsComponent.new)
end

assert_equal exception.message, "Validation failed: Content can't be blank"
end

private

def modify_file(file, content)
Expand Down

0 comments on commit 151a53c

Please sign in to comment.