This is a breakout issue from #516138-139: Move CCK HEAD in core.

I still think that the first screen upon adding a field is confusing, but otherwise adding, configuring and deleting fields seem to work all as expected to me. If I was not clear above, I'd repeat this initial screen issue:

- add a field to the article content type
- you got redirected to a page with settings for the field
- BUT the field has no global settings, and even though we are apparently under the Article content type (see page title), Article specific settings are not displayed either
- go to manage fields again and to edit the particular field and now you get both Article specific settings and actual global settings (which in the previous screen were said to be nonexistent)

Initial screen on adding the field:

Going back to the "same" screen in the navigation:

I'm not going to push for solving this before committing, but it is certainly a very core confusion point IMHO.

Files: 
CommentFileSizeAuthor
#99 552604-99.patch30.16 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 48,957 pass(es).
[ View ]
#97 confusing-s00a-nopatch-editfield-New Thing | confusing-552604-92 2012-11-27 10-35-42.png85.07 KBYesCT
#97 confusing-s00b-fieldsettings-New Thing | confusing-552604-92 2012-11-27 10-37-03.png47.64 KBYesCT
#97 confusing-s01-add-field-manage-fields-2012-11-27_1004.png129.41 KBYesCT
#97 confusing-s02-after-save-global-field-settings-2012-11-27_1006.png105 KBYesCT
#97 confusing-s03-non-global-settings-edit-New Thing settings for Article | confusing-552604-92 2012-11-27 10-09-10.png58.64 KBYesCT
#97 confusing-s04-onenode-hascontent-2012-11-27_1010.png53.61 KBYesCT
#97 confusing-s05-managefields-2012-11-27_1013.png114.34 KBYesCT
#97 confusing-s06-editfieldwithcontentexisting-New Thing settings for Article | confusing-552604-92 2012-11-27 10-14-56.png58.05 KBYesCT
#97 confusing-s07-fieldsettingswithcontentexisting-2012-11-27_1016.png103.83 KBYesCT
#92 552604-92.patch23.94 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 48,793 pass(es).
[ View ]
#83 drupal8.field-instance-settings.83.patch23.66 KBsun
PASSED: [[SimpleTest]]: [MySQL] 48,778 pass(es).
[ View ]
#81 drupal8.field-instance-settings.81.patch20.52 KBsun
PASSED: [[SimpleTest]]: [MySQL] 47,870 pass(es).
[ View ]
#73 drupal8.field-instance-settings.73.patch18.03 KBsun
FAILED: [[SimpleTest]]: [MySQL] 47,822 pass(es), 10 fail(s), and 4 exception(s).
[ View ]
#73 interdiff.txt6.64 KBsun
#72 interdiff.txt5.64 KBsun
#66 552604-field-ui-65.patch21.58 KBLukas von Blarer
FAILED: [[SimpleTest]]: [MySQL] 47,792 pass(es), 12 fail(s), and 4 exception(s).
[ View ]
#66 552604-field-ui-interdiff-65.txt1.62 KBLukas von Blarer
#66 Screen Shot 2012-11-06 at 4.35.53 PM.png19.24 KBLukas von Blarer
#66 Screen Shot 2012-11-06 at 4.36.12 PM.png51.9 KBLukas von Blarer
#66 Screen Shot 2012-11-06 at 4.37.08 PM.png26.49 KBLukas von Blarer
#66 Screen Shot 2012-11-06 at 4.37.39 PM.png61.07 KBLukas von Blarer
#66 Screen Shot 2012-11-06 at 4.36.32 PM.png48.24 KBLukas von Blarer
#62 drupal8.field-instance-settings.62.patch17.13 KBsun
FAILED: [[SimpleTest]]: [MySQL] 37,234 pass(es), 31 fail(s), and 32 exception(s).
[ View ]
#45 drupal.field-instance-settings.45.patch19.08 KBsun
PASSED: [[SimpleTest]]: [MySQL] 29,829 pass(es).
[ View ]
#44 list-field-settings.png22.24 KBsun
#44 list-instance-settings.png21.02 KBsun
#44 text-field-settings.png20.75 KBsun
#44 text-instance-settings.png26.1 KBsun
#43 drupal.field-instance-settings.43.patch17.83 KBsun
PASSED: [[SimpleTest]]: [MySQL] 29,659 pass(es).
[ View ]
#41 field_ui_instance_settings_form-552604-41.patch12.27 KByched
FAILED: [[SimpleTest]]: [MySQL] 29,067 pass(es), 24 fail(s), and 20 exception(es).
[ View ]
#39 drupal.field-instance-settings.39.patch7.93 KBsun
FAILED: [[SimpleTest]]: [MySQL] 29,061 pass(es), 57 fail(s), and 34 exception(es).
[ View ]
#29 drupal.field-instance-settings.29.patch7.02 KBsun
FAILED: [[SimpleTest]]: [MySQL] 28,981 pass(es), 57 fail(s), and 34 exception(es).
[ View ]
#28 drupal.field-instance-settings.28.patch6.34 KBsun
FAILED: [[SimpleTest]]: [MySQL] 28,925 pass(es), 57 fail(s), and 34 exception(es).
[ View ]
#12 first_page.png16.32 KBlambic
#12 second_page.png16.38 KBlambic
#9 552604.patch1.14 KBlambic
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 552604_1.patch.
[ View ]
#5 552604.patch1.08 KBlambic
Passed: 14740 passes, 0 fails, 0 exceptions
[ View ]
#2 552604.patch1.41 KBlambic
Failed: Failed to apply patch.
[ View ]

