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

ChristineP2 · 2019-03-24T21:50:58Z

Added the ability to have a partial in the meta. If partial exists, the component will be added to content_for with a name mv_#{component_name}_#{index} and the partial will be rendered instead of the component.

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

Get Latest from DevNacho

Added the ability to have a partial in the meta. If partial exists, the component will be added to content_for with a name mv_#{component_name}_#{index} The partial will use mv_content_for_name to retrieve that specific content_for value and display it within the wrapper.

The content_for block format (as well as trying partial layout: component_stub.meta_partial to use a block format with :yield) resulted in the rails server exiting with an error code. Removing the block seems to have solved the issue.

Added a bit of info about the partials to the readme

ghost · 2019-03-30T13:08:39Z

app/views/mountain_view/styleguide/show.html.erb

+              <%= render_component @component.name, component_stub.properties.clone %>
+            <% else %>
+              <% content_for_name = "mv_#{@component.name}_#{index}" %>
+              <% content_for :"#{content_for_name}",


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...

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.

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.

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.

README.md

kitop · 2019-03-31T18:19:11Z

Thanks for opening this!!

It's looking really good! I'd like to iterate a bit more on the implementation so it looks clean and follows some existing conventions. And would be really nice to have some integration tests for this, as it's not a trivial feature.

Let me know what you think!

ghost · 2019-04-02T03:11:16Z

Definitely more work is needed, but I wanted to get the proof of concept up. I'm really hung up on the server crashing when I use the block version. If you have any ideas for me there, I'd appreciate it.

ChristineP2 · 2019-04-02T03:15:58Z

It didn't occur to me to create a draft pull request, but if we have those as an option, we could shift this to a draft. I didn't see a way to change it, might have to close and re-open as draft.

kitop · 2019-04-03T20:22:44Z

No worries about the Draft PR, I still forget GH has that feature 😅 We can keep working here, no need to change or reopen.

Thanks again for working on this!

This worked on the mac, now retesting in windows.

Trying rename partial as layout and changing to the example provided in the PR comments, in the hopes it will solve the server crash thing. (also got rid of the pink and added a bit of padding lol)

Co-Authored-By: Esteban Pastorino <[email protected]>

ChristineP2 · 2019-05-10T22:25:58Z

So, with any code configuration I have tried (examples include: the current committed code, as well as render :layout and content_for), if I have a block, mountain_view_demo crashes on load of the page with the block. I have access to a Mac at the moment, so I have tried this on both windows and Mac. Side note, The dummy app does not run on the windows machine I usually develop on, however, it does run on the Mac. I'm still trying to figure that one out. Having said that, the dummy app does not crash with this code on a Mac.

On windows the server returns:

Rendering C:/tools/ruby25/lib/ruby/gems/2.5.0/bundler/gems/mountain_view-c545a5ce9ba1/app/views/mountain_view/styleguide/show.html.erb within layouts/mountain_view

Process finished with exit code 3

If I load the demo site on the Mac, the server returns:

Started GET "/mountain_view/styleguide/header" for 127.0.0.1 at 2019-05-10 17:28:37 -0500
Processing by MountainView::StyleguideController#show as HTML
Parameters: {"id"=>"header"}
Rendering vendor/bundle/ruby/2.5.0/bundler/gems/mountain_view-619f13234e8a/app/views/mountain_view/styleguide/show.html.erb within layouts/mountain_view
Rendered app/components/header/_test_layout.html.erb (2.2ms)
Rendered vendor/bundle/ruby/2.5.0/bundler/gems/mountain_view-619f13234e8a/app/views/mountain_view/styleguide/show.html.erb within layouts/mountain_view (22.2ms)
Completed 500 Internal Server Error in 36ms

