Follow up from #2136197: Move field/instance/widget/formatter settings out of annotation / plugin definition - see #102

Copying over from that issue

Just one more thing about the removal of the CT keys from the various annotations: @yched's summary makes some convincing points about the need to do that, so I won't argue about this, but the reason those settings are attached to field annotations is that they are specific to every field-type. CT cannot automatically provide that information for every field-type contrib may provide, so if we want to remove them we need to introduce a new way for field-type-defining modules to integrate with CT and provide it the information it needs. I guess this is what @jessebeach was referring to in #87 with @TranslationSync plugins. Those might be overkill given the kind of information we need to provide, but at very least a basic _info() hook would be needed.

Postponed until

#2136197: Move field/instance/widget/formatter settings out of annotation / plugin definition

CommentFileSizeAuthor
#11 interdiff.txt1.85 KBswentel
#11 2224779-11.patch8.21 KBswentel
#9 2224779-9.patch7.36 KBswentel
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

swentel’s picture

swentel CreditAttribution: swentel commented
Title: Allow field types to expose translatable info about it's properties » Allow field types to expose translatable info about its properties

typo

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Priority: Normal » Critical

If this is a beta blocker, it is critical.

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented
Issue summary: View changes
plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

I think @yched said the current status is ok regarding field type plugin definition (see https://drupal.org/comment/8606301#comment-8606301.) Do we still need this?

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

I guess we could repurpose this (and demote it!) to make field sync work with entity field definitions and remove configurable field special-casing.

swentel’s picture

swentel CreditAttribution: swentel commented
Assigned: Unassigned » yched

Yeah, I've been thinking about it too whether this one is critical, I also think we can demote, but it doesn't block us afaict. I'll quickly assign to yched to get his thoughts.

jessebeach’s picture

jessebeach CreditAttribution: jessebeach commented
Status: Postponed » Active

Unpostponed!

swentel’s picture

swentel CreditAttribution: swentel commented
Title: Allow field types to expose translatable info about its properties » Move 'column_groups' out of settings in a dedicated annotation
Assigned: yched » swentel

Working on this. We're going to move the 'column_groups' out of the settings into a dedicated annotation.

swentel’s picture

swentel CreditAttribution: swentel commented
Status: Active » Needs review
FileSize
2224779-9.patch7.36 KB

First patch - should be green. Need to work on the presentation now ;)

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented

Looking good! Just one remark:

+++ b/core/lib/Drupal/Core/Field/Annotation/FieldType.php
@@ -81,6 +81,13 @@ class FieldType extends DataType {
+  public $column_groups = array();

I guess we actually don't want this here :) It's a key provided by CT so it should be documented by CT somewhere. We had a similar issue with entity type annotations: #1968970: Standardize module-provided entity info documentation and clean-up the @EntityType annotation. Not sure what would actually be the best place in CT to document those. Probably in the content_translation_field_info_alter() PHP docs.

swentel’s picture

swentel CreditAttribution: swentel commented
FileSize
2224779-11.patch8.21 KB
interdiff.txt1.85 KB

Yeah, that makes sense. New patch moving it to content_translation_field_info_alter()

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
CreditAttribution: plach commented
Status: Needs review » Reviewed & tested by the community

Awesome! RTBC if green.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 4a06454 and pushed to 8.x. Thanks!

  • Commit 4a06454 on 8.x by alexpott: 
    Issue #2224779 by swentel: Move 'column_groups' out of settings in a...
yched’s picture

yched CreditAttribution: yched commented

w00t ! Thanks folks !

Status: Fixed » Closed (fixed)

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