Problem/Motivation

When configuring a field that already has data and clicking on the "Field settings" tab, the user gets this alarming error:

However, the error is a lie. You can change anything on the page. The only time you can't is if you change the field cardinality to be less than already-stored values:

This was fixed in #1266748: Changing cardinality lower than highest existing delta causes data loss upon save.

However, the original error message is still wrong, and creates a bad user experience both by making an alarming message and by making the software seem buggy because the message is proven wrong.

Proposed resolution

  • Remove the current message.
  • Reword the current nonthreateneing message:
    These settings apply to the Image field everywhere it is used. These settings impact the way that field data is stored and the defaults that are provided.
  • Provide a clearer validation message when the allowed values are reduced below the current values in the field.

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#25 data-length-disabled.png153.45 KBbnjmnm
#22 message-field-ui-after-patch.png80.79 KBjoaopauloc.dev
#22 message-field-ui-before-patch.png87.44 KBjoaopauloc.dev
#22 error-message-field-ui-after-patch.png91.36 KBjoaopauloc.dev
#22 error-message-field-ui-before-patch.png91.4 KBjoaopauloc.dev
#21 2895124-21.patch6.67 KBlauriii
#8 interdiff-2895124-8.txt2.15 KBxjm
#8 field_ui-2895124-8.patch6.24 KBxjm
#6 reducing_allowed_values.png135.59 KBxjm
#6 field_ui_message.png141.22 KBxjm
#5 2895124-5-interdiff.txt1.9 KBxjm
#5 field_ui-2895124-5.patch4.09 KBxjm
#3 field_ui_after.png180.87 KBxjm
#3 2895124-interdiff-2.txt1.77 KBxjm
#3 field_ui-2895124-2.patch3.75 KBxjm
#2 field_ui-2895124-1.patch4.17 KBxjm
reduce_cardinality.png116.47 KBxjm
change_everything.png108.39 KBxjm

Comments

xjm created an issue. See original summary.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new field_ui-2895124-1.patch4.17 KB
xjm’s picture

xjm
she/her
Primary language English
commented
Issue summary: View changes
StatusFileSize
new field_ui-2895124-2.patch3.75 KB
new 2895124-interdiff-2.txt1.77 KB
new field_ui_after.png180.87 KB

Better place in the form markup for the description, and a screenshot of the same test as in the summary with the patch applied.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs review » Needs work

Hm, the word "cardinality" does not appear elsewhere in the user interface.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new field_ui-2895124-5.patch4.09 KB
new 2895124-5-interdiff.txt1.9 KB

So come to think of it, we don't really need the description on the allowed values field; we just need a clearer validation error when the user tries to reduce the value too much.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue summary: View changes
StatusFileSize
new field_ui_message.png141.22 KB
new reducing_allowed_values.png135.59 KB

Status: Needs review » Needs work

The last submitted patch, 5: field_ui-2895124-5.patch, failed testing. View results

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new field_ui-2895124-8.patch6.24 KB
new interdiff-2895124-8.txt2.15 KB

Just fixing the test failures, because it's not going to match the period I changed to a comma.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Yay!

The issue title is not 100% correct, as it is not just the cardinality that has restrictions, it's basically up to each field type to define that. But the descriptions are accurate and look great to me.

I also can't imagine that we don't already have an issue for this, but maybe we really don't.

And we might want a separate issue to discuss making the upload destination configurable, because right now it's not even though there is IMHO no reason for that, if anything, we could add a description that it will only affect new files and not existing ones. See \Drupal\file\Plugin\Field\FieldType\FileItem::storageSettingsForm()

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
StatusFileSize
new 2895124-21.patch6.67 KB

Rerolling for 10.1.x

joaopauloc.dev’s picture

joaopauloc.dev commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new error-message-field-ui-before-patch.png91.4 KB
new error-message-field-ui-after-patch.png91.36 KB
new message-field-ui-before-patch.png87.44 KB
new message-field-ui-after-patch.png80.79 KB

Hey.
Tested the patch and works fine.
See attachments for evidence.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Needs followup

There is unaddressed feedback about a possible followup and/or possibly adding a description that it will only affect new files and not existing ones:

And we might want a separate issue to discuss making the upload destination configurable, because right now it's not even though there is IMHO no reason for that, if anything, we could add a description that it will only affect new files and not existing ones. See \Drupal\file\Plugin\Field\FieldType\FileItem::storageSettingsForm()

...Or at least that's the only reason I can see that @Berdir did not RTBC this six years ago. :)

IMO that scope could be handled separately. The more help text there is, the worse the UX. The current patch improves the current state significantly, but I'm not sure if other, semi-related additions would or if they belong in the same patch.

Can't commit this as it's my patch to begin with.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

It would good to see a screenshot here showing what happens if you try to change the length of a text plain field that already has data

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Issue tags: -Needs followup
Related issues: +#3336188: Consider making file field upload destination changeable even after data exists
StatusFileSize
new data-length-disabled.png153.45 KB

Created #3336188: Consider making file field upload destination changeable even after data exists to preserve the suggestion in #9

It would good to see a screenshot here showing what happens if you try to change the length of a text plain field that already has data

OK!

  • bnjmnm committed 0891913c on 10.1.x 
    Issue #2895124 by xjm, lauriii, joaopauloc.dev, Berdir: Field storage...

  • bnjmnm committed 84efb814 on 10.0.x 
    Issue #2895124 by xjm, lauriii, joaopauloc.dev, Berdir: Field storage...
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Reviewed & tested by the community » Fixed

Resisted the urge to bikeshed phrasing minutae because this is a very clear improvement over what was very misleading UI experience. This should have happened long ago and nitpicking text can happen in a followup if needed. Lets take care of the blight.

Committed to 10.1.x and cherry-picked to 10.0.x since it is a bugfix and not particularly disruptive.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Issue tags: +Field UX
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3324140: Convert field_storage_config and field_config's form validation logic to validation constraints

Nice! 🤩

Looking forward to moving this to proper validation constraints next in #3324140: Convert field_storage_config and field_config's form validation logic to validation constraints 🤓

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.