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

Use @as to customize root element #77

Open
alexlafroscia opened this issue Sep 1, 2021 · 7 comments
Open

Use @as to customize root element #77

alexlafroscia opened this issue Sep 1, 2021 · 7 comments
Labels
good first issue Good for newcomers

Comments

@alexlafroscia
Copy link
Collaborator

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

@NullVoxPopuli
Copy link
Collaborator

I'm in favor

@GavinJoyce
Copy link
Owner

GavinJoyce commented Oct 3, 2021

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

@GavinJoyce
Copy link
Owner

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

@NullVoxPopuli
Copy link
Collaborator

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

@GavinJoyce
Copy link
Owner

GavinJoyce commented Oct 3, 2021

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

@GavinJoyce
Copy link
Owner

do we have an RFC for an element helper?

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

@bertdeblock
Copy link
Contributor

👋 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants