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

Link component hook #230

Closed
wants to merge 3 commits into from
Closed

Conversation

SpikedKira
Copy link

@SpikedKira SpikedKira commented Jun 12, 2017

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main concerns are around:

  • adding more things to LinkComponent (you already have this in the drawbacks section)
  • naming of eventAction, is there another name that might work better?
  • if we are going to allow folks to override we should allow them to actually use the default (e.g. adding custom tracking functional would still want to call the current _invoke)

0000-template.md Outdated
@@ -1,52 +0,0 @@
- Start Date: (fill me in with today's date, YYYY-MM-DD)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file shouldn't be deleted, can you bring it back?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

embarrassing :o

@SpikedKira
Copy link
Author

@rwjblue unless I'm missing something, the 3rd point is already covered. The new eventAction would be a public method that calls the private _invoke method. So if a user overrides, they can call _super() to use the default after doing any logging or w/e.

rwjblue added a commit to rwjblue/ember-test-helpers that referenced this pull request Oct 9, 2017
Mostly copied/stolen from emberjs's packages/ember-testing/lib/events.js
but with some tweaks to make it work without jQuery (I think those
changes were made upstream in Ember as well).

The long term goal here is to remove this file and have
ember-test-helpers _include_ the helpers from ember-native-dom-helpers,
but that will require an RFC once the changes proposed in
emberjs/rfcs#230 are implemented.
rwjblue added a commit to rwjblue/ember-test-helpers that referenced this pull request Oct 9, 2017
This was added in an early attempt to simplify / separate things for the
"grand testing unification" RFC, but ultimately was never rolled out to
folks.

At this point, it has become clear that this class based system for
managing test setup is not the way forward (see emberjs/rfcs#230 for
details) and this module (that is unused) has just become "dead weight"
maintenance wise.
rwjblue added a commit to rwjblue/ember-test-helpers that referenced this pull request Oct 10, 2017
Moves all of the future deprecated code into a `legacy-0-6-x` subfolder
(while preserving import paths) in order to ease implementation of
emberjs/rfcs#230.
rwjblue added a commit to rwjblue/ember-test-helpers that referenced this pull request Oct 10, 2017
Moves all of the future deprecated code into a `legacy-0-6-x` subfolder
(while preserving import paths) in order to ease implementation of
emberjs/rfcs#230.
@rwjblue rwjblue added the FCP to close The core-team that owns this RFC has moved that this RFC be closed. label Nov 13, 2020
@rwjblue
Copy link
Member

rwjblue commented Nov 13, 2020

Moving into FCP to close for this, I think the right path forward for folks here is to not use {{link-to at all (instead using https://github.com/emberjs/rfcs/blob/master/text/0391-router-helpers.md).

@rwjblue rwjblue closed this Nov 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FCP to close The core-team that owns this RFC has moved that this RFC be closed. T-components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants