Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

`:layout => !request.xhr?` doesn’t fallback to the custom layout option #563

Closed
EtienneLem opened this Issue · 10 comments

3 participants

@EtienneLem
module MyApp
  class App < Sinatra::Base
    set :erb, :layout => :'layouts/application'

    get '/does-not-work' do
      erb :view, :layout => !request.xhr?
      #=> No such file or directory - myapp/views/layout.erb
    end

    get '/does-work' do
      erb :view
      #=> Render with myapp/views/layouts/application.erb layout
    end

    get '/does-work-too' do
      if request.xhr?
        erb :view, :layout => false
      else
        erb :view
      end
    end

  end
end

Or maybe I’m missing something and it’s meant to be that way?

@EtienneLem

FWIW; erb :view, :layout => true also tries to use the myapp/views/layout.erb layout instead of the one set in the options

@zzak
Collaborator

You could try something like: erb :view, :layout => :'layouts/application' unless request.xhr?.

Even set the :'layouts/application' to a constant or something.

@EtienneLem

Yeah, workarounds are pretty easy.
But it just seems wrong that :layout => true ignores the layout option

@rkh
Owner

Agreed. Let's change this for 1.4.

@zzak
Collaborator

I believe this patch covers the test case:

diff --git a/test/templates_test.rb b/test/templates_test.rb
index e426d32..91f8572 100644
--- a/test/templates_test.rb
+++ b/test/templates_test.rb
@@ -34,6 +34,14 @@ class TemplatesTest < Test::Unit::TestCase
     File.unlink(layout) rescue nil
   end

+  it 'falls back to default layout' do
+    with_default_layout do
+      render_app(:layout => :layout2) { render(:test, :hello, :layout => true) }
+      assert ok?
+      assert_equal "Layout 2!\nHello World!\n", body
+    end
+  end
+
   it 'renders String templates directly' do
     render_app { render(:test, 'Hello World') }
     assert ok?
@rkh
Owner

Yes, thanks!

@zzak
Collaborator

See #574

@zzak
Collaborator

@EtienneLem Could you test this again against master? Should be fixed by d8f1d8e

@EtienneLem

Oh! It does. Thanks & good job :wink:

@zzak
Collaborator

@EtienneLem Mind closing this then?

Thanks!

@EtienneLem EtienneLem closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.