Problem/Motivation

On the first iteration of the prototype from https://www.drupal.org/project/drupal/issues/3343940, we noticed that the way that we chose to represent the available field types was overwhelming. One of the ideas we had was to try to come up with a better grouping of the field types, to hide some of the complexity.

Proposed resolution

Group fields together to make it less overwhelming based on the findings from https://www.drupal.org/project/drupal/issues/3346539#comment-14999770

Remaining tasks

User interface changes

Fields

"Subfields"

API changes

Release notes snippet

Issue fork drupal-3356894

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

hooroomoo created an issue. See original summary.

hooroomoo’s picture

Category: Bug report » Feature request
Status: Fixed » Active
hooroomoo’s picture

Assigned: Unassigned » hooroomoo

This is prototyped up already so assigning it to myself so no one wastes time doing work that already exists.

hooroomoo’s picture

Title: Improve adding a new field experience » Make field selection less overwhelming by introducing groups

lauriii credited ckrina.

lauriii’s picture

Issue summary: View changes
StatusFileSize
new221.95 KB

The groups are based on #3346539-6: [Plan] Improve field creation experience card sort. We have run a round of usability tests with designs that are using these groups, and the results were really encouraging. What we tested was a slightly different design based on modals, but the UX was similar to this. The process will be converted to use modals in #2880003: Use modals on the Manage Fields page after #3358049: Save FieldStorageConfig at the same time as FieldConfig has been resolved.

Discussed with @ckrina to come up with a deliverable that is a step forwards, but doesn't rely on modals. The goal is for this to also help us in the overarching goal to revamp the field creation experience in #3346539: [Plan] Improve field creation experience which is based on modals. Attaching a screenshot of the mockup to the issue summary.

hooroomoo’s picture

Last few commits include:
- changing the field types to radio buttons
- adding an ajax callback to show the different field options when a field type is clicked

Still needs to be done (the form is on FieldStorageAddForm.php):
1. submission handling (I think \Drupal\field_ui\Form\FieldAddForm::submitForm is mostly correct that could probably just be copied but needs to be tweaked to pass in the correct values)

2. adding group descriptions as @annotations instead of hardcoding them. I think each group might need their own file and class for the annotation...? (i would confirm with someone on this) Refer to any of the files with '@FieldType' for an example of annotation
the group description texts can be found here: https://github.com/bbenjamin/drupal/blob/10.1.x/core/modules/field_ui/src/Controller/FieldConfigListController.php#L246

3. display the field description (these are already in annotations) I added this to the showFieldsCallback() but for some reason it isn't showing up)

4. sort the field options so the important ones are first (like in the screenshot) i think for now we're sorting by description length but that may change

hooroomoo’s picture

StatusFileSize
new46.8 KB

hooroomoo’s picture

StatusFileSize
new127.15 KB

Added styling for the top half, styling for the bottom field half needs to be updated

hooroomoo’s picture

Added styling to the rest of the page. navigating with keyboard still needs work

ckrina’s picture

@hooroomoo I'd leave around 2px of white space around the grey wrapper of each icon on the checkbox, and adjust the min-height to the icon height (plus the white space around it). Thanks, it's really cool to see this implemented! Also, I've suggested and icon for the default one on Figma, let's see if it makes sense for you.

srishtiiee made their first commit to this issue’s fork.

hooroomoo’s picture

I updated the form for field types that are not in groups (comment, boolean, email, etc) to have their field type id under 'new-storage-type' which fixed fixed several tests without having to change any of the tests.

In FieldUITestTrait.php I also updated fieldUIAddNewField() function to handle adding fields that are part of a group. This should hopefully also fix several tests without having to change any of them. The fieldUIAddNewFieldJS() function in the FieldUIJSTestTrait.php still needs to handle adding fields that are part of a group.

I think a lot of these tests can just be fixed by using these trait functions and we won't need to have to change the field type strings in every test.

tim.plunkett made their first commit to this issue’s fork.

narendraR made their first commit to this issue’s fork.

narendrar’s picture

Status: Active » Needs work

The reason for test failure above is comment_form_field_ui_field_storage_add_form_alter where it is unset when required condition is met. This is now failing due to change in html structure of FieldStorageAddForm.

We also need to look into other hook_form_FORM_ID_alter() for 'field_ui_field_storage_add_form' to adjust changes as per new flow.

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

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ckrina’s picture

StatusFileSize
new73.62 KB

I just defined several states for this interactive element. You can find them on Figma here to pick colors/sizes/spaces.

hooroomoo’s picture

StatusFileSize
new128.23 KB

The radio buttons position is off right now, I wasn't sure how to fix it without breaking the responsiveness of the icon background filling up the container.

The layout shift on hover, checked, focus states is a little better now after subtracting the size of the border from
each of the margins in the state, but there is still the slightest shift.

hooroomoo’s picture

Status: Needs work » Needs review
wim leers’s picture

Pinch me.

Is #20 really Drupal?! 🤩

bnjmnm made their first commit to this issue’s fork.

smustgrave’s picture

Status: Needs review » Needs work

If this was discussed already apologizes

Testing this out and functionally seems to be working.

Could a different default icon be used? The comments and telephone icons were the same and it looks like a reverse ckeditor image upload icon. I know those fields but almost seems like it would be uploading a file, which obviously neither field does.

This one may have been me. But when I selected the telephone option, it almost felt like I could select multiple fields to add at once. That may be a me issue but wanted to mention.

I also forgot to set my label since that's now first field when previously you had to select a field first before having to put a label. Think this is a good change but definitely new.

Think the biggest one is the default icon for me.

Rajeshreeputra made their first commit to this issue’s fork.

lauriii’s picture


The implementation of the designs still need some adjustments:

  1. The icon width should remain consistent even when the height increases
  2. There shouldn't be empty space between the label and the description
  3. The label text should be 16px without bold
  4. The description text size should be 14px
  5. On some screen sizes, when toolbar is in vertical mode the description text can overflow
hooroomoo’s picture

StatusFileSize
new312.82 KB

Re: #24
- I added a puzzle piece icon that is a lighter shade of gray for the default/fallback icon. Project browser currently has the same thing for projects that don't have an icon.

- I don't notice anything different when I click the telephone card myself

Only local images are allowed.

hooroomoo’s picture

Status: Needs work » Needs review

Feedback in 24 and 26 should be addressed now

smustgrave’s picture

Status: Needs review » Needs work
StatusFileSize
new96.95 KB

error

Left a small comment on the MR.

Also the green outline seems to be running flush with the text. Looks kinda weird.

Wonder if a change record should be started. How do contrib fields easily add an image for example.

hooroomoo’s picture

Status: Needs work » Needs review

Created a CR for FieldStorageAddForm structure change and the new field_type_category_info() hook that explains how contrib modules can add their own category properties using the hook.

smustgrave’s picture

Status: Needs review » Needs work
StatusFileSize
new42.34 KB

Hate to be that guy

overlap

But the green focus now covers partially a letter.

narendrar’s picture

Status: Needs work » Needs review
hooroomoo’s picture

@narendraR Oops looks like we pushed the same fix around the same time 😅

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative, +Pittsburgh2023

Took a look at this during DrupalCon. Small css issues I reported have been resolved.
Tested by adding a few fields, some with with sub options, like entity reference and everything functions.

Very neat feature

lauriii’s picture

Status: Reviewed & tested by the community » Needs work

Posted some feedback on the MR. 😇

hooroomoo’s picture

Status: Needs work » Needs review
hooroomoo’s picture

Updated issue with current UI screenshots

hooroomoo’s picture

Issue summary: View changes
smustgrave’s picture

Status: Needs review » Needs work

Seems 1 small failure.

Will keep an eye out though as imagine will be a quick fix.

narendrar’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Change looks good to me.

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new27.4 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

rajeshreeputra’s picture

Status: Needs work » Reviewed & tested by the community

Rebased, changes looks good to me, hence moving to RTBC.

tim.plunkett’s picture

StatusFileSize
new142.69 KB

Uploading static patch for 10.1.x, please ignore. Use the MR for further work!

bnjmnm’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new135.99 KB
new44.92 KB

Looks like in some cases, depending on where a newline happens, the focus outline overlaps with the text a bit

The date range field could use a custom icon instead of the puzzle piece.

hooroomoo’s picture

Posting date range icon requested in #45 here in case someone else jumps on this. (This svg has been optimized on SVGOMG already)

<svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" fill="none"><path d="M15 2h4a1 1 0 0 1 1 1v16a1 1 0 0 1-1 1H1a1 1 0 0 1-1-1V3a1 1 0 0 1 1-1h4V0h2v2h6V0h2v2ZM2 8v10h16V8H2Zm2 2h2v2H4v-2Zm5 0h2v2H9v-2Zm5 0h2v2h-2v-2Z" fill="#55565B"/></svg>

Utkarsh_33 made their first commit to this issue’s fork.

hooroomoo’s picture

Status: Needs work » Needs review
bnjmnm’s picture

Status: Needs review » Needs work

Pushing back on two remaining things, but this is the final stretch.

hooroomoo’s picture

Status: Needs work » Needs review
lauriii’s picture

Status: Needs review » Reviewed & tested by the community

I've reviewed the code several times in the life cycle of this issue so I'm pretty familiar with it at this point. I went through the recent commits and went through the code changes and all looks good.

bnjmnm’s picture

StatusFileSize
new130.04 KB

This did not apply cleanly to 11.x so here's a patch to be sure it gets through the testbot and I plan to commit once green.

yoroy’s picture

Status: Reviewed & tested by the community » Needs work

Thanks Lauriii for setting up this simplytest.me so that I could play around with this a bit. (https://master-nnmtgk55ieypf82fpkpcsj7mvgb6d0ez.tugboatqa.com/ for as long as it lasts)

I think I found a bug where going back and forth between selecting fields ended up creating a different field than what was selected on save.

Steps to reproduce:
- go to /admin/structure/types/manage/article/fields/add-field
- click 'Boolean'
- click 'Reference'
- select taxonomy term as the reference type
- click Save and continue
- This sends me to the field settings for a boolean field instead of for the taxonomy term reference I selected last

A usability issue that bit at least me *every time* I tried out this procedure is that I forgot to enter a label for the field. The fields types are visually so attractive that the poor little label field above got ignored.

lauriii’s picture

Issue tags: +Needs tests

I think I found a bug where going back and forth between selecting fields ended up creating a different field than what was selected on save.

Good catch 👍 Was able to reproduce with the steps you provided. Definitely worth addressing.

A usability issue that bit at least me *every time* I tried out this procedure is that I forgot to enter a label for the field. The fields types are visually so attractive that the poor little label field above got ignored.

This is similar to what was reported in #24 so might be worth looking into a bit. I'll talk with @ckrina if she has ideas how we could try to improve this. However, we could potentially move the label away from this step after #3347291: Combine field storage and field instance forms.

amateescu’s picture

Overall this looks really good! Just posted an initial review in the MR.

In addition to those points, can we figure out any other solution for field types to declare the icon that should be used, either for a category or for a specific field type? Maybe as a new annotation property, or a method on the field type class? Adding all those libraries just for an icon seems a bit excessive IMO :)

lauriii’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests

In addition to those points, can we figure out any other solution for field types to declare the icon that should be used, either for a category or for a specific field type? Maybe as a new annotation property, or a method on the field type class? Adding all those libraries just for an icon seems a bit excessive IMO :)

This would be really nice! We actually had something like this first but run into challenges with defining the correct path, and figuring out how themes would be able to override those. We realized we don't have this as an existing pattern anywhere in core so we decided to go with the current approach which is used by other places doing anything close to this. We definitely should look if we could introduce it for field groups and couple other places like Toolbar.

amateescu’s picture

Thanks for all the updates, this looks ready to me.

Are we planning to address the feedback about the label field being easy to miss in this issue or a followup?

lauriii’s picture

We're planning to do some more user tests on this once #3358049: Save FieldStorageConfig at the same time as FieldConfig is closer to landing and we could iterate on the designs then. We might be even able to move the label to the second step of the process.

We've tested a similar design to this with users and noticed this issue there as well. However, all of the users were able to recover from this quickly. Given that we will have more options for addressing this once we've made a bit more progress, and that all users we tested this with preferred the current UI over the existing UI, I'd lean towards addressing that in a follow-up. I discussed this with @ckrina some time earlier and she was on board with this plan.

yoroy’s picture

Status: Needs review » Reviewed & tested by the community

> We might be even able to move the label to the second step of the process.

That seems to be the way to go. It's fine to commit this as is as long as we keep iterating but sounds like that's the plan anyway. E.g. the visual relationship between the main item and its subs could still be strengthened by dimming the other main items. But I'm especially wondering why the subs are shown at the bottom instead of spliced in right below the selected item.

Also, on the one hand: good to see a follow up for refining field descriptions, on the other hand, why introduce them in this really not yet great version? It's a bit uncomfortable to see this essentially big improvement introduce its own significant ux regressions.

Ok, done complaining :) This should be committed because it's a very big step in the right direction. Good work!

lauriii’s picture

That seems to be the way to go. It's fine to commit this as is as long as we keep iterating but sounds like that's the plan anyway. E.g. the visual relationship between the main item and its subs could still be strengthened by dimming the other main items. But I'm especially wondering why the subs are shown at the bottom instead of spliced in right below the selected item.

This was always intended as the first iterative step to improve the field creation workflow. I don't think it ever made it to the issue summary but was mentioned in #7. A lot will still change so we didn't want to spend too much time perfecting this. Even now, I think we spent more time than on this iterative step than we anticipated. 😅

Also, on the one hand: good to see a follow up for refining field descriptions, on the other hand, why introduce them in this really not yet great version? It's a bit uncomfortable to see this essentially big improvement introduce its own significant ux regressions.

I agree that this introducing it's own regressions to the UX but the reason we are trying to make this iterative step is that we believe it's a step forward (which we have validated with user tests utilizing this change). It also sounds like you agree with this. 😊

We definitely want to improve the UX from here on but just not on this specific issue. This issue is already introducing changes to 112 files across core, and adds over 1000 lines of code, which makes it really expensive to keep iterating on anything here.

Additional benefit of introducing this change before having everything perfected in core because once we have this change committed, contrib can start working towards making the required changes too.

  • bnjmnm committed 9d522de0 on 11.x authored by hooroomoo
    Issue #3356894 by hooroomoo, lauriii, srishtiiee, Rajeshreeputra,...
bnjmnm’s picture

Status: Reviewed & tested by the community » Fixed

You know what? With 111 files changed there might be a few things that aren't perfect, but I and several other community members have gone over this many times. I do believe this has been reviewed to the point where anything truly problem-causing has been addressed.

The sooner this gets in, the sooner it can be iterated and improved upon with a healthy time window before 10.2, and within those improvements I'm sure any existing nits can be knocked out. For now, this is an excellent step in a the right direction for Drupal's UX with good test coverage (and improvements to tests in general). Thanks all!

larowlan’s picture

Great work everyone 🎉

I was surprised to see the info hook pattern return when we spent a lot of the D8 cycle removing info hooks

Did we consider YML instead?

I see a review from Tim, so I'm surprised he didn't flag this too

dinarcon’s picture

Awesome that this has landed. Thanks everyone!

@bnjmnm mentions 10.2 in the message where the issue status is changed to fixed. Does this needs to be backported to Drupal 10? Or is it meant to be a feature only available in Drupal 11?

smustgrave’s picture

11.x is essentially the main branch right now. So Drupal releases will be tagged off that. So 10.2 will be tagged at some point from the 11.x branch.

tim.plunkett’s picture

Assigned: hooroomoo » Unassigned

@dinarcon See https://www.drupal.org/node/3359338, the branching approach has changed. Everything currently in 11.x will be in 10.2.x

@larowlan I did flag that in discussions with @lauriii and @hooroomoo, and maybe we should have revisited that, but at the time it seemed like a reasonable way to make progress. The hook itself underwent a good deal of iteration in the past few months, and it was "just faster" to develop this way.

I think if we want to switch it away from a hook and can do so before 10.2, it's worth a follow-up!

larowlan’s picture

Thanks Tim

I agree pragmatism and velocity over idealism is the best way to get these big ticket wins in.

I'll make a follow up to explore, plenty of time till 10.2 if we change it slightly

larowlan’s picture

Small follow-up for the theme wrapper suggestion, I think we can specify it a bit more cleanly #3372089: Remove field_ui_theme_suggestions_form_element_alter in favor of specifying the theme wrapper on the element

larowlan’s picture

A minor follow up to make the category grouping more defensive #3372090: Logic to decide if a field category is a category ID or not is brittle

larowlan’s picture

A follow-up to add a BC layer for modules that define their own field UI categories, rather than a hard break #3372091: Assertion error for any module defining its own field type category

larowlan’s picture

A followup to improve the experience for consuming modules #3372092: Allow field_type_categories.yml entries to define asset libraries

larowlan’s picture

ckrina’s picture

Added a follow-up for design adjustments: #3372316: Adjust design implementation for the field types.

mandclu’s picture

Is there any documentation for contrib maintainers on how to ensure modules that provide new fields will be ready for this? If so I'd be happy to cascade.

larowlan’s picture

@mandclu at the moment the change record - but - I'd hold off because we might be removing the hook in favour of a yml file AND we might be simplifying how you attach a library

Status: Fixed » Closed (fixed)

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

claudiu.cristea’s picture

It's so confusing. Switching to Drupal 10.2 force you to reconstruct the history of this change. Can we have an unified change record merging the 2 as both changes are sequential an impacts a single update from Drupal < 10.2 to 10.2

heddn’s picture

And the CR reference See https://www.drupal.org/node/3364271 is for a node that 404s. Please, what do we need to do here? Open a follow-up to fix the CR and unify things?

heddn’s picture

Note, the change records also doesn't discuss any of the implications after upgrading to 10.2 and what that might mean to field widgets. See #3057556: Implement DERItem::getReferenceableBundles, where the States API totally breaks on form elements for fields now being nested in completely different locations.

jonathan1055’s picture

alexpott’s picture

I think we're missing a change record for the support for arrays in the field type plugin description. The CR should cover why and when you should have an array here.