Expose Title and other base fields in Manage Display in Drupal Core

@jonathanshaw Let's discuss this idea before changing the status and summaries of all the related issues.

Perhaps I didn't grasp your proposal, however at first glance it seems to break the rules for Core BC

See comment for explanation.

Thanks @catch. However I've thought so more and I'm pretty sure that it's impossible for this to be automatically enabled and still BC.

Imagine that a site edited node.html.twig as follows.

1. Change "Submitted by UUU on DDD" to "DDD: another cool article by UUU"
2. Moved the author picture after this sentence instead of before and wrap it in a div with some custom classes
3. Move the title after the author picture and use <h3> not <h2>.

How could we possibly even imagine an automatic migration process that would parse every single instance of node.html.twig on the site and create the exact same output using field formatters? So if we automatically enable manage display on this site, then there are two possibilities:

1. We lose the above customisation, reverting to the default title/submitted = big BC break
2. We keep the above customisation, in which case the site admin is able to move the title/author/created around in manage display but it's totally broken - it has no effect or generates bad markup.

Therefore if we are going to remove the extra preprocessing and move to field formatters (which is obviously desirable), there will be a cost:

1. We need a GUI setting to enable.
2. Every site and every theme that has customised title/submitted in node.html.twig will have to re-apply their customisation in a different way before they can upgrade to D11.

Would be useful to have thoughts on the acceptability of this cost.

Thanks @catch and @xmacinfo for continuing the discussion.

I don’t expect a theme working fine in Drupal 10 to work without any change in Drupal 11.

OK, good so it seems that we are in agreement on that.

In summary, this issue causes a modest migration step, but it seems fine with the normal deprecation mechanism so sites have a whole major release to do it.

On the other hand if we force people to do the migration step in a minor release (and possibly they don't realise the non-BC problem until they can no longer roll back) I feel it would not be welcomed.

The GUI option is not so difficult to write.

Right but there are lots of things you can do in manage display that don't actually look very good. It's not really different to adding a new field formatter that's incompatible with a custom or theme preprocess (which happened to me today).

As far as I can see it's unfortunately much more disruptive than that. I updated the IS with a list of options that I can imagine without a GUI setting. Number 3 is the best one, but even that I doubt is allowed in a minor release because it deletes template variables and loses customisations. Can anyone confirm/clarify on this?

I think we should switch the version of this ticket to 10.0.x-dev, though.

Good idea, done.

For this I think we're going to need multiple approaches:

That makes sense, I agree.

The SubmittedFormatter in Manage Display module replicates the current format exactly. If we want to discuss the benefits of that compared with extra fields, we should likely do it in #3033301: Add formatters and other mechanisms as alternative to base fields directly in entity templates.

And I'm wondering if we could dynamically set is_line based on the display configuration

In node_preprocess_field__node() we already set 'is_inline' dynamically based on enable_base_field_custom_preprocess_skipping and isDisplayConfigurable. This code already works in 3 cases:

• SubmittedFormatter
• Show fields separately with existing formatters
• Sites still using the extra custom preprocessing.

My expectation is that the is_inline variable and these special base field templates like field--node--uid.html.twig can be deprecated and eventually deleted. They are another part of the 'magic hackery'. We should have a mechanism to display "Submitted by" that works for any entity type including comment and media in core. It's not attractive to add the same magic hackery (quite complex!) to every similar entity type.

That would mean it wouldn't be possible to use both the submitted variable and also have the fields via manage display in a way that would work, but don't think we need to support that and we could even add validation for that case.

Absolutely, I agree, the two possibilities are fundamentally incompatible. Manage Display GUI of the base fields only works if enable_base_field_custom_preprocess_skipping is set, in which case the submitted variable is not set.

Many thanks @jonathanshaw for putting in so much time and energy even after my reply in the other issue was a bit negative. I haven't fully grasped all that you say. However I believe I have understood it enough to make some initial comments.

It might be the case that we each of us need to write some patches. I believe I can entirely imagine that patch I would write - indeed it is quite simply a case of transferring some code from Manage Display into Core. Perhaps I should write it first without the config setting. This would stand as a candidate solution for where we IMO are ultimately aiming for.

I don't yet understand how you would achieve some parts of the IS proposed resolution in your solution B.

1. The IS describes 'current' and 'new' base field definitions. How would you achieve the updating of the default type in Node::baseFieldDefinitions()? The current definitions seem highly unsuitable for use when base fields are present in Manage Display, so we need a strategy to change them. Arguably this is the key part of the whole BC debate, because there is no halfway option - somehow we need to make a change that's inherently not easily BC.

2. The IS says: "the GUI setting would also trigger the following changes:...". If there's no config option we would also need another way to achieve these parts.

I'm not sure what problems you have in mind, but my purpose in suggesting doing B1 first in 9.4 is to make sure that everyone who has enabled isDisplayConfigurable() has also updated their templates.

I believe we are too late for that. It seems that anything deprecated in D9.4 would not get removed until D11. I haven't seen this written down but that was what @larowlan said related to a different issue.

b. skip preprocessing if EITHER enable_base_field_custom_preprocess_skipping is set OR (field is ->isDisplayConfigurable(TRUE) AND field is a child element (i.e. has been explicitly configured to be visible).

NB the title, UID and created fields are currently visible by default so I don't see how to write code for your concept "explicitly configured to be visible". As soon as someone saves their display settings, config will be saved for the newly visible base field. If the admin didn't explicitly change them, then the defaults will be saved. So I am speculating that the admin makes a minor adjustment to any field display and suddenly their title and submitted display switches to all the "disastrous" scenarios I described earlier.

You've proposed a "Gui setting" ... If we go this way, I think we should default the setting to FALSE, both for existing sites and new installs.

I agree.

it avoids us having us to wait on developing a 100% perfect drop-in reimplementation of the existing experience

I agree it doesn't have to be a 100% identical replacement, however I feel it does have to be something 100% suitable and at least as good.

One of the key objectives is to deprecate and eventually remove the extra preprocessing. Before we do that, we would need something to replace it. I feel at minimum we need a title formatter, and some sort of solution for submitted information - these seem absolutely fundamental aspects of a CMS that should be in Core not Contrib.

I think this issue should not be blocked by #3033301: Add formatters and other mechanisms as alternative to base fields directly in entity templates

I think the issue as currently described is inevitably blocked because it proposes to update the type of a base field definition to something that doesn't yet exist. Possibly we could unblock it by reducing the scope here. However we should still make sure we know how to solve the entire problem eventually.

In summary, I agree it would be clearly advantageous to avoid a config setting, so it's a really good idea to explore that. On the other hand, a config setting is not really such a big problem. I am concerned that the alternative might actually create more problems - in the worst case could be fragile, confusing, and prolong the 'agony' of the current confusion multiple stages as we incrementally solve the problem.

Is there really any urgency to press us to make partial solutions? We've made really good progress so far and sites that want manage display can now get it - with some simple hooks, or using the contrib module "Manage Display".

Before insisting that the whole Drupal community adopt our initiative I feel we should ensure that we are offering something they will welcome, rather than dislike.

OK here's another way to look at it.

I have a proposal the create a new setting for Manage Display of base fields. This makes is very clear to people that they have a choice to make and what the implications are.

You are proposing to guess or deduce people's preference based on other settings. This could work, however I am rather doubtful. For example suppose someone hides the title field because they don't want it displayed. This would apparently alter your algorithm and re-enable extra preprocessing meaning the title field actually is displayed based on node.html.twig.

I have probably spent over 100 hours working on this area. I've tried lots of different things in code and my proposal is based on the best option I've found. I think you to explore your ideas further would be greatly assisted by showing a working patch. So many ideas that seem good in theory don't work out as expected when you try to implement them😃. I would very much welcome another developer to share the work and bring new ideas.

OK here's a new brain-storming idea

We create a new setting in manage display for the base fields: "Use node template", which means enable special preprocessing. This is used as the default for the base fields at least for all existing sites. However it is deprecated and later will be deleted.

Before we dive into more detail, I'd like to take a step back.

I have proposed a strategy:

• It has been available for discussion for 1-2 years.
• It has been tested out thoroughly with a contrib module Manage Display that has 1600 sites using it.
• The migration has been designed to be highly safe and BC: sites can choose when to adopt and can go back.
• We are starting to build momentum and support from core committers.
• The remaining work is mostly to transfer code from Manage Display that has been well tested.
• We can reasonably expect to have a working solution in D10 and remove old code in D11.

You have proposed a strategy:

• It has arrived just now, quite late in the initiative.
• It is only an idea with an untested code fragment. FYI your MR seems totally broken to me.
• The migration (IMO) is potentially risky and confusing: sites are forced to adopt and can't go back; the new UI is potentially confusing with unexpected effects that could break sites. I'm not convinced of your claim it's 100% BC.
• You apparently only solve part of the problem described in this issue.
• I've already spent approx 5 hours discussing it with you and you likely spent the same. With those 10 hours we could possibly have written a patch to add the TitleFormatter to Core.

Can you say in a nutshell what your outstanding concerns are now you see this?

First of all please can you explain your concerns with the established proposal😃.

I'm happy to bow out of the discussion at this point, as you say I'm taking up more of your time than I'm saving.

No I'm not trying to shut you out, rather I'm saying please let's be focused. I'm out of time for speculative discussion. There are two approaches that seem productive:

1. clear specific comments/questions
2. working patches that can be evaluated.

The MR has the wrong target, but the individual commits are fine and should clarify why I think this.

I'm afraid that they don't for me😃. If you can provide a valid patch for D9.4 that covers all 3 node fields then I am happy to test and discuss. The strategy and patches I am proposing are not perfect but are the best I've managed to get to work. It's easy to bring another solution and say that in a certain aspect it is better. However the alternative solution isn't much use if it fundamentally breaks another aspect. I don't currently see the seed of a working solution in the MR. However I would encourage you to continue to work on it and see if you can bring one out.

I don't see we have to wait on #3033301

I.e. you propose some intermediate state - we would set isDispayConfigurable(TRUE) first and then the other changes from the IS in a separate issue. Your MR even suggests a series of 4 separate commits/issues. In response I would ask

• It seems like more work - what is the advantage?
• What exactly are all the intermediate states?
• Do they all work, and make complete sense as a working system? (Apparently not IMO.)
• Is the transition from each one to the next fully BC? (Apparently not IMO.)

2. Whether we use the config or not, I don't see we have to wait on #3033301: Add formatters and other mechanisms as alternative to base fields directly in entity templates before we set isDispayConfigurable(TRUE).

Please see the end of the issue summary where I already presented a list of three possible ways to set isDispayConfigurable(TRUE) and their respective problems.

Many apologies Jonathan if I have caused offence. I'm really not wanting to shut down all debate, so I truly hope others will continue to contribute. I have found out the hard way that lots of ideas that seem promising turn out to be very difficult when you start to code them. If this issue becomes a dialogue of others saying "what about this or this or this" and I try to explain for each of them in turn where I think it will go wrong then it takes up more time that I currently have. Please do try to write some code and discover for yourself the answer to whether it works or not. Of course watch out for BC, especially if changing base field definitions or preprocessing. If anyone can produce a simpler solution that what I'm currently suggesting then I will be delighted.

Already in #27 and #29 I describe several problems that are apparently still present in the MR.

1. It seems that anything deprecated in D9.4 would not get removed until D11.
2. The title, UID and created fields are currently visible by default so I don't see how to write code for your concept "explicitly configured to be visible". As soon as someone saves their display settings, config will be saved for the newly visible base field. If the admin didn't explicitly change them, then the defaults will be saved. So I am speculating that the admin makes a minor adjustment to any field display and suddenly their title and submitted display switches to all the "disastrous" scenarios I described earlier.
3. Suppose someone hides the title field because they don't want it displayed. This would apparently alter your algorithm and re-enable extra preprocessing meaning the title field actually is displayed based on node.html.twig.

However, I still don't see why this issue has to be postponed on #3033301

Please see end of issue summary. The text has been there for some while, and I've been trying to persuade you to read it😃.

The whole essence of the #2353867: [META] Expose Title and other base fields in Manage Display initiative is that Manage Display of base fields is broken. If we enable it without fixing the problem then we give everyone a broken UI. Any attempt to move the author field around in the display order has no effect. If you set a different formatter then it can cause invalid HTML inside a span tag.

It doesn't matter if enabling the opt-in makes stuff behave weirdly, it's simply an advanced option that it only makes sense to enable (at this point) if you have additional modules/themes that fix the problem.

Here is what @catch said

Right but there are lots of things you can do in manage display that don't actually look very good. It's not really different to adding a new field formatter that's incompatible with a custom or theme preprocess (which happened to me today).

In that case, the site owner would have to install a different theme, or if it's a custom theme, handle the fields differently in the template, and then update the configuration and theme at the same time.

Then in #23 I said this

As far as I can see it's unfortunately much more disruptive than that. I updated the IS with a list of options...

And in the IS is says this

It would not help at all to change the theme or the templates. The only fix is to disable the extra pre-processing which requires custom code.

Only isDisplayConfigurable()

Here is my analysis on a possible patch that sets isDisplayConfigurable() but nothing else:

Sites that want the advanced behaviour can already have it right now, without any patch. They can enable Manage Display module, or some other custom module. It's simply a matter of calling isDisplayConfigurable() and $entity_type->->set('enable_base_field_custom_preprocess_skipping').

A patch that sets isDisplayConfigurable() leads to a broken user experience in Manage Display. How can you fix it? Well you can install Manage Display module. In which case the patch was unnecessary.

So I don't understand in what situation it is possible to benefit from this patch.

Changing base field definitions

In #27 item 1 I said this

The IS describes 'current' and 'new' base field definitions. How would you achieve the updating of the default type in Node::baseFieldDefinitions()? The current definitions seem highly unsuitable for use when base fields are present in Manage Display, so we need a strategy to change them. Arguably this is the key part of the whole BC debate, because there is no halfway option - somehow we need to make a change that's inherently not easily BC.

In my view, this is a strong argument for creating a setting. How else can we change the base field definitions? In the MR there is a commit that nonchalantly changes the formatter type of title, but as far as I can see that would be a significant BC break. If we have a setting, then it's very natural that it would also enable isDisplayConfigurable() etc. Hence the strategy proposed in the IS.

===

Anyway I'm exhausted from this debate. I have unsubscribed from this issue, and please feel free to create whatever patches you wish. I feel I have achieved something by completing stage A of the meta issue, and creating Manage Display module that works without patches to core. I'm quite ready for someone else to take over for the next stage.