Fix problems referencing partial 'global/nav' from layout #103

Closed
wants to merge 1 commit into
from

Projects

None yet

1 participant

@timshadel

Layout didn't get the app id appended to it, so referencing a partial
with a name 'global/nav' resulted in the monkeypatching expecting that
the app id was 'global'. Additionally, even if the monkeypatched vesion
received a valid raw path, the raw version was never attempted.

This is fixed in two ways:

  • First, we add the app id to the layout sent to any 'render' call except for layout: false
  • Second, we add a fallback to the monkeypatching where we simply try what express would do with the raw path parameter

The tests show both problems.

Of note is the fact that the problem can be fixed by #2 alone, but then
it always checks the filesystem twice. The change for #1 avoids that.

@timshadel timshadel Fix problems referencing partial 'global/nav' from layout
Layout didn't get the app id appended to it, so referencing a partial
with a name 'global/nav' resulted in the monkeypatching expecting that
the app id was 'global'. Additionally, even if the monkeypatched vesion
received a valid raw path, the raw version was never attempted.

This is fixed in two ways:

  * First, we add the app id to the layout sent to any 'render' call
    except for `layout: false`
  * Second, we add a fallback to the monkeypatching where we simply try
    what express would do with the raw path parameter

The tests show both problems.

Of note is the fact that the problem can be fixed by #2 alone, but then
it always checks the filesystem twice. The change for #1 avoids that.
56c4dc4
@shimaore shimaore referenced this pull request in zappajs/zappajs Apr 22, 2012
Closed

Backporting maurimach/zappa's pull requests #8

@shimaore shimaore added a commit to zappajs/zappajs that referenced this pull request Apr 24, 2012
@shimaore shimaore Fixed problem referencing 'global/nav' from layout.
Applied solution from @timshadel in mauricemach/zappa#103
ae8403e
@timshadel

See comments on zappajs/zappajs#8 (needs tests + doc). Closing since I won't get around to that work.

@timshadel timshadel closed this Jul 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment