[Followup] Add a custom 'number of values' for 'multiple / multi-value' field. (e.g. nodereference, imagefield)

This enhancement was in response to image module issue #581174: Set the allowed number of image attachments. #21 and #22. I would suggest it would be a useful extra for many modules. Please consider adding it, as it would appear a simple change, or please explain if it is not simple and I am missing the point.

Jonathan

Well, I'm using it in a live environment but I guess you may have assumed that already ;-)

I hope it can be included, as it is such a simple patch but with a great increase in usefulness. As more modules start to use CCK node reference instead of rolling their own code, this could become even more of a required enhancement.

Jonathan

It was exactly what i needed atm. thanks for the patch works perfectly.

@El Bandito,
IMO the proposed solution is OK for a simple modification.
an alternative is to change the widget from 'select list' to an 'textbox'. An empty field then means 'unlimited'.
(This is also proposed in #680616: Setting a custom "number of values" for a multi-value ImageField?)
This is a bit more complicated, and would be the better way for definitive approach.

Also I'd like to set the 'number of values' different per Content Type, rather then creating different fields that only differ in this respect. I'm not sure if this is possible.

Sorry for reviving the issue:
Anyone knows if there is something for D7 in contrib?

Comment #6 works for me... I don't like the idea of having to remember to change that every time there is a core update though...

I know the core team worked very hard on D7 and I'm not trying to bash their work, but it's stuff like this that was so simple in D6 CCK, that has now become a pain in the ass with D7 fields... I realize there has to be a give and take somewhere, but I feel like a lot of the work that was put into CCK that made it such a wonderful project, was stripped out when it was incorporated into D7. Now we are left with "work-arounds" to things that were previously possible.

Anyone else kinda frustrated with the approach that was taken with D7 fields in comparison to D6 CCK? I'm sorry but a website that is created today, is not going to be the same website in two years. Things change, people learn and then go back to tweak their initial design. It's almost like the idea in the sprints was to lock things down to make it easier and safer for the new guy, except that the new guy doesn't know enough about what he's doing, to be able to properly sit back and "design" the site for what it will become in two years.

Sorry for the rant but I've run into more than one situation with fields where I can't do what I used to be able to do, just like this topic is describing. I'm not usually one to complain but I'm hoping that the squeaky wheel will eventually get some oil in this area of Drupal.

The approach in #12 works but the change isn't reflected in the settings. Couldn't they be easily overridden?

Why not just do a hook_form_alter() and changing the field to a textfield? Would that break something later on?

Couldn't there be a simple "Field Settings" page that would allow overall customizations such as this? I don't really know what other customizations could be done, but I'm sure somebody could think of one : )

This issue belongs to drupal core field system I think.

so I created this issue : http://drupal.org/node/1588624

Yes, #1588624: Can field cardinality (number of values) range be other than 1-10, unlimited ? replaces this issue. The issue title maybe not clear enough, but the patch specifies a "higher top limit" by letting the user type what he want as "number of values" as talked in #14.

So may I change the status to "fixed" because the problem isn't here but in the drupal core field system.

Features need to go into d8 first.

An addition on teseting #16:
The number 0 is allowed, which was not in the old situation, generating an indetermined situation.

The last submitted patch, cardinality_validation.patch, failed testing.

As @marcingy mentioned, this would need to be patched in Drupal 8 first, per the backport policy.

Hello,

with #6 it's ok for me. But is it possible to create a new function in template.php not to change the code in Drupal core? If we update Drupal, we lost the patch.

Thanks.

What about for D6?

Tagging also

That looks great !

I'd also suggest reducing the number of values in the select - 1 to 4 or 5 should be enough, then you use "More".
(putting that value in a variable would probably help for adjustments and make the code clearer)

We should try to bring a FAPI / UI guru to help with the container inline stuff, I suck at it too :-(

The last submitted patch, 680546-36.patch, failed testing.

Fixed tests + added some additional tests for the new element.

Better patch so in case something else than a numeric value is submitted to the 'other' field, the number validation will show a nicer error message. I think this one can be set RTBC.

Makes sense indeed, old-school interdiff!

What's wrong with #type => 'container'?

Also is there a generic 'select or other' issue somewhere? It'd be good to see if we could implement this as a fapi type rather than the two fields.

How the hell did that happen.

What about replacing the select element by a checkbox for "unlimited"? The select and textfield elements have a functional overlap which reminds me of the date selector we have/had in core: a select element with loads of possible options and one option for custom formats that made a textfield pop up. People tended to overlook the "custom" option.

The textfield is hidden with states when necessary, so it works fine as intented.

I'm not saying it doesn't work as intended. I'm only saying we have/had similar behavior with date formats which didn't work as well as expected, and that there is an overlap between what the select and textfield elements offer. My suggestion was to use a checkbox instead of a select list (so the #states behavior can be kept).

Let's keep it originally, bojhan already approved, no need to bikeshed this again.

@yoroy You're 2 times right. those are Radios, and we want this in and then eventually follow-up.

Looks good. Setting back to RTBC.

Nitpick: Maybe "title or description properties" would be better?

The test needs to be updated now, it's still looking for "Please enter a number."

@Cottser - crosspost hehe. Could be that 'properties' is better, English is not my mother tongue, so I probably miss some details now and then, feel free to reroll :)

And back.

#64: 680546-62.patch queued for re-testing.

Looks like catch has been involved in this one.

Mmmm, this change led to a quite confusing form value structure. The 'container' in there does not really make sense.

Attached patch fixes that.

True, the container should be transparent to the structure of the resulting form values.

#71: drupal8.field-cardinality-other.71.patch queued for re-testing.

#71: drupal8.field-cardinality-other.71.patch queued for re-testing.

The last submitted patch, drupal8.field-cardinality-other.71.patch, failed testing.

The last submitted patch, drupal8.field-cardinality-other.78.patch, failed testing.

Yup, better now :-)

#78: drupal8.field-cardinality-other.78.patch queued for re-testing.

Yes please lets get this in, and also backport it to D7.

Is the custom option going to be called "More" as in the screenshot in #56? I looked at the latest patch, but couldn't find the label text for the custom value.

If it is "More", then that is a not so good choice from an UX perspective. Simply using "Custom" in the drop down would be best.

But maybe I have misunderstood the screenshot?

@Cottser - Ahh, did look for if a commit had been made, obviously I didn't catch that :)

Would be great if you could post screenshots of the UI, including with the drop down in full view. That would give me enough to really understand the UX going on here. Particularly it will give me a better idea of how that "More" is used.

@Cottser - Thanks for so quickly posting those screenshots.

"More" is definitely the wrong choice here. More for me translates into that something is missing from the dropdown. Also, when it is selected it says "More 6" and what does that actually mean, more 6:s, more than 6???

In my opinion "Custom" is the common term used for this and it also makes a lot more sense since above you have static option, unlimited included, and then Custom wont cause any confusion.

Great if you can include that in the follow up patch your working on.

Just to confirm. Is the input field always shown (since its visible with a 6 in the screenshot)? It should only be visible when the Custom option is selected.

@swentel - Thanks for clarifying that, it wasn't obvious for me just looking at the screenshot.

@Cottser - Thanks for pointing me to that issue. Particularly since it has the ambition to create a universal standard.

Also thanks for the quick replies and turnaround of this issue.

The "Custom" change didn't get included in that patch, so here is the updated patch with an interdiff thrown in for good measure.

Yeah, sorry, I was the one who asked for "More" rather than "Other / Custom". I'm fine with "Custom" if UX people like it better.

A couple minor points on #96 :

['cardinality']['cardinality'] is not too great. 'cardinality_container' ? 'cardinality_wrapper' ?

Can we avoid the "by ref" magic ? I find it more confusing than useful here, and it's actually not needed at all for $cardinality_other.

#96: 680546-96.patch queued for re-testing.

Makes sense, I'll code this later

xpost

Thanks swentel!

something like that http://jsfiddle.net/LXtZU/

Ok, this cleans up the container form structure and also addresses the select box problem, to be fair, this makes much more sense indeed.

- edit - This also simplified the tests a lot.

Tried the patch, I think it works pretty well.

- would be good to size the number field a bit smaller
- screenshot has some wording suggestions: lets not talk about users at all, use limited/unlimited and not repeat some of the keywords etc.

(wrong screenshot)

Fixes the test and implements the suggestions from #110.
Setting the size seems to be impossible though for the number element.
The validate function could also be removed, yay for cleanup!

This looks great. Also love @yoroy's simplification.

There's only one trap here: The description makes it sound as if the "Add another item" button would only appear when "Unlimited" is selected. But that's not the case. Field widgets always progressively advance into more rows, unless the limit is smaller than 3 (I believe) — i.e., you always get an "Add another" button, unless the widget shows the maximum allowed items that can be entered already.

I wonder whether we need any description at all though?

Isn't "Unlimited" and "Limited" (+ value) crystal clear already?

@swentel: Regarding #size, just add 'size' to form_pre_render_number(), please. The HTML5 DOM Interface for input type=number still declares it as attribute unsigned long size. I've done exactly that change for some other patch in the queue already, but can't remember which one. Happy to do it here.

The changes for #size look fine to me. Browsers usually size the field a bit larger than the specified size. Did you try smaller values, like 3 or 2 or 1?

Let's try a smaller #size of 2. If that still doesn't work, no biggie, let's move forward.

The situation with the Add another button appearance with regard to unlimited/limited sounds like it would be best to remove the description entirely for now, in combination with a follow-up issue to investigate WTF is actually happening there? ;)

File widgets re-implement their own handling of "multiple widgets + add more".
Can't remember the exact reasons, maybe @quicksketch does - had to do with the "remove" button, that is specific to file / image widgets.

Changes

• Strict comparison
• Fixed whitespace
• Bring back validation +test - was needed indeed.
• Use $has_data with #disabled property to prevent changing limit in case there's data - that was a regression long before this patch
• Use 'limit' instead of 'Number' for hidden title
• Removed obsolete prefixes - couldn't use the container #type though as the title started floating as well - unless I'm doing something wrong

The last submitted patch, 680546-119.patch, failed testing.

Removed the uncommented tests.

Well, because you say please :)

Tested it and it works fine here.