GavinJoyce/ember-headlessui

Use `@as` to customize root element

Opened this issue ยท 7 comments

Some of our component use a @tagName argument to customize the root element, while others use @as

We should use @as across all components for two main reasons:

  1. Consistency with the React/Vue implementations
  2. Accepting a component rather than just a string would be nice, like Menu::ItemElement does (to support LinkTo)
    {{#let
    (if this.tagNameIsComponent @tagName (element (or @tagName 'a')))
    as |Tag|
    }}

Note: this will be a breaking change for the components currently configured to use @tagName

I'm in favor

Might be nice to create a helper named something like component-or-element to remove the need for a backing class in the Menu::ItemElement component:

{{#let (component-or-element @as) as |Tag|}}

Hmm, ^ might not be possible as the element helper is actually an AST transform, not a function that we can invoke:

https://github.com/tildeio/ember-element-helper/blob/main/addon/helpers/element.js

do we have an RFC for an element helper? maybe we need to implement that over in the glimmer-vm?

A quick improvement might be to just create a tag-name-is-component helper, that will allow us to remove the need for a backing class

do we have an RFC for an element helper?

The https://github.com/tildeio/ember-element-helper readme references a couple of RFCs

๐Ÿ‘‹ Interested in contributing to this project and this seems like a good one to pick up (unless someone is already working on this of course).

Couple of questions:

  • Should a tag name and a component always be accepted everywhere or are there cases were we only want to allow a tag name and not a component or vice versa?
  • Should the current @as cases be updated to also accept components?
  • Should @tagName be deprecated first or do we just delete it and release a new minor?
  • It seems {{tag-name-is-component}} does not support imported component classes (e.g. @as={{this.SomeGlimmerComponentClass}}) would it be okay to update this? Can be a separate PR of course