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

Pick an element for <PortalTarget> #415

Open
gossi opened this issue Jan 26, 2022 · 3 comments
Open

Pick an element for <PortalTarget> #415

gossi opened this issue Jan 26, 2022 · 3 comments

Comments

@gossi
Copy link

gossi commented Jan 26, 2022

As of now a <PortalTarget> is rendered as <div>. In some situations you want to be more specific in expressing proper semantics and a <div> isn't the best solution, or you have to wrap the <div> from <PortalTarget> in a more semantic element, but that clutters the actual output.

I'm using a technic to make this customizable quite well and am proposing for this situation, too in using the (element) helper:

Invocation:

<PortalTarget @element={{element 'nav'}} ... />

And inside of it:

{{#let (or @element (element 'div')) as |Element|}}
  <Element ...>...</Element>
{{/let}}

This way it will keep <div> as a default/fallback but provides a strategy to override this.

WDYT?

@simonihmig
Copy link
Owner

Yes, that would be nice! PR welcome! 🙂

An alternative would be to expose a modifier, so you can attach the target to any existing element. However while the addon currently does not support FastBoot correctly, it would be possible to do that probably with the existing component API, while a modifier won't ever run in FastBoot, so that's not so good...

@acorncom
Copy link

@gossi just ran into a case where I'd prefer to have fewer DOM elements as you outlined, did you ever end up tackling this? Not at all urgent but thought I'd ask 😄

@gossi
Copy link
Author

gossi commented Jan 25, 2023

I re-arranged this with:

<nav>
  ...
  <PortalTarget ...>
</nav>

which is fine to take <div>s for me. More elegant is to have control over this for sure. Shouldn't be too hard to write a PR though 👍

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

3 participants