Currently it appears only the title can be used as the entity label. The label method was introduced here: #2558143: Opt-out from optional base fields

Many custom entities don't need a title field but may have other suitable fields which can be used as the label. If there's already a way to do this in the UI then I've missed it completely. The Title property/field seems to be left over from the pre-entity node days and is entirely optional with entities. Entity Labels aren't (or shouldn't be) optional and are more flexible.

Somewhat related, but it could also be good for the ECK content list ( /admin/structure/eck/entity/entity_type ) to have a Label column instead of Title by default.

Could this be done on a per-bundle basis? For example, we could list all the fields of type "string" as potential candidates, as well as the title if it's enabled.

CommentFileSizeAuthor
#41 eck-custom-label-field-2785297-41-beta2-reroll.patch3.79 KBMunavijayalakshmi
#41 eck-custom-label-field-2785297-41.patch4.79 KBMunavijayalakshmi
#40 eck-custom-label-field-2785297-40-beta2-reroll.patch3.79 KBDieterHolvoet
#40 eck-custom-label-field-2785297-40.patch4.79 KBDieterHolvoet
#38 eck-custom-label-field-2785297-38.patch4.79 KBRichardGaunt
#37 eck-custom-label-field-2785297-37.patch3.79 KBRichardGaunt
#34 eck-custom_label_field-2785297-34.patch7.97 KBbmcclure
#32 eck-custom_label_field-2785297-32.patch7.89 KBbmcclure
#27 allow_fields_other_than-2785297-27.patch4.4 KBimclean
#25 allow_fields_other_than-2785297-25.patch4.31 KBlegolasbo
#22 eck_label_field-2785297-22.patch4.37 KBimclean
#18 eck_label_field-2785297-17.patch4.35 KBimclean
#14 eck_label_field-2785297-14.patch4.2 KBimclean
#11 eck_label_field-2785297-9.patch3.78 KBimclean
#7 eck_label_field-2785297-7.patch3.75 KBimclean
#5 eck_label_field-2785297-5.patch1.67 KBimclean
#3 ECK 8 Label.jpg26.68 KBimclean
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

imclean created an issue. See original summary.

imclean’s picture

Issue summary: View changes
imclean’s picture

Issue summary: View changes
FileSize
26.68 KB
legolasbo’s picture

This seems like a really sensible solution. Unfortunately I lack the time to work on this myself at the moment (baby due any moment), but I welcome any help in solving this issue.

imclean’s picture

I don't have a lot of time either but hope I can still contribute something. This patch doesn't do much yet, it just adds the config option to the bundle edit form. It doesn't save properly either, I'm still learning about Drupal 8 and ECK.

Congrats on the baby legolasbo.

imclean’s picture

Status: Active » Needs work
imclean’s picture

Status: Needs work » Needs review
FileSize
3.75 KB

This adds the option to select the label field per bundle type, with fallback to current behaviour. It supports title and string fields as label fields.

imclean’s picture

It also changes the the header text in the ECK content list from "Title" to "Label". It already uses the label callback.

Status: Needs review » Needs work

The last submitted patch, 7: eck_label_field-2785297-7.patch, failed testing.

The last submitted patch, 7: eck_label_field-2785297-7.patch, failed testing.

imclean’s picture

Status: Needs review » Needs work

The last submitted patch, 11: eck_label_field-2785297-9.patch, failed testing.

The last submitted patch, 11: eck_label_field-2785297-9.patch, failed testing.

imclean’s picture

legolasbo’s picture

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

Thanks for your contribution imclean,

The patch looks promising, but I would like to see all added functionality under test before I commit it.

imclean’s picture

I'm still getting up to speed with writing tests. What would need to be tested against here? I don't think there's currently a specific test for the label field and/or method. I've added the new config option.

legolasbo’s picture

I think you have answered your own question. There currently is no specific test for the label field and/or method, but there should be, so that's what needs to be tested.

imclean’s picture

This adds a check, first() isn't always needed. Still needs tests.

legolasbo’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 18: eck_label_field-2785297-17.patch, failed testing.

The last submitted patch, 18: eck_label_field-2785297-17.patch, failed testing.

imclean’s picture

imclean’s picture

Status: Needs work » Needs review
bmcclure’s picture

This is working for me, and IMO this is a pretty major issue for making ECK more versatile and usable in a wider variety of situations. About 1/2 of my entity types don't have a Title field, but do have another field I want to use for labeling.

legolasbo’s picture

Rerolled the patch as it no longer applied to the latest head.

legolasbo’s picture

Status: Needs review » Needs work

This issue still needs test coverage for the new functionality.

While testing the patch manually I found that you can only use textfields as the label. In my opinion fields like date, list (text) or email could be valid labels as well.

Also looking at the code. I think you could replace

$bundle = $this->bundle();
$bundle->getEntityType()->getLabelField();
$config_dependency = $this->getEntityType()->getBundleConfigDependency($bundle);
$config = \Drupal::config($config_dependency['name']);
$label_field = $config->get('label_field');

with

$label_field = $this->bundle()->label_field;
imclean’s picture

I've added a few extra field types, not all have been tested yet. For some reason I couldn't create content if a date field was present but that's a separate issue.

Also looking at the code. I think you could replace...

Using

$label_field = $this->bundle()->label_field;

returned NULL.

These 4 lines still work:

$bundle = $this->bundle();
$config_dependency = $this->getEntityType()->getBundleConfigDependency($bundle);
$config = \Drupal::config($config_dependency['name']);
$label_field = $config->get('label_field');
imclean’s picture

Status: Needs review » Needs work

Still needs tests. Different field types may need to be handled differently, although I'm not sure about this.

imclean’s picture

+++ b/src/Form/EntityBundle/EckEntityBundleForm.php
@@ -90,6 +90,28 @@ class EckEntityBundleForm extends EntityForm {
+    $valid_labels = ['email','datetime','text','list_string','text','string'];

'text' is listed twice.

danharper’s picture

Patch is failing on latest dev, is this something that is going to be turned into a stable feature?

imclean’s picture

We're not using ECK in any new projects so I'm unlikely to work on this.

bmcclure’s picture

Here is a reroll and refactor of some of the code in the previous patch. I went with an event to allow customization of the available field types since hardcoding it seemed too rigid.

I also removed the conditional checking for the "field_" prefix, because custom fields might not have that if they were created in code.

This is barely tested yet, but I needed to get a working version of the patch for the latest dev.

bmcclure’s picture

Adding a related issue, which might also be the one making this patch no longer apply. They aren't solving the same thing, but they are solving similar things related to customizing the label.

bmcclure’s picture

Attempting a reroll against the latest dev. Let's see how well this works.

JLucySong’s picture

Thanks bmcclure
Somehow the label field setting is not always saved successfully
It's still showing the default title instead of the custom field

danharper’s picture

Slightly off topic

If anybody is looking for a quick way to solve this I used https://www.drupal.org/project/auto_entitylabel

1. Install above module
2. Enable title for entity type
3. Configure title to be auto generated with tokens and hidden from display.|

Thanks Dan

RichardGaunt’s picture

I've re-rolled against latest release (beta 2) in case anyone needs.

RichardGaunt’s picture

Also, re-rolled against latest dev version.

RichardGaunt’s picture

Can also report that the custom label configuration is not reliably being saved. I'll investigate if I get a chance.

Munavijayalakshmi’s picture

use short array syntax as per new coding standard.

Munavijayalakshmi’s picture

Assigned: Munavijayalakshmi » Unassigned
Munavijayalakshmi’s picture

Needs work

dqd’s picture

Haven't scrolled the issue yet but please keep an eye on the core and AEL issues raising and getting progress regarding base fields. I think it is important to sync overlapping efforts here.

Too many to link them all here but some out of my head:
#3088103: Entity label (title) field shouldn't be a base field but a required option to choose a field for.
#2353867: [META] Expose Title and other base fields in Manage Display
#3043592: Allow entity to display label field as normal
#2877400: Find a way to make title handling generic in entity templates