Skip to content

[WIP][TwigComponent] add component attribute system/helper (alternative) #229

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

Closed
wants to merge 1 commit into from

Conversation

kbond
Copy link
Member

@kbond kbond commented Jan 19, 2022

Q A
Bug fix? no
New feature? yes
Tickets n/a
License MIT

This is an alternative to #220. This version makes attributes a primary feature (doesn't require a trait to use). In your components, an attributes object is always available. Properties passed to the component twig function that cannot be mounted to your component object are added to this object. This pattern is well established in blade/vue.

I haven't given a huge amount of thought yet to LiveComponent's. I'm thinking the attributes passed to the twig function will be passed back and forth via http with a special proprietary attribute.

This will bring some BC breaks:

  • Properties passed to the component twig function that do not exist on your component class will no longer throw an exception. Instead they are added to the attributes object. Since attributes are required to be string/scalar I could throw an exception if the passed extra values are not.
  • Using ComponentFactory::create() directly (unlikely scenario imo). This method now returns a ComponentContext object instead of the component object.
  • Assuming we release this feature to both TwigComponent and LiveComponent at the same time, if someone upgrades one without the other, there will be problems.

These seem fairly minor to me (we are still experimental) but any ideas to mitigate the above would be appreciated!

TODO

  • Tests
  • LiveComponent support
  • Documentation
  • Finalize ComponentAttributes methods

Usage

{# templates/components/my_component.html.twig #}

<div{{ attributes }}>
  My Component!
</div>

{# render component #}

{{ component('my_component', { class: 'foo', style: 'color:red' }) }}

{# renders as: #}
<div class="foo" style="color:red">
  My Component!
</div>

Defaults

Set default attributes that can be fully overridden by passed attributes

{# templates/components/my_component.html.twig #}

<div{{ attributes.defaults({ class: 'bar' }) }}>
  My Component!
</div>

{# render component #}
{{ component('my_component', { style: 'color:red' }) }}
{{ component('my_component', { class: 'foo', style: 'color:red' }) }}

{# renders as: #}
<div class="bar" style="color:red">
  My Component!
</div>
<div class="foo" style="color:red">
  My Component!
</div>

Merging Defaults

Set defaults but allow them to be appended to by passing these values to the component function:

{# templates/components/my_component.html.twig #}

<div{{ attributes.merge({ class: 'bar' }) }}>
  My Component!
</div>

{# render component #}
{{ component('my_component', { class: 'foo', style: 'color:red' }) }}

{# renders as: #}
<div class="bar foo" style="color:red">
  My Component!
</div>

Only

{# templates/components/my_component.html.twig #}

<div{{ attributes.only('class')) }}>
  My Component!
</div>

{# render component #}
{{ component('my_component', { class: 'foo', style: 'color:red' }) }}

{# renders as: #}
<div class="foo">
  My Component!
</div>

Without

{# templates/components/my_component.html.twig #}

<div{{ attributes.without('class')) }}>
  My Component!
</div>

{# render component #}
{{ component('my_component', { class: 'foo', style: 'color:red' }) }}

{# renders as: #}
<div style="color:red">
  My Component!
</div>

@weaverryan
Copy link
Member

About BC (which we don't need to guarantee at this point, but we should be thoughtful about if breaking):

Properties passed to the component twig function that do not exist on your component class will no longer throw an exception. Instead they are added to the attributes object. Since attributes are required to be string/scalar I could throw an exception if the passed extra values are not.

Looks minor to me. Adding a check and an exception seems good, but we should probably allow for Stringable objects? Or should we be safer and stricter and only allow scalars (that might be better).

Using ComponentFactory::create() directly (unlikely scenario imo). This method now returns a ComponentContext object instead of the component object.

If we stick with this ComponentContext idea, this is indeed a BC break, but needed.

Assuming we release this feature to both TwigComponent and LiveComponent at the same time, if someone upgrades one without the other, there will be problems.

We should be able to manage this with the required version of twig-components from live-components, right?

@kbond
Copy link
Member Author

kbond commented Jan 19, 2022

We should be able to manage this with the required version of twig-components from live-components, right?

If we release this feature in say, 2.1 of both packages, we can ensure live requires a compatible version of twig but not the other way, right? The BC break will be if someone is using 2.1 of twig and 2.0 of live (which is valid)... I think.

@kbond kbond force-pushed the ux-twig-attributes-2 branch from 27b8f57 to abbb524 Compare January 19, 2022 22:43
@kbond
Copy link
Member Author

kbond commented Jan 20, 2022

Closing in favor of #220. After discussion with @weaverryan, we decided this method adds too much complexity and BC breaks/problems.

@kbond kbond closed this Jan 20, 2022
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