hyper-react icon indicating copy to clipboard operation
hyper-react copied to clipboard

components should accept any number of params to be merged

Open sollycatprint opened this issue 8 years ago • 5 comments

From @catmando on January 6, 2016 23:54

for example

  div(attributes, {more: :stuff}, :big, id: "hello", style: {display: :none})

should be the same as

  div(attributes.merge {more: :stuff, big: true, id: "hello, style: {display: none})

Precisely - all args that are hashes should be merged, any non hash arg should be added to the merged hash with a value of true.

Why? because sometimes you have a ready made set of params (i.e. attributes passed to a component) and you want to merge in others.

this is how you process the attributes (i just wanted to make sure it would work)

  args.inject({}) do |h, item|
    if item.is_a? Hash 
      h.merge item
    else
      h[item] = true
      h
    end
  end

Copied from original issue: zetachang/react.rb#114

sollycatprint avatar Jun 13 '16 13:06 sollycatprint

From @ajjahn on January 8, 2016 15:28

@catmando Can you provide a more complete example, so I can be sure I'm following the motivation for this?

sollycatprint avatar Jun 13 '16 13:06 sollycatprint

From @catmando on January 8, 2016 21:35

From our code base. This is the actual example that trigged this issue

          def render
              div do
                button(params.attributes.merge({id: "#{job.finishing.id}_trigger_button"})) do  
           #   fixing this would allow us to write: 
           #   button(params.attributes, id: "#{job.finishing.id}_trigger_button")
                  "Advanced Options"
                end.on(:click) do
                  Element["#finishing_options_modal"].modal('show')
                end
                dialog_box
              end if has_advanced_options?
          end

once you allow the first parameter to be a hash to be merged with the rest, you might as well generalize and allow any combination of hashes, and strings.

sollycatprint avatar Jun 13 '16 13:06 sollycatprint

@zetachang nice easy one I think!

catmando avatar Sep 07 '16 01:09 catmando

Just FYI, the corresponding JSX equivalent is props spread.

https://facebook.github.io/react/docs/jsx-spread.html

zetachang avatar Sep 07 '16 14:09 zetachang

yes exactly, slight semantic differences, based on ruby verses ecmas6, but yes we should do this.

catmando avatar Sep 09 '16 01:09 catmando