Comments

Component:field system» field_ui.module

StatusFileSize
new1.41 KB
Failed: Failed to apply patch.
[ View ]

I'm attaching a patch to fix the title issue, but ideally the first screen should be skipped completely when there are no applicable settings.

Status:Active» Needs review

Status:Needs review» Needs work

The last submitted patch failed testing.

StatusFileSize
new1.08 KB
Passed: 14740 passes, 0 fails, 0 exceptions
[ View ]

rerolled patch

Status:Needs work» Needs review

Sorry, but requires screenshot

Let's also remove that check_plain() w/ drupal_set_title() since it would be run through that function twice.

StatusFileSize
new1.14 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 552604_1.patch.
[ View ]

check_plain() removed.

@lambic How can you not make a screenshot, with check_plain() removed? It should be beautiful now :P

+  $title = isset($instance['title']) ? $instance['title'] : $instance['label'];

I might be missing something, but I don't see where $instance['title'] would come from ?

StatusFileSize
new16.38 KB
new16.32 KB

finally got around to screenshots:

Applied the patch in #9 and this a definite improvement.

Issue tags:-Usability, -Fields in Core

#9: 552604.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Usability, +Fields in Core

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

Priority:Normal» Major

Back looking at this workflow again after awhile, I really think this is a bad enough problem we ought to see if we can do something to smooth it out before release. What, I don't really know, since it's quite late. But while it might make sense to Field API folks why adding a field results in two settings screens shown right after another, the second with exactly the same options as the first (only at the bottom this time for extra pizazz!), I can assure you that no one else on the entire planet will understand why this happens.

Over at #867968: Field UI settings workflow is dumb, Bojhan writes:

Trying to add some clarity to this discussion on actual steps that need to be taken.

1. Correct indicators, where you are in the process of creating a field.
2. Correct labels moving between pages, ie.. "Save and continue" , "Create field"
3. The actual widget settings page is not designed, but pulled together there is no coherence to its hierarchy and ignores most Drupal core UX standards.
4. There are two setting pages [postpone - unless we can easily get it done]

I would suggest if you want to actually move this before we release Drupal 7 is to create individual issues for example those outlined above.

To me, #4 - the fact that there are two settings pages, with no remotely obvious difference between the two - is the biggest UX wtf here. I do not understand why the first one is even needed, since presumably if I navigated away from the first settings form without saving it, my entire site would not explode, which means that Field API must be doing something smart under the hood in terms of defaults. But the rest of those sound good, too. Separate issues might make sense, but let's come up with a plan first.

Can someone remind me again why we have two settings pages in the first place, and can't just immediately show the widget configuration form assuming the defaults from the field?

Sigh. I'm already buried deep down in #616240: Make Field UI screens extensible from contrib - part II ...

Part of the issue is #821390: Fields usable before all steps to create them are completed. : The forms you go through when creating a field are in fact just the edit forms for the field and the instance. You're editing a field that has been created with default settings and properties the moment you pressed 'Save' on the first screen ('Manage fields')

Yep, agreed there on the edit form thing. So forcing users through this workflow is really awkward, as we don't do this elsewhere in core (congratulations, you just created something! here, now edit its settings. and now edit MORE of its settings!). I would say just ditch the redirection to the settings forms altogether except for the fact that it's not at all clear that clicking a link titled "Autocomplete term widget (tagging)" will take you to a settings form, so I'm not sure how someone would ever find their way back there (actually, that's true even now).

What was the reason again that we can't just stick all of the settings on admin/structure/types/manage/article/fields/field_tags like it was with CCK, instead of having 3 separate settings pages? Then that at least would mesh better with our existing UX elsewhere, which is "to change something's settings, click the edit link next to it".

- separate form to change the widget type:
Because the current widget has an effect on the content of the instance settings - back then #ajax was not where it is today.
This being said, I've been thinking lately that we could let users pick and change the widget with a select dropdown straight from the 'Manage fields' table, rather than linking to a separate form that has a single select.

- separate form for field settings:
Because in D6 everybody screamed that the one-form-including-field-and-instance-settings was too damn big and scary.
Also, in D7, with storage abstraction niceties, some *field* settings (the ones that would affect the schema) cannot be changed after the field creation. While you can change anything at the instance level (doesn't affect schema anyway).
This being said, when doing the initial port of D7 CCK in contrib, Karen left a copy of the 'field' settings form at the bottom of the instance settings form, and this was left as is in #516138: Move CCK HEAD in core...
I've been willing to write a patch to remove that bottom 'field settings' fieldset for weeks :-/.

Also of note:
The 'instance settings' form actually contains
- widget settings, that only affect the how the field is edited, and which can vary depending on the selected widget
- and instance settings : properties of the field that can vary from a content type to the other (required, label, allowed file extensions...), that will be there no matter what the widget)
Those two garbled and intertwined in the same fieldset.
While working on #516138: Move CCK HEAD in core, I advocated for a clearer separation of the two, but quicksketch voiced against.

Cool, thanks for the brain-dump, yched. I'll do some thinkering.

Anyone else, feel free to chime in with suggestions, too.

Thx webchick, much appreciated :-)

Probably not of much help here, but I think:

+1 for clearly separating field from instance from widget settings. If there's a technical design concept behind it, then it doesn't make sense to try to hide that concept in the UI. First source of confusion.

+1 for either directly jumping to instance settings or back to "Manage fields" overview page after creation. The field settings page is the least interesting, and changing configuration options most probably is a rare action anyway, as you're performing site-wide changes to the field.

In general, I think that we badly need to clarify the on-screen tips and descriptions, so as to make users understand when they ought to re-use an existing field, and in which common cases it isn't really a good idea. I'm struggling myself in trying to explain it.

+1 for more #ajax. However, given that the other ajaxified field formatter settings patch was relatively simple, we would have to start with "ideal world mockups" here.

Issue tags:+string freeze, +API change

Can we distill what's still possible to do for D7? Some of the sub-issues raised here are indeed major, some others are too late for D7.

subscribe

Status:Needs work» Needs review
StatusFileSize
new6.34 KB
FAILED: [[SimpleTest]]: [MySQL] 28,925 pass(es), 57 fail(s), and 34 exception(es).
[ View ]

Remove confusion.

StatusFileSize
new7.02 KB
FAILED: [[SimpleTest]]: [MySQL] 28,981 pass(es), 57 fail(s), and 34 exception(es).
[ View ]

Additionally resolving:

- you got redirected to a page with settings for the field
- BUT the field has no global settings, and even though we are apparently under the Article content type (see page title), Article specific settings are not displayed either

Assigned:Unassigned» sun

Status:Needs review» Needs work

The last submitted patch, drupal.field-instance-settings.29.patch, failed testing.

Much as I would love to say otherwise, I honestly don't think we can do anything about this in D7 now. We're post-RC. :(

I'd agree, if it wouldn't be technically, fundamentally wrong to expose the field settings form on the instance settings form. Most field settings of most fields cannot be (and must not be) changed, so exposing the field settings on that form in the first place makes absolutely no sense at all.

...not to mention that, during debugging of the List field patch just recently, @grendzy reported that you *can* change field settings on the instance settings form without getting a form error... (rather off-topic - actually needs to be a separate issue, because, regardless of where the field settings are getting exposed, we need to ensure that they cannot be changed)

Is the patch you have uploaded reviewable? (it's failing tests but not sure if those are academic failures or horrible problems failures.) I'd need to evaluate the UI impact to make a call on this, I think. If the patch isn't reviewable, I'd be fine with just a general old vs. new workflow description, too.

(if webchick approves this, I'm all for it - thanks sun !)

The patch works and can be applied. The test failures only seem to be caused by test code that expects the field settings on the instance settings form, so fixing those should be relatively easy.

@yched: Manual testing of this patch reveals that the field settings on the instance settings form currently contain the "Number of values" configuration option... which is not contained in the dedicated field settings form... so does that belong into field settings or instance settings? ;)

Cardinality is a field-level setting. it does belong in the 'field settings' form.

Status:Needs work» Needs review
StatusFileSize
new7.93 KB
FAILED: [[SimpleTest]]: [MySQL] 29,061 pass(es), 57 fail(s), and 34 exception(es).
[ View ]

Alright, so:

1) Moved the "Maximum number of values" setting that was previously hard-coded into the instance settings form into the field settings form, where it's supposed to be.

2) Due to 1), now it makes sense to always show the field settings form as first step after creating a new field, even when the field itself has no settings. Thus, reverted that change from the previous patch and added an inline comment instead.

The field settings form behavior for fields that have stored data already is a bit weird, as it outputs a message that field settings can no longer be changed, but actually does not prevent the form from submitting. That's a different problem and should be tackled in a separate issue.

Status:Needs review» Needs work

The last submitted patch, drupal.field-instance-settings.39.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new12.27 KB
FAILED: [[SimpleTest]]: [MySQL] 29,067 pass(es), 24 fail(s), and 20 exception(es).
[ View ]

should take care of a few fails.

Status:Needs review» Needs work

The last submitted patch, field_ui_instance_settings_form-552604-41.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new17.83 KB
PASSED: [[SimpleTest]]: [MySQL] 29,659 pass(es).
[ View ]

Fixed some more tests.

StatusFileSize
new26.1 KB
new20.75 KB
new21.02 KB
new22.24 KB

Screenshots (with patch applied).

StatusFileSize
new19.08 KB
PASSED: [[SimpleTest]]: [MySQL] 29,829 pass(es).
[ View ]

Sorry, screenshots in #44 actually map to attached patch.

Status:Needs review» Reviewed & tested by the community

Gave it a final test and code review, this is good to go.

Summary :
In current HEAD, Field-level settings are present in both
- The 'Field settings' page, that displays a warning that some settings are not changeable any more when the field has data. The 'Number of values' select dropdown is missing there.
- In a separate fieldset at the bottom of the 'Instance settings' page. The 'Number of values' property is displayed, but not the warning about unchangeable settings. Attempts to change an 'unchangeable setting' (in case the field type module fails to disable the corresponding form element) are not detected there and can produce unpredictable effects.

The patch :
- removes the 'Field settings' fieldset at the bottom of 'Instance settings' page. Field settings are on 'Field settings', instance settings are on 'Instance settings'.
- adds back the missing 'Number of values' select in the 'Field settings' page. This also prevents the WTF effect when creating a field with a type that has no settings, and stumbling on a 'Field settings' step with nothing to set.

Status:Reviewed & tested by the community» Needs work

So I spent about an hour with this patch tonight with backup from sun on IRC.

Things I like about this patch:
- There are no longer two places to configure fields. That was dumb.
- One of said places is no longer NOT validated. That was really dumb.

Things I don't like about this patch:
- It doesn't solve the actual issue. Adding a new field still leads to a confusing "Field settings" form all by itself. And now it's worse because we have another confusing "Field instance" form all by itself and we force site builders to understand Field API internals enough to know the difference.
- It moves around the cardinality setting which breaks UI freeze, several books, and various documentation, not to mention 1000s of existing D7 sites who expect to find that setting on the second form and not the first, and have since CCK D5. (or maybe even 4.7)
- Because it moves this setting, which can always be changed, to a form that displays a "Settings here cannot be changed" error message after data is added to the field, it introduces a UX bug where instead of this message usually lying to people, now it will always lie to them.

So on the whole, it's about a wash. :\

This is what I would really, really like us to do instead:

1. Actually remove the field settings form. It's not needed. The field gets created in the database with default schema values without me ever having to save that form.
2. Add validation to the field settings form that's living at the bottom of the field instance form, along with all the other settings. It's definitely a nasty bug that that isn't happening.
3. Instead of displaying that error string as a non-targeted dsm('error') that lies half the time, append it to the field description instead in the case where it's a disabled field.

When I suggested doing this in IRC, sun said that the only way we could do this is to also cram in the widget settings on the same form, which would be another big change and likely one better suited for D8. Though this might be nice, I actually disagree that the widget settings form needs to move. Why? Because when you add a field, the "field stuff" and the "widget stuff" are already two separate concepts in the user's mind:

Settings are broken down between Fields and Widgets

So two separate links going to two separate settings forms kind of makes sense. At least in the "what we can reasonably do for D7" world. (In D8 we should probably re-do this entire thing to be more WYSIWYG or... something else better.)

What they are not exposed to here though is the difference between a "field" and a "field instance". And this level of detail is totally irrelevant for them to be exposed to until much later in their learning process when they discover you can re-use fields among bundles and they can come up with an actual use case for doing that. I think of all the Drupal sites I've ever built, I've used this feature maybe 5 times. So forcing this knowledge on hapless, non-Field API guru site builders who are still trying to wrap their heads around the fact that content types are "entities" and you can add "fields" to "entities" and here's what "fields" are is way, way too much. Hence the "major" priority of this issue, and hence why I don't like sun's fix. It only makes this problem worse.

Because, if you try and write hands-on instructions for a complete newbie on how to add a field to a node type in HEAD, you end up writing something like this:

1. Go to admin/structure/types and click "manage fields" next to "Basic page".
2. Under "Add new field," fill in the following values, and click Save:
- Label: Number field
- Name: number
- Field type: Decimal
- Widget: Text
3. At the first field settings form, choose a precision value of 16 and click Save field settings
4. At the second field settings form, enter the allowed values of "1.030" and "3.203" and click Save settings

It's really, really sad.

If there's only two forms for them to care about here -- one for "everything to do with fields" and one for "everything to do with widgets" -- it radically simplifies things and also matches more closely to what users are used to from CCK.

Ok, after a detailed IRC session, here's the plan sun and yched and I agreed to.

The changes #48 asks for is too much for D7 as the UI change would be pretty drastic and yched has concerns with it. Similarly the patch in #45, though technically correct, also alters the UI too much for D7 -- the "all in one" field settings form gets removed, which many users depend on since it's been there since early versions of CCK.

So we've decided to (mostly) respect UI freeze, and scale back to only fix the actual bugs in this issue, which means:

1. Add the cardinality selector to the solo field settings form. This is a field-level setting; changing it once will change it everywhere, so it makes sense to show it with the rest of the field-level settings.

2. Fix the lack of validation on the "all in one" field settings form. If there are fields that need to be "locked" once the field has data in it, they should be #disabled.

3. Fix the just plain wrong message that says none of the field settings values are changeable once the field has data in it. Cardinality, for example, will always be changeable with field_sql_storage. Replace with a more loosey-goosey worded one like "Because this field has data in it, some of its settings may be locked and unchangeable." This message needs to appear both at the top of the field settings form and on top of the fieldset in field instance form.

Yeah. The original idea was to have a field settings form that had only the unchangeable stuff on it and show only the changeable stuff on the regular edit form, but when that was created there was no way to differentiate between them, so *temporarily* they were shown in both places. And that never got fixed. And the message about not changing settings would have made sense if it was done that way -- you can't change the stuff on the unchangeable form, you can change the stuff on the regular edit form. And since the cardinality is changeable, it went on the regular edit form.

I also thought it would be nice to add information about the widget to the manage fields screen, which we don't have in D6, with a link to change the widget, and remove that part from the global edit form.

So that's more or less the 'why it was done this way' originally.

In other words, the idea was to not say anything about 'field settings' or 'instance settings', just differentiate based on settings that can or cannot later be changed, with all the settings that are changeable and non-problematic on the big edit form and the others tucked away into the field and widget forms.

Hm. Well #50 makes a heck of a lot of sense. So basically just remove the non-changeable fields from the all-in-one form and make the distinction between the two forms "set 'em up once and never change 'em" vs. "configure 'em any time" rather than "field settings" vs. "field instance settings"? Cool!

I think the problem though from talking to yched is that we don't have an easy, standard way of knowing what fields are changeable and what are not. It'd be the responsibility of the field module author to get this right, and they probably won't. Hence the confusingly worded error message. And some are not quite black and white. For example, list items' allowed values. It's fine to add new ones after the field has data, but not ok to change the keys of a list item that has data associated with it. (Or so is my understanding.)

Still though, Karen's approach is even easier if we can figure out a way to do it.

Subscribing. I have read the entire issue, but i'm not clear on if this will / can result in removing the completely pointless first Field settings page when there are no field settings.

Namely, this screen:

Field settings

These settings apply to the Example field everywhere it is used. These settings impact the way that data is stored in the database and cannot be changed once data has been created.
Example has no field settings.

Version:7.x-dev» 8.x-dev

It does not remove it, no. :( I tried to summarize in #48.

And this issue is fully D8 now, although a D7 appropriate patch (separate issue) would be fixing the validation of the second copy of the field settings form on the instance settings page.

sub

For anyone who's interested, I implemented @webchick's preferred idea in #48 (getting rid of the separate "field settings" form entirely) as part of the Backports module in Drupal 7.

See: #1277270: Remove the separate field settings page

The code as written probably isn't that useful for an eventual Drupal 8 patch, but I thought I'd mention it anyway. (It also may be useful if anyone wants to click around and see what the UI would look like if we wind up going in that direction here.)

In my own testing of it, it mostly seemed to work well, but I saw some issues when creating List fields.

There, the current workflow of having two forms actually makes a lot of sense. On the first form, you enter the allowed values (which is basically the most important thing you want to focus on when adding a List field), and then when you get to the second form they are already saved, which means that you are able to select one of them in the "default value" dropdown if you want to.

When everything is on one page, though, the allowed values are hidden down at the bottom of the form, and furthermore, since they don't exist yet, you have no options that you can choose from for the "default value". You have to enter the allowed values at the bottom of the form, save the field, then go back and re-edit if you want to set a default value. This is not a great user experience.

There are other kinds of fields where decisions made on the first form affect what appears on the second, esp anywhere that those field settings affect the choices made later (or when those choices affect the way the default value is constructed). Also we have some settings that can later be changed and some that cannot, and it makes sense to have them in separate places, and for the unchangeable settings to be somewhat hidden by not being on the main edit form. And the other issue is that the main edit form is already really too long for good UX, I don't think we want to make it longer by adding back all the things that got moved off of it.

I think the fundamental problems are 1) some fields have no settings and we should really not make a stop on the settings form when a new field is created if there is nothing to do/choose, 2) some values are showing up in two places but they should show up in only one, and 3) The first form should really be a somewhat hidden form that is limited to values that cannot or should not be altered after there is data and the second form, which is easly discovered, should include values that can safely be changed even if there is already data.

Could these types of dependency problems not be mitigated with the use of AJAX or the #states system, though?

Yes, AJAX could fix that probably. But there's the question of order too (what fields should appear next to each other to make the relationships clear).

It's worth pointing out that any problem along these lines is actually a pre-existing bug/issue, just one that would be more noticeable after this change. (Because even in D7, for example, after a field is created you can go back and change its allowed values later, and the default value options will not update to reflect those until the next page load.)

Assigned:sun» Unassigned
Issue tags:-Needs usability review

I am not quite sure what to review here?

Status:Needs work» Needs review
StatusFileSize
new17.13 KB
FAILED: [[SimpleTest]]: [MySQL] 37,234 pass(es), 31 fail(s), and 32 exception(s).
[ View ]

We absolutely need to get this resolved for D8.

Attached patch is a plain re-roll of #45 against HEAD, which obviously had many conflicts. I didn't test it manually yet.

Status:Needs review» Needs work

The last submitted patch, drupal8.field-instance-settings.62.patch, failed testing.

Priority:Major» Normal

I've added this to #1770720: [META] Gradual changes to Field UI and am demoting.

Issue tags:+D8MI

This would also solve / help solve a usability WTF for translatable fields, since you cannot actually set the translatability of a field when setting it up initially due to certain things not appearing in the form :/ #1831608: Show or hide the "Make field translatable" checkbox on the add field form depending on translatability was marked postponed on this one.

Issue tags:-D8MI
StatusFileSize
new48.24 KB
new61.07 KB
new26.49 KB
new51.9 KB
new19.24 KB
new1.62 KB
new21.58 KB
FAILED: [[SimpleTest]]: [MySQL] 47,792 pass(es), 12 fail(s), and 4 exception(s).
[ View ]

I rerolled the patch and fixed the test issues.

Here is how the UI looks like now:

1. Choose field name and type (nothing has changed)

Screen Shot 2012-11-06 at 4.35.53 PM.png

2. Make global field settings (All global settings are available now)

Screen Shot 2012-11-06 at 4.36.12 PM.png

3. Make field instance settings

Screen Shot 2012-11-06 at 4.36.32 PM.pngScreen Shot 2012-11-06 at 4.37.08 PM.png

4. Editing global settings after it contains data displays a message that doesn't make sense anymore since global values can be changed.

Screen Shot 2012-11-06 at 4.37.39 PM.png

Local tasks wording

I find the wording of the tabs misleading. A first suggestion which does make it clear to me, but maybe not understandable for other people:
"Edit" -> "Instance settings"
"Field settings" -> "Global settings"

Status:Needs work» Needs review
Issue tags:+D8MI

Status:Needs review» Needs work

The last submitted patch, 552604-field-ui-65.patch, failed testing.

@Lukas Awesome that you are working on this! It looks like a good solution, I would however like to split off this multilingual fix from what this issue is about. Because your solution doesn't really solve what this issue is about, could you perhaps open a new issue for your solution specifically?

There is a difference between things like the 'number of values' setting (which can be changed at any time, and things that affect the database storage (like the type and length of a field). The number of values can be changed at any time and it is wrong to indicate that it cannot. Other things cannot be changed later. In some cases it is physically possible to change them but a bad idea (you might mess up your data).

The most logical way to construct this is to make the first form things that should be set once and never changed and the second one things that can be changed any time. The problem is that the first form is used to create the field, which means you need to have all the field settings, including those that can be changed later. So the first form has to ask what it needs to know to create the field, not just for things that cannot be later changed.

The second form can be more flexible. There we can display only things that can be changed (which includes the number of values). The problem here is that we don't have a good way to know which field values can be changed because we never implemented an API that would tell us that. I'm not sure we can fix this form without fixing that underlying problem.

StatusFileSize
new5.64 KB

Hm. The interdiff in #66 is actually not complete. I've manually constructed an interdiff against #62.

The big difference seems to be that #66 introduces an entirely new form submit handler, whereas earlier patches did not have to do that. Was there any particular reason for that?

Status:Needs work» Needs review
StatusFileSize
new6.64 KB
new18.03 KB
FAILED: [[SimpleTest]]: [MySQL] 47,822 pass(es), 10 fail(s), and 4 exception(s).
[ View ]

That additional submit handler didn't appear to be necessary.

Attached is a straight re-roll of #62 against HEAD.

Also attaching an interdiff against #66.

@Lukas von Blarer:
I somewhat like your suggestion of changing the tab titles to "Instance settings" and "Global settings". However, I think that should happen in a separate issue.

@KarenS:
Yes, we're definitely missing an API there. However, that doesn't really change the fact that the global field settings really shouldn't be exposed on the instance edit form, since touching those performs changes to something completely else than the context of the form suggests (editing instance settings).

Status:Needs review» Needs work

The last submitted patch, drupal8.field-instance-settings.73.patch, failed testing.

@sun: We've talked quite a bit about this in IRC and our sprint, the problem is that there are multiple possible directions and according to Bohjan, adding more things to the initial form is the oppositive of what they want. ( Which is also the opposite of #1831608: Show or hide the "Make field translatable" checkbox on the add field form depending on translatability, so consider me confused).

I totally agree with you that this is technically the correct solution, certainly compared to the mix that we have right now, but it seems what the UX team wants is to merge those two settings pages into one.

+++ a/core/modules/translation_entity/translation_entity.moduleundefined
@@ -558,7 +558,7 @@
+function translation_entity_form_field_ui_field_edit_form_alter(array &$form, array &$form_state, $form_id) {
-function translation_entity_form_field_ui_field_settings_form_alter(array &$form, array &$form_state, $form_id) {
   $field = $form['#field'];

This was done to implement the mentioned issue as a proof of concept. Which also required the $form['#field'] change from above.

That said, this should probably be done in that issue, as a follow-up, as it will also require documentation updates and so on, which would make this patch unecessarly bigger. *If* this issue is actually implement like this. If not, then that other issue might become irrelevant anyway.

@KarenS: Apart from what sun said, the field types are also very inconsistent in regards to what they allow/prevent. Changing the cardinality to a lower value than you have entities with number of items is allowed, e.g. removing allowed values that are used is not (which in most real world examples that I've encountered - during development, is a huge pain, as it's not even possible through the API, e.g. with features).

In my opinion there are three types of settings for a field:

Global settings which can't be changed after field creation (data structure)

  • Field name
  • Field type
  • Field specific setting E.g.: Text field length

Global stetting that can be changed after field creation but and can be potentially destructive

Number of values
Translation
Field specific setting E.g.: File field upload destination

Field instance settings

Label
Help text
Required
Field widget (I don't see the requirement of that being on a separate page)
Field specific setting E.g.: File field upload destination

I think we should group the settings similar to this and present them on one page visually clrearly separated with fieldsets or similar.

I fully agree that the current Field UI organisation is not optimal.

However, do we really want to reshuffle this now (which IMO can easily take another couple 10s of comments, between those that want to split and those that want to merge), and start over again when core has an official Popups API that we're officially allowed to use ?
Or start to come up with a solution that works well with popups ?

re #75:

adding more things to the initial form is the oppositive of what they want.

Mmm, I'm confused — this patch here does not "add more things" to the initial form. It only removes the form elements from the second form, which are on the initial form already.

In short, the difference is this:

Before:
1) Field settings form
2) Widget settings form
3) Instance AND Field settings form

After:
1) Field settings form
2) Widget settings form
3) Instance settings form

Whereas 3) is especially confusing when adding a new instance by re-using an existing field — the instance settings form suggests that you'd be able to edit the global field settings, too (I'm allowed and supposed to change the controls that I'm presented with, right?) and gives no indication or whatsoever that touching those controls will have a site-wide impact on the field wherever it is used.

The global field settings controls should not be exposed on the instance settings form to begin with.

That is the sole and only purpose of this issue since the beginning. This was originally discovered very late in the D7 cycle, and it was deemed to be too late to change it for D7 (#49). Very soon, we are in the identical situation for D8.

For the sake of sanity and doing pragmatic progress, let's please, please, please not make that same mistake again. :)

We can still completely revise this flow and do whatever we think would be nice, but let's please do that in a dedicated follow-up issue.

I think if we can do this simple fix quickly, then it would be great to get it in ASAP and then work on even more rework later. Leaving it in the D7 stage would be pretty sad. If we are hooked up here discussing how to do it for a non-trivial time, than it is not worth it to do in multiple layers (IF somehow the better improvement happens sometime).

Right, sorry, got confused by some comments above.
#78 is a plan I'm perfectly fine with.

Status:Needs work» Needs review
StatusFileSize
new20.52 KB
PASSED: [[SimpleTest]]: [MySQL] 47,870 pass(es).
[ View ]

Fixed all test failures.

@sun

- The form that I meant is the one you see when adding new fields (see issue title :)). Right now, you often have very few or no settings at all there, this patch means that all field settings show up there. Bohjan wants to get rid of that step completely.

- Make sure you test your changes with translation_entity.module enabled. Without changing it, it means that the setting which it adds shows up outside of the fieldset on the instance settings form.

StatusFileSize
new23.66 KB
PASSED: [[SimpleTest]]: [MySQL] 48,778 pass(es).
[ View ]

1) Yeah, understood. :) There's room for rethinking this user interface entirely. But let's first make sure that we don't continue the current weirdness. :)

2) Thanks for the pointer — manually tested that and adjusted it.

