feat(input): support multiple attribute for email #8987

shahata · 2014-09-09T08:48:44Z

shahata · 2014-09-09T08:51:21Z

shahata · 2014-09-09T08:52:28Z

Not sure if ng-model should expose an array in this case. Would be nice to hear someone's opinion about that...

shahata · 2014-09-09T08:53:53Z

src/ng/directive/input.js

-    return ctrl.$isEmpty(value) || EMAIL_REGEXP.test(value);
+    if (!ctrl.$isEmpty(value)) {
+      value = attr.multiple ? value.split(/\s*,\s*/) : [value];
+      for (var i = 0; i < value.length; i++) {


I would really like to use Array.prototype.every here instead of this for loop. Since IE8 is no longer supported, can we do that?

Narretz · 2014-09-09T10:00:13Z

Honestly, I'd love to have a more robust solution for this that works with all (text-based)-inputs similar to ngList (which exposes an array). I don't know what the HTML spec says about delimiters, supported input types etc. though.

Is this really all the information there is: http://www.whatwg.org/specs/web-apps/current-work/multipage/forms.html#attr-input-multiple?

shahata · 2014-09-09T11:07:31Z

afaik the multiple attribute is only relevant for email and file inputs.
I'll update the PR tonight to expose an array in this case.
regarding delimiters, the spec says comma separated.

On Tue, Sep 9, 2014 at 1:00 PM, Narretz notifications@github.com wrote:

Honestly, I'd love to have a more robust solution for this that works with
all (text-based)-inputs similar to ngList (which exposes an array). I don't
know what the HTML spec says about delimiters etc. though.

—
Reply to this email directly or view it on GitHub
#8987 (comment).

shahata · 2014-09-09T11:22:27Z

some references...

supported only for email and file:
http://www.w3.org/TR/2012/WD-html5-20121025/the-input-element.html#input-type-attr-summary

comma separated:
http://www.w3.org/TR/html-markup/input.email.html#input.email.attrs.value.multiple

Narretz · 2014-09-09T11:57:10Z

Only supported for email, how odd!
Still, wouldn't it be possible to extend $$runValidators in a way that when you have multiple / ngList + the $modelValue is an array, to validate each array item? I like that better then to put the logic inside the validators.

shahata · 2014-09-09T12:01:04Z

Sounds reasonable, I'll look into it.

shahata · 2014-09-09T22:56:23Z

I've refactored this to expose an array and keep the array validation not specific to emails. I think it is hackish and ugly though. Would welcome some suggestions.

shahata · 2014-09-09T23:13:30Z

src/ng/directive/input.js

+    runProcessor(processAsyncValidators);
+
+    function runProcessor(fn) {
+      if ($attr.multiple && isArray(modelValue) && nodeName_($element) !== 'select') {


this condition can be easily changed to work with ngList as well, but it is a pretty big breaking change, so I'm not sure...

the nice thing about getting this into ngList as well is obviously that we could use validators like ngPattern, maxlength, etc. which currently don't work correctly with ngList

petebacondarwin · 2014-09-19T19:32:56Z

@shahata - a cool bit of works as usual. I like the idea of reusing the ngList functionality but I think if we are going to go down this road we should consider a more significant refactoring. In particular I feel uncomfortable about the if statement that has to check we are not in a select element.

How about we factor out the ngList functionality into some helper function (maybe but probably not a service). This could be mixed into the input[email] and ngList independently and possibly be made available (in the future) to allow other directives to make use of it. In the case of ngList it would be turned on all the time. In the case of input[email] it would be turned out by the presence of the multiple attribute as in this PR.

Any thoughts?

shahata · 2014-09-20T12:08:41Z

@petebacondarwin - I've refactored this a bit more, so multiple will no longer be a directive and we can remove those strange nodeName_($element) === 'select' checks. I hope this is what you meant :)

shahata · 2014-09-20T13:48:40Z

src/ng/directive/attrs.js

  var normalized = directiveNormalize('ng-' + attrName);
  ngAttributeAliasDirectives[normalized] = function() {
    return {
      restrict: 'A',
      priority: 100,
      link: function(scope, element, attr) {
+        // binding to multiple is not supported for select
+        if (propName === 'multiple' && nodeName_(element) === 'select') {


had to do this in order to reinstate ngMultiple for non-select elements (it is unsupported for select elements since d87fa00)

petebacondarwin · 2014-09-21T20:54:07Z

@shahata - sorry I haven't got around to looking at this again yet

Closes #6032

jimmywarting · 2014-10-10T09:58:29Z

I know that angular don't support file input (coz of #1375)
This is just a speculation... and maybe of topic

@petebacondarwin if something like file input would even make use of this multiple-directive/service/helper function (in a custom directive or by angular itself in some future), I'm guessing the modelValue would be equal to input.files[0] if multiple is not present. And when it is, the modelValue would be Array.prototype.slice.call(input.files);?

Wouldn't like there to be any conflict by other element using the multiple attribute in any other ways

stryju · 2015-04-24T13:17:05Z

bump

jimmywarting · 2015-09-18T13:47:46Z

ba da bump

Narretz · 2015-09-18T14:55:25Z

I am currently working on this.

shahata reviewed Sep 9, 2014
View reviewed changes

mary-poppins added cla: yes and removed cla: no labels Sep 9, 2014

Narretz added needs: investigation component: forms type: feature labels Sep 9, 2014

shahata reviewed Sep 9, 2014
View reviewed changes

tbosch force-pushed the master branch from a3d7934 to 1418383 Compare September 10, 2014 18:14

petebacondarwin force-pushed the master branch from 02dc2aa to fd2d6c0 Compare September 16, 2014 11:26

shahata mentioned this pull request Sep 17, 2014

ngList is misstaken for the list attribute #9132

Open

petebacondarwin added frequency: moderate needs: public api needs: review severity: inconvenient stable: no labels Sep 19, 2014

petebacondarwin added this to the Backlog milestone Sep 19, 2014

shahata reviewed Sep 20, 2014
View reviewed changes

shahata added 2 commits September 23, 2014 01:13

feat(input): support multiple attribute for email

9abf6dd

Closes #6032

fix(input): observe multiple attribute

e891122

jeffbcross force-pushed the master branch 2 times, most recently from cad9560 to f294244 Compare October 2, 2014 22:09

btford force-pushed the master branch from 9011a65 to 46db47b Compare October 6, 2014 22:54

jeffbcross force-pushed the master branch 3 times, most recently from 8292c21 to 7b9fddf Compare October 9, 2014 20:43

jeffbcross force-pushed the master branch from e8dc429 to e83fab9 Compare October 10, 2014 17:38

petebacondarwin force-pushed the master branch from 4dd5a20 to 998c61c Compare October 19, 2014 10:58

Narretz self-assigned this Jun 22, 2015

Narretz removed the branch: 1.3.x label Sep 13, 2015

petebacondarwin modified the milestones: 1.5.x - migration-facilitation, Backlog Sep 18, 2015

Narretz force-pushed the master branch from 70049ae to b77e14b Compare January 16, 2016 21:59

Narretz modified the milestones: 1.5.x, Backlog Apr 21, 2017

monishdeb mentioned this pull request Nov 27, 2017

CRM-21486: Support multiple test mail civicrm/civicrm-core#11332

Merged

shahata closed this by deleting the head repository Jun 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(input): support multiple attribute for email #8987

feat(input): support multiple attribute for email #8987

shahata commented Sep 9, 2014

shahata commented Sep 9, 2014

shahata commented Sep 9, 2014

shahata Sep 9, 2014

Narretz commented Sep 9, 2014

shahata commented Sep 9, 2014

shahata commented Sep 9, 2014

Narretz commented Sep 9, 2014

shahata commented Sep 9, 2014

shahata commented Sep 9, 2014

shahata Sep 9, 2014

shahata Sep 9, 2014

petebacondarwin commented Sep 19, 2014

shahata commented Sep 20, 2014

shahata Sep 20, 2014

petebacondarwin commented Sep 21, 2014

jimmywarting commented Oct 10, 2014

stryju commented Apr 24, 2015

jimmywarting commented Sep 18, 2015

Narretz commented Sep 18, 2015

feat(input): support multiple attribute for email #8987

feat(input): support multiple attribute for email #8987

Conversation

shahata commented Sep 9, 2014

shahata commented Sep 9, 2014

shahata commented Sep 9, 2014

shahata Sep 9, 2014

Choose a reason for hiding this comment

Narretz commented Sep 9, 2014

shahata commented Sep 9, 2014

shahata commented Sep 9, 2014

Narretz commented Sep 9, 2014

shahata commented Sep 9, 2014

shahata commented Sep 9, 2014

shahata Sep 9, 2014

Choose a reason for hiding this comment

shahata Sep 9, 2014

Choose a reason for hiding this comment

petebacondarwin commented Sep 19, 2014

shahata commented Sep 20, 2014

shahata Sep 20, 2014

Choose a reason for hiding this comment

petebacondarwin commented Sep 21, 2014

jimmywarting commented Oct 10, 2014

stryju commented Apr 24, 2015

jimmywarting commented Sep 18, 2015

Narretz commented Sep 18, 2015