quickedit_test_entity_view_alter() creates a non compliant field render array

I should ping somebody with significant d.o rights about removing the old component.

Hrm:

/**
 * Implements hook_entity_view_alter().
 */
function quickedit_entity_view_alter(&$build, EntityInterface $entity, EntityViewDisplayInterface $display) {
  $build['#attributes']['data-quickedit-entity-id'] = $entity->getEntityTypeId() . '/' . $entity->id();
}

So it is indeed 100% a test problem, i.e. the "simulation of a pseudofield generated by Display Suite" in quickedit_test_entity_view_alter() needs to be updated.

I don't know what exactly that should look like, but I think you do, yched? :)

Hmm yeah, I started fearing that. The reason why DS uses theme field is because DS also allows you to swap the field templates, for both 'core' fields, and letting the pseudo fields use theme_field is just convenient. But yes, this doesn't fly nicely anymore in D8.

I'll have to figure out whether I can register the fields that are created in DS UI or in code as something that TypedData will understand and then well run through a formatter I guess. However, we'd still need some kind of opt-out if possible.

Other - sad - option is to rework the code completely and register my own theme function. I'll have to think a bit about this - and also have a look at the code, it's been a while :)

Well the use case is that since DS is using field_theme - which pretty much is the only contrib afaik doing that - quickedit picks in and adds a pencil to edit the field. Now, DS fields aren't necessarily editable data (sometimes it is, sometimes it's just simple markup, or maybe a combination of some fields wrapped into one output with some funky manipulations), and there's no widget either so the pencil simply does nothing and the javascript breaks completely.

Basically, I guess you could say that in some way, DS fields are computed, maybe that's a way out of it, haven't investigated that option yet.

Uploading patch that removes this check - will try to test later today with DS to see how I can get around it

Looking at this today

To be clear, I'd love to see this "pseudo field" support in Quick Edit removed :) It was added solely for Display Suite.

Well it's added for anyone that wants to use the field theme function. It's strange to have a theme function that only can be used in a specific use case.

But I do see DS is the only contrib module doing this trickery at the moment.

So Yeah +1 to remove it if we don't loose field templates in DS.

Damn, I wish I could be in Barcelona...

Found a workaround so DS can still use theme_field(), see patch for DS at #2563925: Figure out if we need to use a different theme function for our fields

Changes: introduce '#is_multiple' on FormatterBase.

Also removed another is_object() check in template_preprocess_field().

You're right yched :)

This patch fully reverts #2057973: Don't add Edit module's data- attributes on pseudo fields (as used e.g. by Display Suite). Therefore, from a Quick Edit POV, this is definitely RTBC.

The first 2 hunks make tiny modifications to Field API. Do those need additional test coverage?

I guess that works, until some other (custom / contrib) field preprocess or template wants to access $field_item->getFieldDefinition() (or otherwise uses $field_item as a FieldItemListInterface), then the DS calls will break as well ?

Good point, the only thing I can come up with is that we should document this in DS - maybe even on the project page - and add an additional property on our field so that custom/contrib knows that this will be a invalid field. I'm fairly sure that accessing the field definition won't happen every 5 minutes around the planet, so I can totally live with that.

We should probably open an issue allowing to create runtime field definitions so DS can create a 'fake' field then, that should be possible for 8.1 no ?

Since, according to #10, DS using theme_field has the added drawback of kicking quick_edit in, which is not intended ?

Quick edit doesn't kick in anymore in our patch for DS since we're telling our fields that their view mode is _custom - that check exist in quickedit itself already. So at least this part is fine.

1)
DS has a mechanism to inject any arbitrary output into the entity display.
By using the defualt core field template to wrap the output we can ensure the display looks OK for any non coder/themer.

2)
On top of that, we have a feature called "field templates", it allows people to customize the field wrappers on a field basis in the UI.
Technically it is using field theme suggestions. This feature is used a lot.

I'm pro looking at different ways to achieve this goal. But at the moment I have no clue how to do it properly AND at the moment my time is very limited as I only can work on DS one day each month. It would be sad to not have DS finished when 8.0.0 is out.

The only way to decouple this completly without losing to much time is "duplicating" the field theme templates.
But I don't think this ia future proof solution.

I guess that works, until some other (custom / contrib) field preprocess or template wants to access $field_item->getFieldDefinition() (or otherwise uses $field_item as a FieldItemListInterface), then the DS calls will break as well ?

Yes that's true, in DS itself, we have some "not so nice" object checks.

Yay! :)

Committed e686c9d and pushed to 8.0.x. Thanks!

The wrong patch was committed, the one from #12, not #17

Attaching the interdiff from 17 again as a patch.

Oops indeed - thanks for noticing. Committed bfcba53 and pushed to 8.0.x. Thanks!

Could probably use some wisdom in #2604220: PHP notice for single value image field configured with a default image (no image present) and a hidden label please :)