#<Thread:0x00007f831a928e80@/Users/cpanus/.rbenv/versions/2.5.3/lib/ruby/2.5.0/webrick/server.rb:286 run> terminated with exception (report_on_exception is true):
Traceback (most recent call last):
/Users/cpanus/Code/mountain_view_demo/vendor/bundle/ruby/2.5.0/gems/actionpack-5.2.2/lib/action_dispatch/journey/router.rb:35:in `each': machine stack overflow in critical region (fatal)

Any tips, tricks or suggestions would be greatly appreciated. Also, iirc, I did try a very simple block on the main page of mountain_view_demo and it crashed there as well. Originally I thought this was a problem with using Windows, but that appears not to be the case. I can keep digging, but if you have any ideas, I'd be grateful for a pointer or two.

ChristineP2 · 2019-05-24T14:51:23Z

README.md


+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`.


Needs to be rewritten

kitop · 2019-06-01T10:47:13Z

Hi, I'm sorry for the delay on reading this!! Can't find any obvious suspects in the code...

Can you provide more specific examples on how to reproduce what's failing locally? How are you getting those errors?
eg: checking out this branch and pointing mountain_view_demo to it? Or the dummy app inside?

ChristineP2 · 2019-06-13T16:36:18Z

Sorry for the delayed reply. I have pointed mountain_view_demo at

gem "mountain_view",
    git: 'https://github.com/ChristineP2/mountain_view',
    ref: '619f132'

I could not get the dummy app to work in windows. The dummy app does work on mac and does not err.

I get the errors with the DEMO app on both mac and windows.

ChristineP2 added 4 commits March 23, 2019 23:03

Merge pull request #6 from devnacho/master

0155f14

Get Latest from DevNacho

Remove content_for block format

15dbc51

The content_for block format (as well as trying partial layout: component_stub.meta_partial to use a block format with :yield) resulted in the rails server exiting with an error code. Removing the block seems to have solved the issue.

Update to Readme

aa09f5a

Added a bit of info about the partials to the readme

ChristineP2 mentioned this pull request Mar 24, 2019

Adding a wrapper in the style guide #86

Open

ghost reviewed Mar 30, 2019

View reviewed changes

kitop reviewed Mar 31, 2019

View reviewed changes

README.md Outdated Show resolved Hide resolved

ChristineP2 and others added 3 commits May 10, 2019 12:37

Use Block - Test for Windows

c545a5c

This worked on the mac, now retesting in windows.

Renaming Partial

619f132

Trying rename partial as layout and changing to the example provided in the PR comments, in the hopes it will solve the server crash thing. (also got rid of the pink and added a bit of padding lol)

Update README.md

5ab322d

Co-Authored-By: Esteban Pastorino <[email protected]>

ChristineP2 commented May 24, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ChristineP2 commented Mar 24, 2019

ghost Mar 30, 2019

kitop Mar 31, 2019

ghost Apr 2, 2019 •

edited by ghost

Loading

kitop Apr 3, 2019

kitop commented Mar 31, 2019

ghost commented Apr 2, 2019

ChristineP2 commented Apr 2, 2019

kitop commented Apr 3, 2019

ChristineP2 commented May 10, 2019 •

edited

Loading

ChristineP2 May 24, 2019

kitop commented Jun 1, 2019

ChristineP2 commented Jun 13, 2019 •

edited

Loading


		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`.

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

Are you sure you want to change the base?

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

Conversation

ChristineP2 commented Mar 24, 2019

ghost Mar 30, 2019

Choose a reason for hiding this comment

kitop Mar 31, 2019

Choose a reason for hiding this comment

ghost Apr 2, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

kitop Apr 3, 2019

Choose a reason for hiding this comment

kitop commented Mar 31, 2019

ghost commented Apr 2, 2019

ChristineP2 commented Apr 2, 2019

kitop commented Apr 3, 2019

ChristineP2 commented May 10, 2019 • edited Loading

ChristineP2 May 24, 2019

Choose a reason for hiding this comment

kitop commented Jun 1, 2019

ChristineP2 commented Jun 13, 2019 • edited Loading

ghost Apr 2, 2019 •

edited by ghost

Loading

ChristineP2 commented May 10, 2019 •

edited

Loading

ChristineP2 commented Jun 13, 2019 •

edited

Loading