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

Expose a parentElement property for (tagless) components #168

Closed
simonihmig opened this issue Sep 21, 2016 · 36 comments
Closed

Expose a parentElement property for (tagless) components #168

simonihmig opened this issue Sep 21, 2016 · 36 comments

Comments

@simonihmig
Copy link
Contributor

Components have an element property that gives you the DOM element of the component itself. Getting its parent is pretty easy, using element.parentElement or this.$().parent(). But these approaches fail when dealing with tagless components, which have no element and no this.$() either. Obviously, they still have a parent though!

So far I have used this utility function:

import Ember from 'ember';

const { get, $ } = Ember;

export default function getParent(view) {
  if (get(view, 'tagName') === '') {
    // Beware: use of private API! :(
    if (Ember.ViewUtils && Ember.ViewUtils.getViewBounds) {
      return $(Ember.ViewUtils.getViewBounds(view).parentElement);
    } else {
      return $(view._renderNode.contextualElement);
    }
  } else {
    return view.$().parent();
  }
}

This seems to work for all Ember versions (pre and post Glimmer2), but is obviously pretty hacky as it is using private APIs. And there does not seem to be any way to get the parent element for a tagless component using just public APIs, unless I am overlooking something!?

So my suggestion would be to introduce a parentElement property, in analogy to the element property.

An example use case: I want to make a tooltip component, that should be used like this:

<button>
  Create
  {{bs-tooltip title="Just create something"}}
</button>

The tooltip component must have access to its parent element, to attach event listeners to it, but should not (until the tooltip is actually shown) add anything to the DOM. Thus a tagless component.

Any thoughts?

@rwjblue
Copy link
Member

rwjblue commented Sep 21, 2016

As you discovered, there is private API in Ember 2.9+ that allows access to a components bounds (including parentElement). This support was added for glimmer2 support in Ember Inspector.

I think we would definitely consider making that method (Ember.ViewUtils.getViewBounds) public API, but I doubt we would want to add another property to the component itself for it.

@nathanhammond
Copy link
Member

This also seems like something where element modifiers might play in as well.

@simonihmig
Copy link
Contributor Author

@rwjblue would be fine for me as well!

@nathanhammond to be honest I don't see the connection here. Could you elaborate on it?

@miguelcobain
Copy link
Contributor

There's also the "eternal" emberjs/ember.js#12500
Might be interesting for the discussion. :)

@simonihmig
Copy link
Contributor Author

@miguelcobain Thanks for pointing this out! Yeah, if that PR would land, it would solve this issue as well. Maybe It's time to "revive" your PR when Glimmer2 has finally landed in 2.9? :)

@simonihmig
Copy link
Contributor Author

As it has been quiet on this one for a while, how can we proceed?

Is making Ember.ViewUtils.getViewBounds public the preferred solution then, as @rwjblue suggested? If so, should I try to put an RFC together that goes into that direction?

@rwjblue
Copy link
Member

rwjblue commented Nov 19, 2016

Ya, an RFC for that seems like a good next step.

@simonihmig
Copy link
Contributor Author

Ok, great, will go for it.

My motivation for this came from the specific use case of getting the parent from a tagless component. But Ember.ViewUtils.getViewBounds does actually provide more than that, in particular the component bounds. Are there any additional valid use cases to make that whole thing public, that should be mentioned in the RFC?

@kybishop
Copy link

@simonihmig is there an official RFC for this?

In the meanwhile, @pzuraq came up with a clever workaround for ember-popper where we get the parent node of an {{unbound}} element which we stick in our tagless component.

@pzuraq
Copy link
Contributor

pzuraq commented Aug 25, 2017

Oh yeah, this would be much better than my hack. Would definitely love to see this become an RFC 😄

@mehulkar
Copy link
Contributor

mehulkar commented Apr 1, 2020

@simonihmig now that modifiers are a thing, I think this can be closed right?

@simonihmig
Copy link
Contributor Author

now that modifiers are a thing, I think this can be closed right?

Well, 2.5y later I wasn't even aware this was still open! 😝

And yes, now with Glimmer components the original request to extend Ember.Component with something does not make too much sense anymore.

But still not sure how we could implement the use case properly with modifiers or whatever other public API we have now. Given the original example:

<button>
  Create
  <BsTooltip @title="Just create something"/>
</button>

<BsTooltip> is a tagless component (Ember or Glimmer) that renders into another destination element using in-element. So attaching a modifier to anything it renders would not allow to traverse to the parent.

