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

sj26's fork with specs #2

Merged
merged 3 commits into from Mar 1, 2012
Merged

sj26's fork with specs #2

merged 3 commits into from Mar 1, 2012

Conversation

rwz
Copy link
Owner

@rwz rwz commented Feb 29, 2012

No description provided.

justinfrench added a commit that referenced this pull request Mar 1, 2012
sj26's fork with specs
@justinfrench justinfrench merged commit 8c153df into rwz:master Mar 1, 2012
@justinfrench
Copy link
Contributor

Pulled in, thanks!

justinfrench added a commit that referenced this pull request Mar 1, 2012
@rwz
Copy link
Owner Author

rwz commented Mar 1, 2012

Awesome. Now could you please kindly release this as new gem version? Cause I actually want to use nestive as a dependency in other gem of mine. Tnx!

@justinfrench
Copy link
Contributor

I will, but would like to go play with what we have a grok the tests for a bit before I push a new gem into the unsuspecting public. Expect one next week and feel free to hassle me around Wednesday :)

@justinfrench
Copy link
Contributor

Also, what's the dependency? Something cool I want to know about?

@rwz
Copy link
Owner Author

rwz commented Apr 6, 2012

Release? :)

@justinfrench
Copy link
Contributor

@rwz I've had a play around, but I've forgotten the main difference in @sj26's fork — can you give me a refresher with a quick gist example?

@sj26
Copy link
Contributor

sj26 commented Apr 10, 2012

The difference was that you could nest in layouts as well as in views, the original prototype only rendered the top-most layout containing the bottom-most view.

@justinfrench
Copy link
Contributor

@sj26 can you give me a concrete "this didn't work, now it does" gist? I seem to be able to do this in the current gem, but am probably just misunderstanding :)

@rwz
Copy link
Owner Author

rwz commented Apr 11, 2012

@justinfrench something like that: https://gist.github.com/2357403

Basically, you can extend layout that is already an extension of some other layout and so on. this was not possible before @sj26 patches.

@sj26
Copy link
Contributor

sj26 commented Apr 11, 2012

Correct. Before the patch you could nest views, but not layouts.

@justinfrench
Copy link
Contributor

@sj26 @rwz this is really surprising to me, i must be missing something. I'm trying to come up with the most minimalist test case for what you're trying to describe. Is this gist it?

https://gist.github.com/2362758

There's two samples there, one with a layout specified in the controller, one in the view. In both cases, the layout is "blog" and "blog" extends "application". Pretty damn sure that works just fine, correct me if I'm wrong :)

So, trying to figure out what I'm missing... @rwz's gist had a yield in it — is that it? In all my examples, I don't use yield — I always declare an area then put content into it, rather than having a default content area for whatever gets dropped into the view without an append/replace/prepend.

Assuming that's it, can one of you fork my gist and add the smallest amount of change possible to show what's now supported? It might also help to know exactly which specs are testing this new feature.

Sorry I'm slow on this, but my head has been deep inside two big apps without nestive for months and months :)

@rwz
Copy link
Owner Author

rwz commented Apr 12, 2012

I don't know what to say here, really. I prefer default rails approach with yield and it works as expected with patches.

Every layout i use is an extension of some other parent layout with small changes and additions. I usually do 3 to 6 extensions for some inner layouts. They all are inherited from most basic layout with only head and body tags in it.

That approach does not work without the patch since it tries to render only top level layout inside bottom level layout.

@rwz
Copy link
Owner Author

rwz commented Apr 12, 2012

This stuff is covered in can extend already extended layouts spec btw. The spec fails without patches.

@sj26
Copy link
Contributor

sj26 commented Apr 12, 2012

Here's an example rails repo.

Broken:

Broken

Fixed:

Fixed

(the extends path change is just a nicety, not a significant thing.)

@justinfrench
Copy link
Contributor

@rwz that's completely cool with me — I'm not trying to back out the change, I'm trying to understand the benefit and get reacquainted with my own gem before imposing it on others :) Now that I know exactly what stuff to play with, should be no problem at all.

@sj26 cheers!

@rwz
Copy link
Owner Author

rwz commented Apr 26, 2012

Ship!

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

3 participants