phlex-ruby / phlex

A framework for building object-oriented views in Ruby.
https://beta.phlex.fun
MIT License
1.28k stars 83 forks source link

feat: add Hash support for `class` and `style` attributes #737

Closed ElMassimo closed 4 months ago

ElMassimo commented 4 months ago

Description :book:

This pull request adds support for passing Hash in class and style attributes in a way that feels idiomatic in Ruby and achieves composability.

Background đź“ś

When writing Phlex components, things usually start simple:

div(id: "hero", class: "section")

but when you need a conditional class, things get a bit awkward:

div(id: "hero", **classes("section", ("active" if active)))

This proposal is meant to provide an alternative which is easier to understand, and easier to write:

div(id: "hero", class: ["section", active:])

Examples 🧪

class

A component such as:

def active = true

def inactive = false

def view_template
  div(class: ["dropdown", inactive:, active:])
end

would render:

<div class="dropdown active"></div>

style

A component such as:

def view_template
  div(style: {font_size: "16px", opacity: 0})
end

would render:

<div style="font-size:16px;opacity:0;"></div>

Precedents

This feels like the Ruby equivalent of what frameworks like Vue provide in their template system:

As noted by @Spone, this behavior is consistent with the class_names helper in Rails.

joeldrapper commented 4 months ago

I really like the look of this!

joeldrapper commented 4 months ago

@bradgessler this makes your idea in #720 even more useful.

bradgessler commented 4 months ago

This syntax has never really jived with my expectations—I think it's too clever.

For example, what does this return?

def active = true

def inactive = false

def priority = %[low medium high].sample

def view_template
  active = false
  inactive = true
  div(class: ["dropdown", inactive:, active:, priority:])
end

I find it surprising that it binds to the value of a method and shows the class, or not, if its true or false.

Instead I'd propose methods that make it clearer what these things are doing. Here's an idea if method is used:

def active = true

def inactive = false

def priority = %[low medium high].sample

def view_template
  # This would inflect on the Method class. Token is Method#name and if the return
  # value is a string, it would populate with that string. If it's boolean it would return
  # Method#name token if it's true. Otherwise nothing if its empty.
  div(class: ["dropdown", method(:active), method(:inactive), method(:priority))]
end

If I'm reading this code it's much clearer to me, "Oh, this is the :active method". I can even copy and paste the code into my debugger to look at it.

There could probably be some Phlex helper methods that dress this up a bit more, but I think at a minimum using the already existing method method would improve readability and prevent this API from being painted into a corner.

hangsu commented 4 months ago

+1 for style Hash support. We've used a custom implementation at Clearscope for some time now.

I tend to agree with @bradgessler's sentiments around class. We've always used arrays of strings and have never been tempted to do more:

def view_template
  div(class: ["btn", ("active" if active?)])
end

def active? = true

# OR

def view_template
  div(class: ["btn", active_class])
end

def active_class = "active" if condition?
joeldrapper commented 4 months ago

@bradgessler I think the example wasn’t clear because it used the same class name as conditional. It’s not calling the key as a method. I think this might be a better example:

div(class: ["rounded", "bg-blue color-white font-bold" => @active])

Read => above as “if”.

Spone commented 4 months ago

Quick note: this is similar to the token_list Rails helper (aliased class_names) which is in my experience quite easy to work with.

bradgessler commented 4 months ago

@bradgessler I think the example wasn’t clear because it used the same class name as conditional. It’s not calling the key as a method. I think this might be a better example:

div(class: ["rounded", "bg-blue color-white font-bold" => @active])

Read => above as “if”.

Oh I see, so the "magic method" stuff is gone since the tokens helper is removed? What you propose is much more sane and less surprising since the behavior is congruent to how it works for HTML attributes.