Giter VIP home page Giter VIP logo

Comments (36)

rwjblue avatar rwjblue commented on May 14, 2024 2

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.

from rfcs.

rwjblue avatar rwjblue commented on May 14, 2024 1

@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+.

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024 1

@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).

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024 1

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?

from rfcs.

nathanhammond avatar nathanhammond commented on May 14, 2024

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

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

@rwjblue would be fine for me as well!

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

from rfcs.

miguelcobain avatar miguelcobain commented on May 14, 2024

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

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

@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? :)

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

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?

from rfcs.

rwjblue avatar rwjblue commented on May 14, 2024

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

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

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?

from rfcs.

kybishop avatar kybishop commented on May 14, 2024

@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.

from rfcs.

pzuraq avatar pzuraq commented on May 14, 2024

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

from rfcs.

mehulkar avatar mehulkar commented on May 14, 2024

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

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

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...

from rfcs.

Panman82 avatar Panman82 commented on May 14, 2024

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.

from rfcs.

chriskrycho avatar chriskrycho commented on May 14, 2024

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.

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

@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...

from rfcs.

rwjblue avatar rwjblue commented on May 14, 2024

@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.

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

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.

from rfcs.

rwjblue avatar rwjblue commented on May 14, 2024

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?

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

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.

from rfcs.

nightire avatar nightire commented on May 14, 2024

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?

from rfcs.

rwjblue avatar rwjblue commented on May 14, 2024

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

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

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.

from rfcs.

nightire avatar nightire commented on May 14, 2024

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.

from rfcs.

rwjblue avatar rwjblue commented on May 14, 2024

@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.

from rfcs.

rwjblue avatar rwjblue commented on May 14, 2024

@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.

from rfcs.

nightire avatar nightire commented on May 14, 2024

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.🤣

from rfcs.

jelhan avatar jelhan commented on May 14, 2024

@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

from rfcs.

webark avatar webark commented on May 14, 2024

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.

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

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! :)

from rfcs.

Skwai avatar Skwai commented on May 14, 2024

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

from rfcs.

NullVoxPopuli avatar NullVoxPopuli commented on May 14, 2024

@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

from rfcs.

wagenet avatar wagenet commented on May 14, 2024

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

from rfcs.

simonihmig avatar simonihmig commented on May 14, 2024

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.

from rfcs.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.