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

Update paper-button to native class syntax and move it to pod structure. #1141

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Shajansheriff
Copy link
Contributor

Refactor focusin & focusout at focusablemixin to event listeners.

Refactor focusin & focusout at focusablemixin to event listeners.
@Shajansheriff
Copy link
Contributor Author

@miguelcobain Do let me know this approach is good enough to gradually migrate other components as well. Or shall we refactor them to tagless component and move all the decorators bindings to template? Even though there are ember-decorators to support native class syntax for ember-component, Ember is strongly advising to start using tagless component and modifiers in template for event handling. What is your suggestion?

I also see a small problem in migrating paper-button to tagless, there is href logic to create button as anchor tag in init hook, which might be an obstacle. Any workaround?

@miguelcobain
Copy link
Owner

miguelcobain commented Mar 31, 2020

@Shajansheriff indeed, the idea is to use tagless components (with the decorators @tagName(''). It will make the transition to glimmer components so much easier in a near future.

You bring up a good question regarding the href logic in paper-button.
The way to solve that is to use the ember-element-helper addon like:

{{#let (element (if @href "a" "button")) as |Tag|}}
  <Tag ...attributes>{{yield}}</Tag>
{{/let}}

Don't forget to include the ember-element-helper in the dependencies and not devDependencies of package.json.

Doing this will force you to migrate away from ColorMixin and FocusableMixin.
Those two should be easy to refactor using normal templating.

The ProxiableMixin however might require a bit more testing, but I think shouldn't be too hard as it is only used on the <PaperItem> component.

Let me know if you need any further clarification, and thank you for your work!

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

Successfully merging this pull request may close these issues.

None yet

2 participants