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

Support Parameterised Layouts #192

Merged
merged 3 commits into from
Apr 4, 2024
Merged

Conversation

joeldrapper
Copy link
Collaborator

@joeldrapper joeldrapper commented Apr 2, 2024

With this change, you will be able to use parameterised layouts, e.g.

layout -> { ApplicationLayout.new(theme: :blue, show_header: false) }

For more details on why you might want to do this, see https://jardo.dev/adding-parameterized-layouts-to-phlex-rails

Todo

  • Because of the way I’ve implemented this, the current tests already exercise this code, however, it would be good to add a test that uses parameterised layouts

@joeldrapper joeldrapper changed the title Support Parameterized Layouts Support Parameterised Layouts Apr 2, 2024
@@ -60,5 +44,30 @@ def self.included(klass)

klass.extend(Interface)
end

def render(view_context, *args, **kwargs, &block)
if @_context
Copy link
Collaborator Author

@joeldrapper joeldrapper Apr 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If @_context is set, we're already rendering and this should not be treated as an external call to render the layout. It should do the normal render behaviour instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably define a method for this, e.g.

def rendering?
  !!@_context
end

output = view_context.capture(&block)
end

unsafe_raw output
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should return nil after pushing with unsafe_raw.

@joeldrapper joeldrapper marked this pull request as ready for review April 4, 2024 16:42
@joeldrapper joeldrapper merged commit c11650d into main Apr 4, 2024
13 checks passed
@joeldrapper joeldrapper deleted the better-layouts-non-breaking branch April 4, 2024 16:42
@Burgestrand
Copy link
Contributor

Burgestrand commented Apr 17, 2024

Hi!

I like this change. I needed something like this in the past.

However, it appears to have broken this kind of code:

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

  def view_template
    html do
      body do
        render partial: "tracking" # boom
      end
    end
  end
end

It raises:

     ArgumentError:
       wrong number of arguments (given 0, expected 1+)
     # /Users/kim/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/phlex-rails-1.2.1/lib/phlex/rails/layout.rb:48:in `render'

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.

2 participants