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

Pre-RFC: Splat Areas #517

Open
Tracked by #816
knownasilya opened this issue Jul 26, 2019 · 15 comments
Open
Tracked by #816

Pre-RFC: Splat Areas #517

knownasilya opened this issue Jul 26, 2019 · 15 comments

Comments

@knownasilya
Copy link
Contributor

knownasilya commented Jul 26, 2019

Based on https://mobile.twitter.com/knownasilya/status/1152293795901464577

With the addition of ...attributes, element modifiers and having multiple top elements in a glimmer component, it has become obvious that ...attributes is too limited (I'm sure the API was mean't to be simple while patterns emerge). Think of a component that wraps an input and a label into a nice package. Currently you'd need to either have the user yield out subcomponents and recreate the same structure or provide custom arguments for each element/component.

What if we could expose elements/components for customization by name, for example:

<UserCard
  @user={{@user}}
  {{on 'click' this.openProfile}}
  [avatar
    class='avatar-sm'
    @size='small'
    {{on 'click' this.openAvatarModal}}
  ]
/>

And the user card would apply it like so:

<div ...attributes class='container'>
  <Avatar @img={{@user.avatar.url}} ...[avatar] class='avatar' />
  {{! more stuff here }}
</div>
@buschtoens
Copy link
Contributor

buschtoens commented Jul 26, 2019

Side note: can you amend your snippet with "root" attributes, that would apply to ...attributes in .container?

Edit: I can't read. L3 ({{on 'click' this.openProfile}})) is a "root" splattribute.

@buschtoens
Copy link
Contributor

I hit the same limitation as well... A currently possible solution (for many cases) is to yield the sub-components:

<UserCard
  @user={{@user}}
  {{on 'click' this.openProfile}}
  as |uc|
>
  <uc.Avatar
    class='avatar-sm'
    @size='small'
    {{on 'click' this.openAvatarModal}}
  />
  {{! more stuff here }}
</UserCard>
<div ...attributes class='container'>
  {{yield (hash
    Avatar=(component 'avatar' img=@user.avatar.url classNames='avatar')
  )}}
</div>

I do not think that this ⬆️ is a good solution.

  1. The component consumer needs to nest the yielded components in the correct order and has to remember to yield all required components.
  2. Components that would otherwise only consume data and element modifiers, suddenly consume a nested block and HTML structure.
  3. Passing HTML attributes to the {{component}} helper is still awkward, even with ember-component-attributes, and sometimes not even possible: What if Avatar is a `GlimmerComponent? Do I smell an RFC?

@buschtoens
Copy link
Contributor

Have you considered merging this with yieldable blocks somehow?

@knownasilya
Copy link
Contributor Author

knownasilya commented Jul 26, 2019

@buschtoens I have thought about a possible way, but nothing stands out.
Something like this might work..

<UserCard
  @user={{@user}}
  {{on 'click' this.openProfile}}
  [:avatar
    class='avatar-sm'
    @size='small'
    {{on 'click' this.openAvatarModal}}
  ]
/>

and

<div ...attributes class='container'>
  <Avatar @img={{@user.avatar.url}} {{yield to='avatar}} class='avatar' />
  {{! more stuff here }}
</div>

But that seems to diverge a bit from ...attributes, which this seems more like.

@ef4
Copy link
Contributor

ef4 commented Jul 26, 2019

While I know the use cases you have in mind are real and I have encountered them too, I have always eventually come to regret making a component that's big enough to have multiple internal elements that users want to customize the markup of.

Also, I suspect named blocks can do the same things as this proposal.

@knownasilya
Copy link
Contributor Author

I would love for named blocks to support this, but afaict they cannot be used as modifiers.

The easiest examples of components that really need this pattern are form fields that combine input and label.

@jgwhite
Copy link
Contributor

jgwhite commented Nov 18, 2019

@knownasilya thanks so much for raising this topic. It’s great that you’re exploring this space.

I have a little nitpick with your example: having the {{on 'click' on both the parent and child element means you have a button-within-a-button situation. Not great for accessibility, but also a potential source of bugs — without special handling, clicking the avatar would also trigger openProfile. This is probably not a situation the component author would want to allow.

It may seem like splitting hairs, but I raise this because I think the relatively restrictive nature of ...attributes might actually help protect us from these sorts of problems.

@knownasilya
Copy link
Contributor Author

knownasilya commented Nov 18, 2019

Agree on the issue, but seems like we have that today.

<label ...attributes>
  <input ...attributes>
<label>
<Field {{on "click" this.open}}/>

Not sure if there could be a way to exclude fields or error if the same event is nested.

@jgwhite
Copy link
Contributor

jgwhite commented Nov 18, 2019

Haha, I didn’t even realise you could use ...attributes twice in the same component.

@pzuraq
Copy link
Contributor

pzuraq commented Nov 19, 2019

As a sidenote, this is one reason I don't really think it's a good idea to use {{on}} to add event listeners to components, unless you're adding an event listener which really isn't a concern of the component, and should not be part of its public API.

For instance, in these examples, if UserCard and Avatar should both be able to respond to user input, and that's one of their primary use cases, then I would personally expose a @onClick and @onAvatarClick type method (I'd also question the double click handler like @jgwhite did, but let's assume for the moment it's correct behavior).

Fundamentally, ...attributes exposes internal implementation details of your component such as the HTML structure. This is useful in a lot of cases (e.g. adding CSS classes, which needs to happen a lot, id, and various other attributes), but relying on it too heavily could make your components very inflexible and difficult to refactor down the line, IMO.

@chriskrycho
Copy link
Contributor

One thing this calls out (which I've been talking about with folks as we look at how to design components around these kinds of APIs going forward) is that add-ons should document the expectations of ...attributes.

@knownasilya
Copy link
Contributor Author

Agreed @pzuraq and @chriskrycho.

Maybe the solution is something like

<label ...attributes>
  <input ...{{attributes @inputAttributes}}>
<label>
<Field @inputAttributes={{hash
  name="inputname"
}}/>

where modifiers are not allowed.

@lolmaus
Copy link

lolmaus commented Mar 29, 2020

Here's a great case for Splat Areas, which, I believe, has been a source of frustration for many Ember developers: cibernox/ember-power-select#1203

@Gaurav0
Copy link
Contributor

Gaurav0 commented Mar 31, 2020

@knownasilya I really like the updated proposal. Especially since it doesn't introduce some new syntax characters [] to handlebars.

@wagenet
Copy link
Member

wagenet commented Jul 22, 2022

I'm leaving the open for now since it is referenced in @NullVoxPopuli's meta issue.

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

No branches or pull requests

9 participants