ef4 / ember-overlays

Annotated overlays for Ember apps.
MIT License
12 stars 10 forks source link

question about getViewBounds #10

Open samselikoff opened 5 years ago

samselikoff commented 5 years ago

I've seen getViewBounds in several of your addons. Is this one of Ember's "intimate APIs", that will almost certainly have an upgrade path if/when it changes?

ef4 commented 5 years ago

Yes, they’re pretty safe.

For all but the most esoteric use cases I think element modifiers can give you the same information without using private API. I should probably update to that where possible.

We have talked about publicizing these, probably under a name that isn’t a throwback to the views days.

The main challenge with using them is that they are a snapshot in time, they don’t update. Though that is also why they’re quite safe: it’s cheap to expose this information.

ef4 commented 5 years ago

I forgot to add: one reason these are stable is that the ember inspector needs them.

ef4 commented 5 years ago

Somebody should do an RFC to make this API public, the only sticking point is figuring out how this kind of pro API fits into the teaching story.

samselikoff commented 5 years ago

Cool. Thanks for all the info.

In my case I'm using them to make a <DropdownList> component completely renderless:

<DropdownList as |List|>
  <ul class='flex'>
    <List.item as |Item|>
      <li class='w-64 {{if Item.isActive 'bg-yellow'}}>
        Item A
      </li>
    </List.item>
  </ul>
</DropdownList>

Both <DropdownList> and its contextual item are tagless, but use getViewBounds to (1) ensure there's only one child in the root of the yielded content, and (2) attach & remove event listeners, wiring up the isActive state on the items. So in this case, I don't think element modifiers would help.

There's other situations I've wanted to be able to "iterate over children" too.

ef4 commented 5 years ago

A thing I'm going to encourage at least in ember-animated's docs is using angle invocation for components that truly represent elements, and curly for ones that don't. In a situation like

<AnimatedContainer>
  {{#animated-if showThing}}
    <div>Thing</div>
  {{/animated-if}}
</AnimatedContainer>

you can see the real DOM structure, because AnimatedContainer generates an element and animated-if does not. {{#animated-if}} produces the same results as {{#if}} so it's good that it looks the same. I want people to think of animated-if and animated-each as falling into the same cateogry as if and each.

I think if you have a strong requirement that a tagless component must have exactly one child, then it's a hint that tagless is not a good idea. It has recently become much easier to let people customize the DOM you are providing for them, so that is no longer a good reason to go tagless.

This

<DropdownList as |List|>
  <ul class='flex'>

Could just be

<DropdownList class='flex' @tagName='ul' as |List|>

with no loss of generality. Even element modifiers like

<DropdownList {{on "click" something}} as |List|>

are supported in angle invocation, as long as your component sets ...attributes somewhere.

samselikoff commented 5 years ago

Hmm...

<DropdownList class='flex' @tagName='ul' as |List|>

Do/will Glimmer components support @tagName? I thought these were exactly the APIs we're trying to get away from.

I like your convention in Ember Animated but also don't necessarily see the problem with doing some validation on what kind of children are passed into a tagless component. I think

<DropdownList as |List|>
  <ul class='flex'>

generally feels way cleaner & more composable than using @tagName. Here's a more complete example and I like that the static DOM pieces are clearly identifiable just by looking at the template.

samselikoff commented 5 years ago

Of course, I didn't mention the one case that actually motivated this to begin with:

<List.item as |Item|>
  <li class='w-64 p-4 {{if Item.isActive "bg-yellow"}}'>
    Item 2
  </li>
</List.item>

In this case, my <li> can use Item.isActive to customize its appearance, because it is in lexical scope of the yielded variable. In the case of

<DropdownList class='flex' @tagName='ul' as |List|>

that is not true, leading to strange APIs like <DropdownList @activeClass='bg-yellow'>. So I think there actually is a loss of generality.

ef4 commented 5 years ago

What we're trying to get away from is making component authors use tagName just to customize their own element. Letting callers customize your components tag is a different beast.

I agree that @tagName is slightly ugly, but can be worth the tradeoff. For AnimatedContainer I think it's a definite win, because making people type two levels of elements every time is more confusing. Especially when only one of them will result in a real element.

Your example using lexical scope is a good argument for why in some cases it's more powerful.