Sitepress does not seem to work with Phlex based layouts

[johnmcdowall]

Hello - thanks for the super cool gem!

Just trying it out on a new Rails 7.1 app, and following the docs for installation of Sitepress and Phlex. Default Sitepress routes are in play in the routes.rb file.

I set my ApplicationController to use the ApplicationLayout view as shown in the docs:

class ApplicationController < ActionController::Base
  layout -> { ApplicationLayout }

  ...snip...
end

And indeed I have an application_layout.rb file under app/views/layouts/appliation_layout.rb, which contains the following:

class ApplicationLayout < Phlex::HTML
  include Phlex::Rails::Layout

  def template
    render BaseLayout.new do
      main do
        render("shared/flash")
        yield
      end
    end
  end
end

But straight away I'm getting a Template is missing exception saying the ApplicationLayout can't be found:

Maybe this is a bug with using Sitepress?

I see this in the stack trace:

# Renders the markup within a resource that can be rendered.
def page_rendition(resource, layout: nil)
  Rendition.new(resource).tap do |rendition|
    rendition.layout = layout
    pre_render rendition
  end

Relevant section of stack trace:

[rails (a6099ed2b7a5) actionpack/lib/action_controller/metal/rendering.rb:147:in `render_to_string'](http://localhost:3000/#)
[sitepress-rails (4.0.2) rails/app/controllers/concerns/sitepress/site_pages.rb:68:in `pre_render'](http://localhost:3000/#)
[sitepress-rails (4.0.2) rails/app/controllers/concerns/sitepress/site_pages.rb:48:in `block in page_rendition'](http://localhost:3000/#)
[<internal:kernel>:90:in `tap'](http://localhost:3000/#)
[sitepress-rails (4.0.2) rails/app/controllers/concerns/sitepress/site_pages.rb:46:in `page_rendition'](http://localhost:3000/#)
[sitepress-rails (4.0.2) rails/app/controllers/concerns/sitepress/site_pages.rb:57:in `render_resource_with_handler'](http://localhost:3000/#)
[sitepress-rails (4.0.2) rails/app/controllers/concerns/sitepress/site_pages.rb:38:in `render_resource'](http://localhost:3000/#)
[sitepress-rails (4.0.2) rails/app/controllers/concerns/sitepress/site_pages.rb:28:in `show'](http://localhost:3000/#)
[rails (a6099ed2b7a5) actionpack/lib/action_controller/metal/basic_implicit_render.rb:6:in `send_action'](http://localhost:3000/#)

Seems like Sitepress isn't expecting a callable constant as a Layout?

[johnmcdowall]

Yep, modifying the page_rendition method to do this:

        rendition.layout = layout.constantize.call

Gets past that exception, but then it seems that no helpers are available in the layout context, as I now get an exception about a helper that should be available (and is in non-sitepress based pages) isn't:

[johnmcdowall]

Here's a fresh repro repo, each step in the process is a commit: https://github.com/johnmcdowall/sitepress-phlex-layout-bug

[bradgessler]

Here's how I got this working in one of my projects:

class SiteController < Sitepress::SiteController
  layout false

  protected

  def pre_render(rendition)
    rendition.output = render_to_string(layout do
      render_to_string inline: rendition.source, type: rendition.handler
    end)
  end

  def layout(&)
    # You might have a different class name.
    PageView.new \
      title: current_page.data.fetch("title"),
      subtitle: current_page.data["subtitle"],
      &
  end
end

I'm still thinking through the pre-render API because there's cases where people want to render a page within a page to extract bits of it via Nokogiri, etc. Curious what feedback you have to make it clearer. I think the code snippet above isn't super clear, but it does work.

One problem with this approach is the layout frontmatter key won't work as you'd expect. I think in the world of Phlex the layout key has to refer to a Phlex component.

For example, in Rails with Erb files, a page might have this:

title: Privacy policy
layout: legal

But in Phlex it might look like this:

title: Privacy policy
layout: LegalComponent

I think that's OK? But something feels off about it, especially if people are running Sitepress inside of a Rails app with both Phlex and Erb.

[johnmcdowall]

I guess it depends on what you, the lib author, want the behaviour to be. Do you specifically want to force Phlex users to eject and customize, or do you want to bake magic into Sitepress?

My PR makes it magical and just works, but I can see a case for either way to be honest.

With regards to the layout key in the frontmatter, changing the layout to be a properly cased constant made total sense to me, I didn't even have to think about it. It was just what I did after installing Phlex. Installing Phlex and internalizing classes instead of view got me there. So I think if you're the type to install Phlex, you'll get that the layout should now be a class name, not a string for a file name.

Personally I'd rather not have to eject SitePress just for this, I'd expect it to work out of the box, and I'd rather not have to learn about pre_render etc.

[johnmcdowall]

Btw my PR will fall back to expected ERB lookup behaviour if the layout is not actually a Phlex component. So it covers all bases, even if you are using ERB and Phlex in the same project.

[bradgessler]

I was playing around with this today, tired layout: BlahComponent and didn't like it because the approach doesn't provide access to a constructor. The good news is I think I found an approach that I like.

Let's say a page has this frontmatter:

---
title: Blah
layout: article
---

This would look up the article_layout method on the SiteController. As you'll see in the prototype implementation below, it finds the method and renders whatever is in that method.

class SiteController < Sitepress::SiteController
  layout false

  protected
    def article_layout(page)
      ArticleView.new title: page.data.fetch("title"), subtitle: page.data["subtitle"] do
        render_resource_inline page
      end
    end

    def page_layout(page)
      PageLayout.new title: page.data.fetch("title"), subtitle: page.data["subtitle"] do
        render_resource_inline page
      end
    end

  private
    def render_resource_with_handler(resource)
      render layout_component(resource)
    end

    def render_resource_inline(resource)
      render inline: resource.body, type: resource.handler
    end

    def layout_component(resource)
      # rendition.resource.data.fetch("layout", "PageView").constantize
      method_name = resource.data.fetch("layout", "page").concat("_layout")
      layout_method = self.method(method_name)
      layout_method.call resource
    end
end

I think this approach could be used with Erb renditions, have sensible fallbacks, and remain performant.

Using Phlex would require ejecting the controller to have a place to add the view constructors, but that's fine. I also think the ejected controller would have the Phlex integration packaged up into a module so it would look like this:

class SiteController < Sitepress::SiteController
  layout false
  include Sitepress::Phlex

  protected
    def article_layout(page)
      ArticleView.new title: page.data.fetch("title"), subtitle: page.data["subtitle"] do
        render_resource_inline page
      end
    end

    def page_layout(page)
      PageLayout.new title: page.data.fetch("title"), subtitle: page.data["subtitle"] do
        render_resource_inline page
      end
    end
end

There's some other cleanup too, like the render_resource_inline page should try to look and feel more like Rails render method. For example, render_resource_inline page would be render_resource inline: page

[johnmcdowall]

[bradgessler]

Started work on this in branch https://github.com/sitepress/sitepress/tree/simplify-renderer