universal-ember / ember-primitives

Primitives for building UI faster
https://ember-primitives.pages.dev/
MIT License
33 stars 15 forks source link

Use ember-stargate for Portals? #108

Open simonihmig opened 1 year ago

simonihmig commented 1 year ago

Shamelessly wondering whether you have considered using ember-stargate as the implementation for portals? 😀 And if so, is there anything that you miss?

Except for code reuse in general, I would see these benefits...

  1. Flexible targets This library (and another similar private implementation we both have seen) makes assumption about the possible targets (popover, tooltip, modal), but what if you have use cases beyond that?

One case I had (which sparked the creation of that addon) was rendering into a navigation/footer bar, that is owned by the parent route (stable component invocation even when switching between sub routes) from the subroute (think of back/next buttons, or some status indicators). The footer stuff depends on the state of the subroute, but the UI layout would require the footer to be part of the parent route (or application). This would call for arbitrary portal targets, and the ability to put them separately in arbitrary spots (not just top-level DOM).

  1. Decouple rendering the portal and the target

The low-level in-element requires the target to be already in the DOM, otherwise it throws. And I believe (have not tested tbh) the implementation here propagates this behavior, as findNearestTarget would also throw when it does not find the target (yet).

But what if it just happens (because of the UI structure, think again of a footer element), that the target is getting rendered after the portal? (even in the same render cycle, just "bad" timing)

Like this test would not work, if you would render <PortalTargets> after <Portal>, would it?

ember-stargate abstracts this away, see these tests for example.

NullVoxPopuli commented 1 year ago

Wrapping ember-stargate is certainly worth it! (Only reason I didn't use it initially is because i was still prototyping out concepts). Very happy to adopt a robust implementation!

I'd even take a pr for it! (I'd get to it eventually, but i gotta finish my other open pr first)

NullVoxPopuli commented 1 year ago

ember-stargate probably resolves: https://github.com/universal-ember/ember-primitives/issues/91

NullVoxPopuli commented 1 year ago

looks like this logic in ember-stargate:

would need to be updated to find the "nearest portal target"? https://github.com/universal-ember/ember-primitives/blob/main/ember-primitives/src/components/portal-targets.gts#L20

This is what allows layering of targets (popovers with tooltips nested in modals with tooltips, for example)

makes assumption about the possible targets (popover, tooltip, modal), but what if you have use cases beyond that?

An argument would be passed here: https://github.com/universal-ember/ember-primitives/blob/main/ember-primitives/src/components/portal.gts#L15

Decouple rendering the portal and the target

this is quite nice tho

simonihmig commented 1 year ago

This is what allows layering of targets (popovers with tooltips nested in modals with tooltips, for example)

Yeah, this is interesting indeed! Not sure if that's really viable with the approach used in stargate, as targets are registered on a service, and not found by traversing the DOM... 🤔

An argument would be passed here

How would you render the (custom) target? Not use <PortalTarget> and render <div data-portal-name="custom"></div> manually? Would work, but exposes some (then hard to change) implementation details, right?

NullVoxPopuli commented 1 year ago

Would work,

you got it

but exposes some (then hard to change) implementation details, right?

I guess, but is an attribute really a big deal?

NullVoxPopuli commented 1 week ago

Reminder to myself to try this out (since e-stargate is now 1.0!) and see if it passes my existing test suite