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

Use partial and content_for to wrap components in style guide #88

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ To customize the styleguide, override the style guide layout by adding `mountain

### Custom meta data for stub examples

You can customize the title, description for each example in the stub, as well as the classes that surround the stub example. In order to override the default title, add a `title` key to the `mv_stub_meta` hash. Additional special keys include `description` which will add a description under the title for a given example and `classes` which will add classes for a specific example.
You can customize the title, description for each example in the stub, as well as the classes that surround the stub example. In order to override the default title, add a `title` key to the `mv_stub_meta` hash. Additional special keys include `description` which will add a description under the title for a given example, `classes` which will add classes for a specific example, and `partial` which will allow ou to embed the component in a partial (see below for details).
ChristineP2 marked this conversation as resolved.
Show resolved Hide resolved

E.g: `app/components/card/card.yml`
```yml
Expand All @@ -256,6 +256,7 @@ E.g: `app/components/card/card.yml`
:title: "Specific Example"
:description: "Instructions for use case or other UX considerations"
:classes: "black-background"
:partial: "card/card_styleguide_partial.html.erb"
:title: "Aspen Snowmass"
:description: "Aspen Snowmass is a winter resort complex located in Pitkin County in western Colorado in the United States. Owned and operated by the Aspen Skiing Company it comprises four skiing/snowboarding areas on four adjacent mountains in the vicinity of the towns of Aspen and Snowmass Village."
:link: "http://google.com"
Expand All @@ -268,7 +269,11 @@ E.g: `app/components/card/card.yml`
:title: "Depth"
:number: '71"'
```
### Wrapping Components in the Style Guide

Sometimes you may want to wrap extra content around the component in the style guide. For example, you may want to include a form tag around a custom form component without including the form tag in the component itself. You can do this by adding the key `partial` in the `mv_stub_meta`.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Needs to be rewritten


To include the component in the partial, you reference `<%= content_for :"#{mv_content_for_name}" %>`

## Improving performance
Rendering a large amount of partials in a request can lead to a performance bottleneck, usually this is caused by the parsing and rendering of template code such as ERB or HAML.
Expand Down
9 changes: 8 additions & 1 deletion app/views/mountain_view/styleguide/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,14 @@
<h2><%= component_stub.meta_title(@component.title, index) %></h2>
<div class="mv-component">
<div class="mv-component__item <%= component_stub.meta_classes %>">
<%= render_component @component.name, component_stub.properties.clone %>
<% if component_stub.meta_partial.blank? %>
<%= render_component @component.name, component_stub.properties.clone %>
<% else %>
<% content_for_name = "mv_#{@component.name}_#{index}" %>
<% content_for :"#{content_for_name}",
Copy link

Choose a reason for hiding this comment

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

Could theoretically use a layout with yield as well... for some reason I can't use a block though... my server crashes if I use a block with content_for or render layout...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting... Yes, that would look much better, IMO. Passing a mv_content_for_name feels kinda dirty.

Ideally this would be something like:

<%= render component_stub.meta_layout do %>
  <%=  render_component @component.name, component_stub.properties.clone %>
<% end %>

And we could generalize it more if we returned an empty partial (with just yield) by default in MountainView::Stub#meta_partial (and only override if specified), so we don't need the if component_stub.meta_partial.blank? check.

Copy link

@ghost ghost Apr 2, 2019

Choose a reason for hiding this comment

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

I like that idea a lot... but I have to figure out why using the render with a block, as above, causes the server to crash. Have you heard anything about that? I lean toward blaming windows :/ But it probably needs to work on windows, so I'd have to figure it out.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I haven't heard anything about that :(
But if you want to push those changes that make your server crash either here or to a different branch, I'm more than happy to take a look and try to see if I can reproduce or not.

render_component(@component.name, component_stub.properties.clone) %>
<%= render component_stub.meta_partial, mv_content_for_name: content_for_name %>
<% end %>
</div>
<div class="mv-component__description">
<h3><%= t('mountain_view.show.instruction_heading') %></h3>
Expand Down
4 changes: 4 additions & 0 deletions lib/mountain_view/stub.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,9 @@ def meta_description
def meta_classes
@meta[:classes]
end

def meta_partial
@meta[:partial]
end
end
end
4 changes: 4 additions & 0 deletions test/dummy/app/components/header/_test_partial.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<div class="test_partial" style="background-color: pink">
Custom Partial around style use
<%= content_for :"#{mv_content_for_name}" %>
</div>
1 change: 1 addition & 0 deletions test/dummy/app/components/header/header.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
:title: "Specific Example"
:description: "Instructions for use case and UX considerations"
:classes: "black-background"
:partial: "header/test_partial.html.erb"
:id: 1
:title: "20 Mountains you didn't know they even existed"
:subtitle: "Buzzfeed title"
Expand Down
9 changes: 9 additions & 0 deletions test/mountain_view/stub_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,15 @@ def test_meta_classes
'nil not set for stub meta without classes'
end

def test_meta_partial
test_object = stub_to_test
assert_equal header_stub_meta[:stubs][0][:mv_stub_meta][:partial],
test_object[0].meta_partial,
'Stub Partial not found'
assert_nil test_object[1].meta_partial,
'nil not set for stub meta without partial'
end

def test_properties
test_object = stub_to_test
assert_equal header_stub_meta[:stubs][0].except(:mv_stub_meta),
Expand Down
3 changes: 2 additions & 1 deletion test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ def header_stub_meta
mv_stub_meta: {
title: "Specific Example",
description: "Instructions for use case and UX considerations",
classes: "black-background"
classes: "black-background",
partial: "header/test_partial.html.erb"
},
id: 1,
title: "20 Mountains you didn't know they even existed",
Expand Down