Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Directive scope variables using hungarian type notation #5370

Closed
Haemp opened this issue Dec 11, 2013 · 4 comments
Closed

Directive scope variables using hungarian type notation #5370

Haemp opened this issue Dec 11, 2013 · 4 comments
Assignees

Comments

@Haemp
Copy link

Haemp commented Dec 11, 2013

Using variable names prefixed by a single letter seems to have changed in behaviour from 1.1.5 to 1.2.0. In 1.1.5 I used to be able to set oStart as a property name and reference it with o-start, but that seems to now have broken.

Using 1.2.0 http://plnkr.co/edit/EgRNDE?p=preview
Using 1.1.5 http://plnkr.co/edit/r8Q9bj?p=preview

Skimmed through the breaking changes and could't find anything about this and no other related issues as far as I was able to search. Any thoughts?

@caitp
Copy link
Contributor

caitp commented Dec 11, 2013

You're right, the key in the Attributes object is o, even though the attribute name should be oStart. I'm poking through the blame to see what caused that. I can probably have a patch ready shortly.

@caitp
Copy link
Contributor

caitp commented Dec 11, 2013

You know why this is? It's because of the special directive-start and directive-end attributes. It won't apply to attributes which don't end with Start or End... although I can see how that's sort of inconvenient.

              var directiveNName = ngAttrName.replace(/(Start|End)$/, '');
              if (ngAttrName === directiveNName + 'Start') {
                attrStartName = name;
                attrEndName = name.substr(0, name.length - 5) + 'end';
                name = name.substr(0, name.length - 6);
              }

...so, I don't know if this really needs a patch, unless people think this rule needs to be tightened up a bit or something

It is actually mentioned in the breaking changes here

  • due to e46100f, existing directives with name ending with "-start" or "-end" will stop working.

    This change was necessary to enable multi-element directives. The best fix is to rename existing directives, so that they don't end with these suffixes.

I agree that this isn't necessarily good, as you may wish to have onDragStart or onDragEnd or similar names, so I think it's probably justifiable to only enforce these rules if the directiveNName actually matches an existing directive... but that's how it is for now I guess

@Haemp
Copy link
Author

Haemp commented Dec 11, 2013

Ah, totally missed that - well that solves that then. I'm fine with it being that way, just caught me off guard. Thanks for looking into it.

@jeffbcross
Copy link
Contributor

I'm closing this issue, as I think @caitp has captured the problem and solution more accurately in #5372.

caitp added a commit that referenced this issue Jul 16, 2014
Directives which expect to make use of the multi-element grouping feature introduced in
1.1.6 (e46100f7) must now add the property multiElement
to their definition object, with a truthy value.

This enables the use of directive attributes ending with the words '-start' and '-end' for
single-element directives.

BREAKING CHANGE: Directives which previously depended on the implicit grouping between
directive-start and directive-end attributes must be refactored in order to see this same behaviour.

Before:

```
<div data-fancy-directive-start>{{start}}</div>
  <p>Grouped content</p>
<div data-fancy-directive-end>{{end}}</div>

.directive('fancyDirective', function() {
  return {
    link: angular.noop
  };
})
```

After:

```
<div data-fancy-directive-start>{{start}}</div>
  <p>Grouped content</p>
<div data-fancy-directive-end>{{end}}</div>

.directive('fancyDirective', function() {
  return {
    multiElement: true, // Explicitly mark as a multi-element directive.
    link: angular.noop
  };
})
```

Closes #5372
Closes #6574
Closes #5370
Closes #8044
Closes #7336
ckknight pushed a commit to ckknight/angular.js that referenced this issue Jul 16, 2014
Directives which expect to make use of the multi-element grouping feature introduced in
1.1.6 (angular@e46100f7) must now add the property multiElement
to their definition object, with a truthy value.

This enables the use of directive attributes ending with the words '-start' and '-end' for
single-element directives.

BREAKING CHANGE: Directives which previously depended on the implicit grouping between
directive-start and directive-end attributes must be refactored in order to see this same behaviour.

Before:

```
<div data-fancy-directive-start>{{start}}</div>
  <p>Grouped content</p>
<div data-fancy-directive-end>{{end}}</div>

.directive('fancyDirective', function() {
  return {
    link: angular.noop
  };
})
```

After:

```
<div data-fancy-directive-start>{{start}}</div>
  <p>Grouped content</p>
<div data-fancy-directive-end>{{end}}</div>

.directive('fancyDirective', function() {
  return {
    multiElement: true, // Explicitly mark as a multi-element directive.
    link: angular.noop
  };
})
```

Closes angular#5372
Closes angular#6574
Closes angular#5370
Closes angular#8044
Closes angular#7336
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants