[FEATURE ember-routing-linkto-target-attribute]

[selvagsz]

Taking up the row of #3924 . Supports target attribute for the link-to anchor element

[rwjblue]

For the feature flags to be stripped by defeatureify you can not combine logic (it only looks for the single conditional).

Also, both sides of this are doing the same thing, is the conditional needed?

[selvagsz]

Oops that was my bad!! those lines has to be

if (Ember.FEATURES.isEnabled("ember-routing-linkto-target-attribute")) {
  if(!this.get('target')) {
    event.preventDefault();
  }
} else {
  event.preventDefault();
}

The preventDefault should not happen when a target attribute is provided

[rwjblue]

I see, that makes sense.

[jayphelps]

💯 I'm currently injecting this ability into LinkView so I'm very excited for this.

[stefanpenner]

can you add tests?

[selvagsz]

@stefanpenner Added the test cases. I do have a query.
Do i need to preventDefault target='_self'? It's the default one if there is no target attribute specified.

[jayphelps]

@selvagsz @stefanpenner IMO since I'm using similar functionality in my apps: I chose to have it behave just like as if you didn't include it. i.e. it does still preventDefault. The reason why was that you can use {{link-to "Go Here" "some.place" preventDefault=false}} and I think that's much more clear to accomplish that behavior. Also, as you said, _self is the default target if one isn't provided so in my mind, it shouldn't make any difference to ember either.

[selvagsz]

@jayphelps Thanks for your suggestion. Updated my PR now

[tomdale]

So, we're going to merge this. I was opposed at first because I was afraid that this was a slippery slope, due to:

1. {{link-to}} can take a tagName attribute, allowing it to become an arbitrary element.
2. At that point, developers would be right in asking for any arbitrary attribute to be supported by the {{link-to}} helper.

However, given some of the new capabilities coming in HTMLbars, there should no longer be any need to use {{link-to}} with a custom tagName. So long as we keep the attributes limited to those used by the a element, I'm happy with that.

[selvagsz]

Does this need anything else to get into the beta train

[stefanpenner]

@rjackson please pull into beta

[rwjblue]

@stefanpenner - Generally speaking we do not add new feature flagged items to the beta branch. We would normally enable by default on master/canary then let it roll to beta at the next cycle. Do you think we need to circumvent that in this case?

[stefanpenner]

@rjackson i have no idea why i said that

[stefanpenner]

please don't listen to me

[jayphelps]

@stefanpenner

[leostera]

👍

[tomdale]

This was go-ed at today's core team meeting.

[selvagsz]

Thanks 👍