FWIW, it's still using that empty TextNode hack...

@Panman82
Copy link

Panman82 commented Apr 2, 2020

To move this to a modifier, what about instead of using {{in-element}}, have the user put a <BsTooltipContainer /> component in their application.hbs template. The component would then monitor a BsTooltipService service which the {{bs-tooltip}} modifier would add/remove things from/to/whatever. That would open the door to;

<button {{bs-tooltip "Just create something"}}>
  Create
</button>

Which provides access to the element to add/remove event listeners (hover). I might have a second to create a quick demo if you'd like.

@chriskrycho
Copy link
Contributor

A quirky workaround that I think should get the job done, and which doesn't require any hackery or use of services, though it isn't necessarily great:

import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';
import { guidFor } from '@ember/object';

export default class WillHaveTooltip extends Component {
  @tracked id;

  constructor() {
    super(...arguments);
    this.id = guidFor(this);
  }
}
<button id={{this.id}}>
  Create
  <BsTooltip @title="Just create something" @parentId={{this.id}}/>
</button>

Then BsTooltip can just do something like this:

const targetEl = document.querySelector(`#${this.args.parentId}`);
assert(targetEl instanceof HTMLElement);
// off to the races

This is, to be clear, less elegant than we might like – in particular, it would be nice if we could do all of that nicely in a template-only component – but it should work.

@simonihmig
Copy link
Contributor Author

@Panman82 Thanks for the suggestion, which probably would work. But given that the motivation for bringing this topic up is to have a public and easy to use API for that use case, the amount of orchestration needed in your approach does not really qualify as easy 😉

Also it wouldn't work for <BsTooltip>'s bigger brother, <BsPopover> which is used in block form:

<button>
  Create
  <BsPopover @title="Popover">
    Can contain <FancyComponent>arbitrary</FancyComponent> content.
  </BsPopover>
</button>

A quirky workaround that I think should get the job done, and which doesn't require any hackery or use of services, though it isn't necessarily great

Yeah, that's possible, and indeed is already supported as an alternative way to specify the tooltip's triggerElement, using a CSS selector. But the default is to automatically attach to the parent element, and that seems to require some hackery - one way or the other...

@rwjblue
Copy link
Member

rwjblue commented May 20, 2020

@simonihmig - TBH, I got confused a bit when I read through this (partially because of its age, and the community thinking has changed quite a bit since you original posted the issue). Can you help me understand why you would want to know the caller's parent element?

Also, RE: getViewBounds, I think it would probably be fine to make this public API (along the lines of other public but lesser used APIs) but I think that it has somewhat fundamental issues with template only components but @chancancode would probably know better.

@simonihmig
Copy link
Contributor Author

Can you help me understand why you would want to know the caller's parent element?

Basically what I wrote in my initial posting:

The tooltip component must have access to its parent element, to attach event listeners to it, but should not (until the tooltip is actually shown) add anything to the DOM. Thus a tagless component.

So having a way to support this API, that attaches a tooltip to its parent element:

<button>
  Create
  <BsTooltip @title="Just create something"/>
</button>

With a classic Ember component, this would be possible using this.element.parent (which is what others are doing: https://github.com/sir-dunxalot/ember-tooltips/blob/master/addon/components/ember-tooltip-base.js#L173), but I want it to be tag-less (to not add anything to DOM until needed), where this does not work. Same for Glimmer components obviously.

Copy link
Member

rwjblue commented May 20, 2020

Hmmm, when do you need to know your parent DOM element? Only after you do want to render something, or eagerly in order to set things up for later?

@simonihmig
Copy link
Contributor Author

Hmmm, when do you need to know your parent DOM element? Only after you do want to render something, or eagerly in order to set things up for later?

Eagerly, as I want to attach event listeners to the parent element.

@nightire
Copy link

Eagerly, as I want to attach event listeners to the parent element.

I think a modifier is more suitable for this use case, at least I've created one to do the exact thing in our project. Are there any possibilities that a modifier can't access the parent DOM element?

Copy link
Member

rwjblue commented May 21, 2020

Yeah, agree. I think providing both the modifier and the component to do the rendering seems good/fine to me for this case. 🤔

@simonihmig
Copy link
Contributor Author

Ok, but how would you "connect" the modifier (which attaches an event listener) to the component (which should render stuff when the event occurs)? If we had contextual modifiers, we could do something like this I guess:

<BsTooltip @title="Just create something" as |attach|>
  <button {{attach}}>Create</button>
<BsTooltip>

But that's no a thing for now. Again to be clear, it's not that there are no ways to do this, it's just that I would want to make this as simple as possible, and not offload any kind of orchestration work to the user for such a trivial thing as adding a tooltip. Just invoking a tooltip component would satisfy that constraint, but again requires some way to access the parent element.

Given that this issue stems from a time of classic APIs like this.element, no outerHTML component, no modifiers etc., and there are some (rather awkward) ways to solve this (either private getViewBounds or the empty text node trick I am currently using), I am actually fine with closing this issue. Especially as this seems more like an edge case, which addons might have to find a way to deal with, and not so much affect apps.

@nightire
Copy link

Below is how we did to implement a modifier based <Tooltip /> component:

https://ember-twiddle.com/bb00a66732941c05d82be32f0046cdc6

@simonihmig I understand that if there's a reference of the parent element, things could be a little easier, at least in the demo above, I could remove the line of modifiers/tooltip.js#20 and pass in this.element.parentElement instead. But IMO, to get the parent element, components should render at least one element in place, otherwise this.element.parentElement would not make sense.

In this particular example, a Tooltip component sends its content over to a portal container by using in-element, and the consequence is, there's no this.element after in-element took effects. It is why I use a placeholder element before in-element and remove it after I got the reference of the parent element.

@rwjblue A tagless component may render nothing in place and send over all of its content to any other places in the DOM tree instead. I believe this is the reason that @simonihmig requires a reference of the parent element here.

@rwjblue
Copy link
Member

rwjblue commented May 23, 2020

@simonihmig FWIW, I am not really trying to suggest that we close this. I'm mostly trying to properly understand the problem space. I just haven't hit this type of issue / usage in the past.

@rwjblue
Copy link
Member

rwjblue commented May 23, 2020

@nightire sure, that makes sense, but these exact scenarios have gotten us into massive trouble in the past (look at the whole slew of nearestBy* APIs that Ember.View had). Trying to hide complexity from your users and make things work magically isn't always better than a more explicit but easier to understand invocation.

@rwjblue
Copy link
Member

rwjblue commented May 23, 2020

@simonihmig the snippet you pasted actually seems pretty nice:

<BsTooltip @title="Just create something" as |attach|>
  <button {{attach}}>Create</button>
<BsTooltip>

The contextual modifiers RFC already landed in emberjs/rfcs#432, and a bit of work towards implementing the plumbing has been done and absorbed in the rendering engine updates from Ember 3.17+.

@nightire
Copy link

The contextual modifiers RFC already landed in emberjs/rfcs#432 , and a bit of work towards implementing the plumbing has been done and absorbed in the rendering engine updates from Ember 3.17+.

Wow, I don't even know there's an RFC.🤣

@simonihmig
Copy link
Contributor Author

@nightire thanks for that example, makes things clear now! As an addon author I would be worried though to (temporarily) render that <div> in-place, as it could cause unwanted visual glitches (in the not so likely but possible case that some CSS selector is matching that button div). So I would rather keep that empty text node trick in place, which does not allow the use of a modifier, but guarantees that it does not cause anything unwanted to get rendered.

the snippet you pasted actually seems pretty nice:

Well, yes and no IMHO. In terms of the use of Ember's new and modern primitives (modifiers, contextual modifiers) maybe, but the initial snippet still seems preferable to me. This one suggest that the button is a child of a Tooltip, which is not the case. And it's less readable I think, as the button is actually the important and meaningful part in the template, the tooltip is secondary.

FWIW, I am not really trying to suggest that we close this

Yeah, I know, it was more myself who suggested that! 😉 As that empty text node trick is "good enough" for that rare use case, and probably does not justify increasing the public API surface therefore. Especially as an API for this does not really fit that well with the modern Ember world anymore (not even a this.element anymore in Glimmer components).

simonihmig added a commit to ember-bootstrap/ember-bootstrap that referenced this issue Sep 2, 2020
With rehydration the FastBoot rendered empty TextNode (used to find the component's parent element) is reused. So the TextNode instance that the component has created at runtime is not attached to the DOM, thus cannot be used to find the parent. In this case Ember's private API `Ember.ViewUtils.getViewBounds()` is used as a fallback.

Related: emberjs/rfcs#168
@jelhan
Copy link
Contributor

jelhan commented Sep 2, 2020

@simonihmig the snippet you pasted actually seems pretty nice:

<BsTooltip @title="Just create something" as |attach|>
  <button {{attach}}>Create</button>
<BsTooltip>

This wouldn't support using <BsTooltip> in block mode:

<button>
  ...
  <BsTooltip>
    Some content for the tooltip.
  </BsTooltip>
</button>

I think it's easier to read in block mode than with @title argument. Also block mode makes it way easier to use HTML in the tooltip content:

<button>
  ...
  <BsTooltip>
    <em>Tooltip</em> <u>with</u> <b>HTML</b>
  </BsTooltip>
</button>

This is explicitly supported by Bootstrap.

I think there needs to be a replacement available before Ember.ViewUtils.getViewBounds() private API is removed. Especially cause the empty text node work-a-round mentioned by @simonihmig earlier doesn't seem to work reliable with FastBoot rehydration: ember-bootstrap/ember-bootstrap#1219

@simonihmig
Copy link
Contributor Author

Given that the proposed alternatives all have more or less substantial drawbacks, and that empty TextNode trick does not work with FastBoot rehydration, I am thinking of finally writing up an RFC for this.

@rwjblue you approved that a long time ago, would you still think in the current Octane era that an RFC to make Ember.ViewUtils.getViewBounds() public API would have a solid chance of being accepted?

@webark
Copy link

webark commented Sep 2, 2020

make Ember.ViewUtils.getViewBounds() public API

In your RFC, could we explore a different name for this @simonihmig ? just makes me think more of "getBoundingClientRect" then anything.

@simonihmig
Copy link
Contributor Author

In your RFC, could we explore a different name for this

Haven't given this much thought yet, but sure! Also I think we would need to provide a proper module export, rather than accessing it from the Ember.* namespace. But I think let's get into those detailed discussions a part of the actual RFC! :)

@Skwai
Copy link

Skwai commented Feb 1, 2022

I'm also encountering similar issues here when authoring a popover Glimmer component in Octane.

Being able to reference the parent element from an in-element component would be very helpful but I can't seem to find a way to cleanly do it. From my understanding getViewBounds doesn't work with Glimmer2?

I can pass a target element argument (so that the popover knows what to bind to) to my popover component but it is a bit cumbersome as I have to rely on did-insert render modifier to assign the element every time I want to use a popover:

// parent-view/template.hbs

<button type="button" {{on "click" this.open}} {{did-insert this.didInsertButton}}>Open me</button>

{{#if this.open}}
  <MyPopover @target={{this.buttonElement}}>
    You just clicked a button
  </MyPopover>
{{/if}}
// my-popover/template.hbs
{{#in-element this.popover.element}} <-- in-element target is persisted in a PopoverService
  <div class="MyPopover">
    {{yield}}
  </div>
{{/in-element}}

An API to allow my-popover to know the context of where it was rendered from would be very helpful

@NullVoxPopuli
Copy link
Contributor

NullVoxPopuli commented Feb 2, 2022

@Skwai this is how I do popovers with button elements:
https://github.com/NullVoxPopuli/ember-popperjs/blob/main/addon/-private/popper-j-s.ts#L100

tldr:

  • I yield two modifiers, 1 for the target, one for the popover

api is this:

<PopperJS as |reference popover|>
 <button {{reference}} {{on "click" this.yourClickHandler}}>
   {{yield to="trigger"}}
 </button>

 {{#if this.yourVisibilityIndicator}}
   <div {{popover}}>
     This is a popover!
     {{yield to="default"}}
   </div>
 {{/if}}
</PopperJS>

modifiers can be passed willy nilly as args, so you could thread through to whatever component you need to

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

@simonihmig is there still an intent to write an RFC? How can we help facilitate that?

@simonihmig
Copy link
Contributor Author

is there still an intent to write an RFC?

Hm, not really. I have some things in mind to write an RFC for, which feel more important (to me).

Currently we have two options at hand, which seem "good enough":

  • rendering an empty text node to get the parent element (currently done by ember-bootstrap, it works, even if a bit hacky)
  • yielding modifiers/components as suggested by @NullVoxPopuli (uses clean idiomatic Ember APIs, but the template code is a bit more verbose)

So I'll close this for now, as this narrow use case (for which there are ok-ish solutions) probably does not fully justify a new public API.

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