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

question about getViewBounds #10

Open
samselikoff opened this issue Jan 29, 2019 · 8 comments
Open

question about getViewBounds #10

samselikoff opened this issue Jan 29, 2019 · 8 comments

Comments

@samselikoff
Copy link

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
Copy link
Owner

ef4 commented Jan 30, 2019

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
Copy link
Owner

ef4 commented Jan 30, 2019

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

@ef4
Copy link
Owner

ef4 commented Jan 30, 2019

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
Copy link
Author

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
Copy link
Owner

ef4 commented Jan 31, 2019

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
Copy link
Author

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
Copy link
Author

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
Copy link
Owner

ef4 commented Jan 31, 2019

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.

0xadada pushed a commit that referenced this issue Jan 31, 2020
* update ember-concurrency

* ran ember-cli-update

* update yarn.lock

* patching up readme after merge

* sure use yarn

* releasing

* Fix installation command

* ember-cli-update

* codemods

* factoring out some uses of jquery

* removing jquery usage in tests

* dep updates and deprecation cleanup

* drop jquery scenario because we always still use jquery

* releasing

Co-authored-by: Edward Faulkner <[email protected]>
Co-authored-by: vladucu <[email protected]>
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

2 participants