1. In case I wasn't clear. I agree 100% with you. And I tried to explain it to Bohjan but wasn't very successful :). He's the person you need to convince that it makes sense to do this as a first step and *then* try to improve it further.

@Bojhan: I hope we can agree on a pragmatic intermediary commit to facilitate progress. :) I'm waiting for this clean-up to happen since August 2010 (#25) and I wouldn't like to see us run into the same pitfall as with D7. ;) It almost looks like it would be best to retain this issue for further mockups and discussions afterwards, since there's plethora of background, technical, and usability information in the above comments already.

@sun I dont get the solution, and or how it moves us closer to solving this issue. All it seems to do is change some labels, and trow some fields around. It probably doesn't make it worse, so from that pov I am fine with committing it.

Status:Needs review» Reviewed & tested by the community

I like the change, it cleanly separates the configuration on the page it needs to be - granted, probably more on technical reasons (fields vs instances vs widget), but then again, it makes sense. We can always clean up more in follow ups, so let's get this in!

Updating the issue summary might make this easier for a committer to evaluate. (It was RTBC since Nov 9 and has not gotten in) Anyone up for re-doing the issue summary?

Status:Reviewed & tested by the community» Needs work

Shucks. This one no longer applies due to the details patch. REALLY happy to see it RTBC though! Can we get a quick re-roll?

Status:Needs work» Needs review
StatusFileSize
new23.94 KB
PASSED: [[SimpleTest]]: [MySQL] 48,793 pass(es).
[ View ]

Here's the re-roll, should be fine I think.

Status:Needs review» Needs work
Issue tags:-Usability, -Needs issue summary update, -Fields in Core, -string freeze, -API change, -D8MI

The last submitted patch, 552604-92.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Usability, +Needs issue summary update, +Fields in Core, +string freeze, +API change, +D8MI

#92: 552604-92.patch queued for re-testing.

Test passes on my local machine and the failing one was in views which really doesn't have anything todo with this patch.

Status:Needs review» Reviewed & tested by the community

And moving back to RTBC

manual testing, screenshots, trying to understand the scope of the (proposed) fix

00a. screen shots with 8.x edit field (shows all global settings in a collapsed/expanded fieldset at bottom)
confusing-s00a-nopatch-editfield-New Thing | confusing-552604-92 2012-11-27 10-35-42.png

00b. and 8.x field settings (only some of the global settings, no number of fields for example, has warning cannot change these)
confusing-s00b-fieldsettings-New Thing | confusing-552604-92 2012-11-27 10-37-03.png

with patch

01. creating a field on manage fields
confusing-s01-add-field-manage-fields-2012-11-27_1004.png
02. after save global field settings
confusing-s02-after-save-global-field-settings-2012-11-27_1006.png
03. non global settings edit field page
confusing-s03-non-global-settings-edit-New Thing settings for Article | confusing-552604-92 2012-11-27 10-09-10.png
04. make a node with content in the field
confusing-s04-onenode-hascontent-2012-11-27_1010.png
05. mangage fields
confusing-s05-managefields-2012-11-27_1013.png
06. edit field (shows just the settings that are not global, no global collapse/expand)
confusing-s06-editfieldwithcontentexisting-New Thing settings for Article | confusing-552604-92 2012-11-27 10-14-56.png
07. field settings (show all global settings, with wanrning cannot change. but can change them)
confusing-s07-fieldsettingswithcontentexisting-2012-11-27_1016.png

seems like plan is to fix this like that here.

then make follow-ups for remaining things to change, like:
fixing the red [edit: read->red] warning,
relabeling the tab to global settings..
were there any other follow-ups that need to be opened?

Assigned:Unassigned» swentel
Status:Reviewed & tested by the community» Needs work

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new30.16 KB
PASSED: [[SimpleTest]]: [MySQL] 48,957 pass(es).
[ View ]

Re-rolled.

Status:Reviewed & tested by the community» Fixed

Looks like webchick's already given this a good, look, and it's had to be re-rolled a few times, so committed this while it still applies. Thanks!

this commit (git checkout 08df7b0db2369bb) introduced #1872786: Can't change widget types

That is actually a bit more nefarious than mere notices; it prevents you from changing the widget type at all. Escalated to a critical.

Status:Fixed» Active

According to #85 and others, this was only an intermediary commit and the issue should remain open. (Or at least, a new issue should be filed as a followup.)

Assigned:swentel» Unassigned

@YesCT - I guess we can close this one now then with the new ones ?

Well, the main new issue that's needed (or the reason to keep this open) is for the fundamental usability issue that you still get redirected to a confusing Field Settings form (and confusing multi-step process involving fields vs. instances) in order to simply create a field.

So for example, #48 or other ideas mentioned above...

Issue tags:-API change

Not an API change anymore. Cleaning out the API changes proposed against Drupal 8.