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

feat(input): allow templates on the name attribute of an <input> #3135

Closed
wants to merge 1 commit into from

Conversation

lgalfaso
Copy link
Contributor

@lgalfaso lgalfaso commented Jul 4, 2013

Handle the name attribute on an <input> as a template

Handle the `name` attribute on an <input> as a template
@pkozlowski-opensource
Copy link
Member

@lgalfaso there are several PRs opened for this already:
#2988
#3115

Could you have a look at those and compare to your work? Is there any particular reason for choosing an interpolation expression name="{{nameExp}}" over name="nameExpr"?

@lgalfaso
Copy link
Contributor Author

lgalfaso commented Jul 6, 2013

@pkozlowski-opensource I think that a form field should not be dynamic as it adds some undesirable side-effects and comes at a cost (even if this makes this less flexible). E.g.

  • if the expression evaluates as something that is not a string
  • input of type radio need a name, if one is not defined (read if the name is undefined), one is generated. This does not work well with making the name dynamic nor with making name an expression

#2988 handles this by making name an expression, but at the end of the day, I think it makes more sense to be sure this evaluates to a string
#3115 talks about name at the <form> element, to be consistent I would also use interpolation here, and, once again, not dynamic.

@pkozlowski-opensource
Copy link
Member

@lgalfaso so, clearly people are after dynamically generated names. But introduction of dynamic names creates interesting corner cases. For example, given the HTML:

<input type="text" name="{{alias}}" ng-model="value" required/>'

I could have the alias value changed after the input directive is evaluated. So people using expressions like:

<span ng-show="form[alias].$error.required">Required!</span>

could still be confused...

@lgalfaso
Copy link
Contributor Author

@pkozlowski-opensource Agree, but it was a conscious decision not to allow input names change dynamically. I understand that this may not make everybody happy (the same can be said with ng-switch-when), but given that there are input fields like radio that need a name at the moment the controller is called and that they expect this name not to change, then I find it reasonable that the attribute is evaluated once.

A minor side-effect is that we do not have to watch for changes on the value, so we would save a few cycles here and there

@Kleptine
Copy link

I've been playing around with this fix a bit, and came across this scenario that doesn't seem to work. Is this a bug?

it('should support dynamically generated input names', function () {
    scope.names = ["input1","input2"];

    doc = $compile(
        '<div ng-form="myForm">' +
        '   <input type="text" ng:repeat="n in names" name="{{n}}" ng-model="n"/>' +
        '</div>')(scope);

    expect(scope.myForm.input1).toBeDefined();  // Fails
    expect(scope.myForm.input2).toBeDefined();  // Fails
  });

One-off aliasing is great, but most of the time I want to generate my inputs dynamically.

@lgalfaso
Copy link
Contributor Author

@Kleptine ng-repeat is implemented async, so at your test, after $compile(...); you have to do scope.$apply() and then the test will pass. Outside of a test, you do not have to call $apply and things will work as expected.

@Kleptine
Copy link

Ah, of course. Looks good then.

@trask
Copy link

trask commented Sep 6, 2013

+1
I need this also. For now, cherry-picked @lgalfaso's patch and it is working well. Thanks.

@mllrjb
Copy link
Contributor

mllrjb commented Sep 19, 2013

+1
Need this as well. Using a directive as an interim solution.

@nadavsinai
Copy link

+1

1 similar comment
@elado
Copy link

elado commented Feb 4, 2014

👍

@Narretz
Copy link
Contributor

Narretz commented Oct 13, 2014

This was implmented in 729c238

@Narretz Narretz closed this Oct 13, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants