Fields with unlimited cardinality show 1 extra input field

Created on 20 June 2018, about 7 years ago
Updated 19 January 2023, over 2 years ago

If a field has unlimited cardinality, then it is always displayed with one input too many. For example, I added a textfield named "Labels" to a content type and gave it unlimited cardinality. I create three label values, as illustrated here:

Then I Save the form and go to Edit it again. When I do, one extra Label input has been added, even though I have not clicked the "Add another item" button. And there's no way to get rid of this extra input field:

Always adding an unused text field gives users the impression that the form was not completely filled out even when it was, or that they clicked the "Add another" button when they did not.

I tracked the source of the "bug" down to a single line in the WidgetBase class. It looks like an off-by-one error to me, with respect to the difference between how the number of elements is calculated for a fixed, multiple cardinality field vs. how the number of elements is calculated for an unlimited cardinality field.

🐛 Bug report
Status

Needs review

Version

10.1

Component
Field 

Last updated 3 days ago

Created by

🇺🇸United States lisastreeter

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Status changed to Needs work over 2 years ago
  • 🇧🇷Brazil joaopauloc.dev

    Hey @darren-oh, I will adjuste that.
    Also I'm having some problems to fix this test failing. This particulary test are failing without my patch. I don't know how to fix it.

  • 🇺🇸United States smustgrave

    FYI the ultimate fix will be covered in Initial number of fields visible on the form should be configurable for fields set to unlimited value Active truly doubt this one will be merged.

  • 🇧🇷Brazil joaopauloc.dev

    Thanks @smustgrave, I was trying to fix the php units that are failing.
    Also the approach to have config to set how many empty items to be displayed is better.

    Should we close this issue?

  • Issue was unassigned.
  • 🇺🇸United States darren oh Lakeland, Florida

    The issues are similar but not the same.

  • Status changed to Needs review over 2 years ago
  • 🇧🇷Brazil joaopauloc.dev

    No matter if this issue will be merged or not, I spent too much time investigating this issue to not provide the possible fix that I found :). If the issue Initial number of fields visible on the form should be configurable for fields set to unlimited value Active is still not merged someone can use this patch.

    Removing extra input when the field settings have a default value. See the picture attached.

    With this change, we don't have an extra input by default in any situation, only when the user clicks in add more button.
    Also, the test that failed for me doesn't make sense so I changed how the test works. But I think is necessary to be validated by someone else.

    This is the test that failed.

    Drupal\Tests\field\Functional\FormTest::testLabelOnMultiValueFields
    Behat\Mink\Exception\ExpectationException: The string "alert('a configurable field');" was not found anywhere in the HTML response of the current page.

    The test check $this->assertSession()->assertEscaped("

    alert('a configurable field');

    ");
    But when we create a field with this label the tag script is filtered and the label becomes alert('a configurable field');
    So I think the correct way to validate if the tag script is filtered is to check if the tag script doesn't appear on the page as my changes do.
    But will amazing if someone else confirms that in a code review.

    $this->assertSession()->pageTextContains("alert('a configurable field');");
    $this->assertSession()->pageTextNotContains("

    alert('a configurable field');

    ");

  • 🇮🇳India prasanth_kp

    @joaopauloc.dev thanks for the patch. #58 🐛 Fields with unlimited cardinality show 1 extra input field Needs work Patch Applied on 10.1.x-dev and it fixes the issue.

  • Status changed to Needs work over 2 years ago
  • 🇧🇷Brazil joaopauloc.dev

    Didn't work as expected when we use unlimited fields with paragraphs.

  • 🇦🇺Australia dpi Perth, Australia

    Seems to me like there might be a lot of crossover between this and Fixed maximum number of field values, but use «add more» similar to when cardinality «unlimited» is used Needs work

    Maybe both should be triaged into a _Optimising add more button behavior_ issue

  • 🇩🇪Germany Anybody Porta Westfalica

    I like this idea, but I would suggest that this should be something that could be configured in the widget settings.

    Strongly agree! :)

  • 🇧🇪Belgium gorkagr

    Hi!

    Thanks joaopauloc.dev for the patch. Unfortunately i could not apply it for 9.5.x so here it goes a path for that version.

    I have noticed a duplicated declaration of $context that you have removed in your patch (I think the reason it does not work with 9.5.x), but i have opened an issue ( https://www.drupal.org/project/drupal/issues/3378657 🐛 Duplicate declaration of $context in WidgetBase Fixed ) to spot that duplication and remove it, so i have omitted it from my patch.

    PD: I have not tested it within fields inside paragraphs, but once i have a bit of time, i will check this too

    Best

  • last update almost 2 years ago
    Custom Commands Failed
  • Pipeline finished with Failed
    almost 2 years ago
    Total: 879s
    #19675
  • last update almost 2 years ago
    30,048 pass, 12 fail
  • Pipeline finished with Failed
    almost 2 years ago
    Total: 1081s
    #19677
  • 🇧🇷Brazil joaopauloc.dev

    Hey folks, I created a new merge request regarding the comments above, the number of extra inputs to be displayed should be a configuration.

    A new configuration was added to the field storage settings, like the image below:

    When the unlimited option is selected a new field is displayed for the user to choose how many extra inputs will be displayed.

    1. By default, I choose 1, because this is how it works today, one extra input appears.
    2. If the user selects 0, no extra input will appear, if the user is adding the form entity. Only one input will appear.
    3. If the user selects 0 and is editing the form entity, if there is no value one input will appear, otherwise only the input with value populate will appear without any extra input.

    Example:
    Field with 3 extra inputs to be displayed:

    So, for the editing form, as I configured 3 extra inputs but populated only 2, any extra input will appear.

    Once we define a number of extra inputs to be displayed when the user is adding the number of inputs selected will appear, If the user is editing but there is no value added previously, the number of inputs keeps the configuration, since the user added more or fewer items that were configured, only the values populated is displayed without any extra input.

    Maybe we need to discuss the rules mentioned above, how we should display the inputs if the user is editing.

  • last update almost 2 years ago
    30,056 pass, 11 fail
  • Pipeline finished with Failed
    almost 2 years ago
    Total: 1584s
    #19706
  • last update almost 2 years ago
    30,150 pass, 4 fail
  • Pipeline finished with Failed
    almost 2 years ago
    Total: 1235s
    #19734
  • last update almost 2 years ago
    27,602 pass, 894 fail
  • Pipeline finished with Failed
    almost 2 years ago
    Total: 1682s
    #20049
  • last update almost 2 years ago
    30,156 pass, 2 fail
  • last update almost 2 years ago
    30,162 pass
  • Pipeline finished with Failed
    almost 2 years ago
    Total: 1437s
    #20483
  • Status changed to Needs review almost 2 years ago
  • Status changed to Needs work almost 2 years ago
  • 🇺🇸United States smustgrave

    If we are going to change the schema we will need an upgrade path with tests.

  • 🇩🇪Germany Anybody Porta Westfalica

    There's another aspect, which makes this fix major from my perspective. If the added field is a required field (or contains a required field in a compound custom widget type), there's no way to remove the extra field and you're unable to submit the form with the required field!

    That makes using #required impossible on custom compound field widgets!
    But without the required indicator, the user can't see which compound fields are required, until submit (validation)!

    So the best solution is to not add the new line automatically and be able to remove it.
    Just ran into this when implementing Homebox 3.x.

  • 🇩🇪Germany Anybody Porta Westfalica
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update over 1 year ago
    Build Successful
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update over 1 year ago
    Build Successful
  • 🇺🇸United States edwardsay

    Adding 10.2 support for patch #3

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 5.3 & MySQL 5.5
    last update over 1 year ago
    Patch Failed to Apply
  • Assigned to sathishkumar
  • Issue was unassigned.
  • 🇮🇹Italy plach Venezia

    @sathishkumar

    Thanks for the patches, but the changes will be merged to the development branch first, then backported.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update over 1 year ago
    29,689 pass
  • 🇦🇺Australia pameeela

    I can't reproduce the behaviour reported in #69 on D11.

    Steps:

    1. Add a new field with unlimited cardinality
    2. Add a node with one value
    3. Edit the node, see the extra input
    4. Save

    Bumping down to normal, unless someone has different steps to reproduce that? I am 50-50 on the usability aspect, I could see both sides of the argument.

  • 🇮🇳India prashant.c Dharamshala

    prashant.c changed the visibility of the branch 11.x to hidden.

  • Pipeline finished with Failed
    about 1 year ago
    Total: 478s
    #260189
  • 🇮🇳India prashant.c Dharamshala
    • Rebase the current PR branch with 11.x
    • Tested on Drupal 11 have following points
    1. The placement of the section Extra inputs to displayis not good from the usability point as it giving an impression that the cardinality text field is standalone, it has moved the cardinality text field which should be next to the dropdown Allowed number of values
    2. When the cardinality has been specified, the value in the Extra inputs to display text field should be validated for the max value either on Form submit or on value change, currently it allows greater values than cardinality, however on the frontend displaying the fields correctly but the value is still allowed to be entered

    Thanks!

  • 🇳🇿New Zealand quietone

    I was triaging Remove auto added empty field from multiple values field. Needs work and came here because this seems like a duplicate. In fact this was closed as a duplicate of a different issue and credit was moved over there. That was issue Initial number of fields visible on the form should be configurable for fields set to unlimited value Active . After reading three incomplete issue summaries and the comments on three issue, I have decided that this one should go back to the original intention, which was a UX issue. And, since this is the older issue and has more discussion I am closing Remove auto added empty field from multiple values field. Needs work as a duplicate and moving credit. The work here, from about #47 is out of scope and should move to Initial number of fields visible on the form should be configurable for fields set to unlimited value Active , the original issue to add the extra configuration.

    This issue is solely about the UX of the extra blank field that appears on edit. See the remaining tasks for what needs to be done.

    Everyone, please keep the issue summaries up to date. Incomplete summaries makes it more difficult to work with an issue and to get it to commit. Thanks.

  • 🇩🇪Germany Anybody Porta Westfalica

    Re #76 I still think this is Major, see the reason given in my comment in #69.
    That also means @quietone (#80): I wouldn't say this is *only* UX, it's also a heavy logical validation issue for all cases where this applies to compound fields (widgets) like IEF ( 🐛 Required fields make an optional IEF (erroneously) required Needs work ) and others.

    Until this issue is fixed, if the additionally added empty entry, if containing a required field, blocks saving the parent entity due to

    • Form validation (could be circumvented in the module, but logically it would just be better to never have that empty entry!)
    • HTML5 required attribute validation (can just be hacked)

    So getting this fixed isn't only UX for more complex widgets than just autocomplete.
    For that reason, I'm setting this back to Major, but of course we can still discuss that. Even better: Let's just fix it and also unblock Initial number of fields visible on the form should be configurable for fields set to unlimited value Active then.

  • First commit to issue fork.
  • Pipeline finished with Failed
    24 days ago
    Total: 171s
    #577347
  • 🇩🇪Germany Grevil

    I don't think we should add the new "cardinality_display" setting on the field definition? Outside the Widget, this setting seems to be quite useless IMO.

    I think it should be rather implemented as a Widget Setting, which only appears if the cardinality is set to above 1 (including -1 for unlimited). I'll create a second branch for this, so we can always go back, to the original field definition solution by @joaopauloc.dev.

  • Pipeline finished with Failed
    24 days ago
    #577432
  • Pipeline finished with Canceled
    24 days ago
    #577456
  • Pipeline finished with Failed
    24 days ago
    #577459
  • Pipeline finished with Running
    23 days ago
    #577483
  • 🇦🇺Australia pameeela

    Based on the screenshot in #78 🐛 Fields with unlimited cardinality show 1 extra input field Needs work , I'm concerned we are trading one usability problem for another. This increases the complexity of this form widget a lot.

    I agree that having the empty widget causes confusion and have had this complaint from clients. I can see the case for having it but I don't really think this needs to be a setting in the UI. If people really want to be able to override it, we can still have the configuration and allow code overrides?

  • 🇩🇪Germany Anybody Porta Westfalica

    @pameeela, thanks for the feedback. That's not a bad idea. In core we could add methods for the programmatic part, but leave out the widget settings (maybe) and implement them in contrib, if anyone needs it?

    So we'd have clean solution in core available, but not cluttering the UI!

    One difficult thing @grevil and me had issues with is the difference between:

    1. Showing an empty line if the field is not filled at all (Which might be expected for many field types, like autocomplete or text, but not for others, especially complex ones or compound with required fields that fail then, like IEF)
    2. Showing an additional empty line if there are values present already (current behavior)
    3. Showing a number of lines initially (similar to what https://www.drupal.org/project/sam and Add multiple items at once Active does)

    All are different valid cases that the Core API should support better!

    Both need to be handled separately for an excellent and complete solution in our eyes!

    Might this be a topic for the UX meetup?

  • 🇦🇺Australia pameeela

    Of course, it is never simple! And I agree also that having better support for configuring this generally would be ideal.

    As for this specific issue, I would say by default:

    • If the field is empty, show one
    • If there are values, don't show an extra one

    And then add support for configuring both of those in code?

  • 🇩🇪Germany Anybody Porta Westfalica

    @pameeela: Great! Agreed on that! Here are the details I'd like to propose:

    If the field is empty, show one

    I'd say we should solve that using the cardinality_display variable, defaulting to 1 so it can also be set to 0 or any number of fields to show generally (<= cardinality)

    If there are values, don't show an extra one

    I'd vote for a boolean option for this one. Do you have an idea for a good machine name?
    Maybe sth. like:

    • add_one
    • add_blank
    • additional_input
    • blank_input
    • ...?

    I'm not a native speaker, so maybe you have a better idea! :)

    Once we're clear on these points, let's do it!

  • 🇪🇸Spain ckrina Barcelona

    Agreed with the latest comments: as @pameeela suggested, if settings can be moved out of the UI would help not increasing Drupal's complexity. And since several of these assumptions don't follow a specific or generic UX rule and can change based on project & form specific case (but are necessary decisions to be taken) a setting in code is the best solution.
    Just make sure the work happens on the right issues as @quietone suggested: specific and focused changes have better chances to get in, faster (it took me 30min to understand what happened in this issue).

Production build 0.71.5 2024