[META] Apply formatters and widgets to rendered entity base fields (all besides node.title)

This issue is about converting all other entity properties: node.title, node.author, taxonomy_term.name, and so on.

Let's get a list of what all those other ones are into the issue summary, not just for node, but for all core entity types. We can find them by searching HEAD for function baseFieldDefinitions(). The question then becomes do we all agree that the scope of this issue should only be about *rendered* base fields, as per the issue title, and if so, how do we define that? For example, should we consider user.mail as a (potentially) rendered base field, even though it's normally considered private information and not typically rendered anywhere? Alternatively, "rendered" might be the wrong criterion here, so any thoughts on what the correct criterion should be is welcome. For example, while in-place editing only cares about rendered fields, translation UIs care about all editable fields. But maybe editable, non-rendered fields should be done in a separate issue?

Also, looks like user.signature and term.description are in process of being converted to configurable fields: #1548204: Remove user signature and move it to contrib and #569434: Remove taxonomy term description field; provide description field for forum taxonomy, so we should probably leave them out of the scope of this issue.

Alternatively, "rendered" might be the wrong criterion here, so any thoughts on what the correct criterion should be is welcome.

I left a comment on #1798140-11: Agree upon terminology for configurable fields asking for help figuring this question out.

#1988612: Apply formatters and widgets to rendered entity base fields, starting with node.title is now pretty far along with node.title as the pilot use case. So retitling this issue accordingly.

Support for widgets/formatters on node.title currently relies on a smart hack (altering the EntityDisplay object at runtime, making it treat 'title' both as an "extra fields" and as a "field").

That was needed to move forward, but before we start spilling similar hacks in more places for more base fields, it would be best to tackle #2144919: Allow widgets and formatters for base fields to be configured in Field UI, which defines the final "clean, non hackish way" for a base field to leverage widgets & formatters.

In case it's helpful, here's my recommendation on how to approach this, but I'm not attached to it being done this way:

Step 1) Identify all the fields we want to cover in this issue, and get to consensus on that list (see #2).

Step 2) Start with a patch that converts one field of each type, so that we can see what unique issues come up per type (e.g., node.created may turn up some tricky stuff due to being a date and node.uid may turn up some tricky stuff due to being an entity (user) reference) and work through those issues. At this stage, I think interim patches that continue with the hook_entity_display_alter() approach until #2144919: Allow widgets and formatters for base fields to be configured in Field UI is completed is fine, since that's just a few lines of easy code that can be removed from the patch once that issue is in.

Step 3) Finish by getting all the other fields converted, by which time, #2144919: Allow widgets and formatters for base fields to be configured in Field UI will hopefully be done.

Agreed, makes lots of sense.

Converting more base fields will probably involve changing their field type (like you had to do for node.title), since most base fields currently still use "old" basic field types that were designed for base fields only and predate "merge (configurable) Field API with the new EntityField API" - and those have no widgets/formatters at all...

So, that's some work in itself, and yep, the sooner it starts, the better

Raising priority per #1988612-52: Apply formatters and widgets to rendered entity base fields, starting with node.title.

#2111887: Regression: Only title (of base fields) on nodes is marked as translatable would also need this issue for an ideal solution. Currently even though that patch works for making base fields multilingual, it cannot make the UI match it (most properties still say "all languages"), because the form elements used for those base fields are in custom nested/named structures, so pairing up form elements with the base fields is not trivial. We can make up our own matching there, but if they become widgets, there is less of a point (although it all comes down to timeframes :/).

Related: I opened #2150511: [meta] Deduplicate the set of available field types

Nono, #2111887: Regression: Only title (of base fields) on nodes is marked as translatable enables translation configuration for author and date, but it is in no way required to make them use widgets and formatters. The two are not related. The UI for displaying them as translated or applying to all languages becomes *way* easier and cleaner once they are widgets, since then we don't need to match up base field names with arbitrary nested/names form elements.

In other words, *this* issue would help resolve the UI concerns in #2111887: Regression: Only title (of base fields) on nodes is marked as translatable so probably the other way around (although we can work around this there, but it will/would be ugly).

Hm. Actually, #2111887-25: Regression: Only title (of base fields) on nodes is marked as translatable brings up an interesting snag. Currently, the form element for the node.uid field is $form['author']['name'].

1) For widgets, we typically follow a 1:1 mapping of field name to (root) element name, so if we want to continue with that, this would become $form['author']['uid']. Are there any objections to doing that rename?

2) We don't have Fieldgroup in core, so we either need to add it, or implement the grouping into the 'author' element in some custom way. Thoughts?

@Gabor: If we accept doing the rename, can #2111887: Regression: Only title (of base fields) on nodes is marked as translatable move forward by doing that rename there, without waiting on the widget work here? Or does the fieldgroup issue also need to get figured out for that issue to proceed?

"This would become $form['uid']", you mean ?

Not sure of the impact of the fieldgroup thing (is that related to translatability ?), but field groups in core sounds a bit out of reach :-)

We could:
- Either make "author" not configurable in Field UI - the widget is uses is hardcoded in the field definition
- Or keep it configurable in Field UI, but reordering it has no effect, the form element gets form_altered into the fieldset ?

"This would become $form['uid']", you mean ?

Whether we add Fieldgroup to core (which I agree is a long shot, unless it's possible to strip it down to something very minimal and allow the contrib module to add the bells and whistles not needed by any core use cases), or do either of #16's alternate options, the end result will still be that it's $form['author']['uid'] after all the alters have run. So the key question of #15.1 is whether there's any reason to not rename the element from HEAD's $form['author']['name'] to what a widget would do by default: $form['uid'] which some other TBD code would alter to $form['author']['uid']?

Given that this will involve changing the field types of some base fields, and changing form element names in commonly altered forms, I strongly suggest trying to get it done before beta. If we get close to beta without this being close, we may need to split out a patch containing name and type changes to get in by beta, and allow the rest to happen after, but hopefully, we can just get the whole thing done by beta and not have to worry about that :)

"If this blocks the beta it by definition blocks the release" (@catch via #2047633-115: Move definition of menu links to hook_menu_link_defaults(), decouple key name from path, and make 'parent' explicit). Or is there a semantic difference between "beta target" and "beta blocker"?

As for the form element names, I think widgets work with top level keys on the form based on field names, no? So if at least the custom form elements are named according to the base fields (but are nested under other elements), so long as any nested element that is named like a field can be considered a representation of the field in the form (and the same key name is not reused on *any* level in the form for other things), that would be enough for [#8215017], however I think the widget solution would eventually make them appear on the root of the form, no?

There is a semantic difference between "beta target" and "beta blocker". The latter will block beta, which does also mean, block release, and therefore, implies critical. "beta target" means there are very compelling reasons to get into a beta, and if that fails, the issue needs to be reevaluated as to whether or not it needs to be postponed to 9.x.

Possibly, this issue should be "critical" and "beta blocker" on its own merits, but I'm not aware of a D8 committer approving that. Adjust the attributes accordingly if you believe it's appropriate.

however I think the widget solution would eventually make them appear on the root of the form, no?

Not necessarily. The part of the code that invokes the widgets to add their elements does result in those items being initially put at the root of the form, but modules like Field group end up running follow up code (an alter hook, I think, though maybe in D8, it will get integrated into the rendering pipeline some other way than an alter hook) that can move the elements into whatever organization is needed. If we do this issue without relying on Field group (per #16), then there will be code in NodeFormController or similar to do that grouping in a hard-coded way.

so long as any nested element that is named like a field can be considered a representation of the field in the form (and the same key name is not reused on *any* level in the form for other things), that would be enough for [#8215017]

Yes. That is reasonably safe to assume. We don't actually require widgets to comply with that (perhaps we should, but that's a separate issue), but WidgetBase does comply with it, and so long as no one argues against #15.1, we should comply with it for all core widgets. Therefore, see #2111887-28: Regression: Only title (of base fields) on nodes is marked as translatable.

Yes. That is reasonably safe to assume.

Oops. I should point out one caveat: if an entity form is inlined into a bigger form (e.g., a form that lets you edit multiple entities at once), then the uniqueness is only guaranteed within the section of the form that corresponds to a single entity. There are variables around to assist with determining that, but I don't recall them off the top of my head.

We don't actually require widgets to comply with that [nesting elements in an entry named after the field], but WidgetBase does comply with it,

Right, currently it's not enforced, it's just the base, usually non overriden, default.
Actually, I've been thinking a couple times that this wrapping shouldn't be the job of the widgets / formatters , but should be left to the caller code.
Widgets / formatters would be more flexible to use standalone (no forced wrapper to unwrap), and calling code could then enforce whatever it sees fit.
Just never found the time :-/.

But sure, separate issue - I opened #2151693: Widgets / formatters should return unwrapped $elements

To allow edit module JS each entity needs a wrapper element, needs opinions about default entity wrapper element of template #2186653: $build['#attributes'] added in hook_entity_view_alter() are not output for entity types that provide neither a #theme nor #theme_wrappers (breaks Quick Edit module for those entity types)

I'm going to follow this thread because I'm interested in this and DrupalSouth made me want to contribute. I'll pop into next week's core mentoring.

Worked a bit on this, so far, we only have widgets for string/text field types.

This will obviously break tons of tests that submit forms as the field names change.

Suppose better to split the issue into sub-issues on per module/entity basis because each entity needs special fixes for tests
For example:
- contact module #1856562: Convert "Subject" and "Message" into Message base fields
- feed entity description #2181549: Provide a StringLong field item with schema type 'text' without a 'format' column & #2149845: Convert the description field of the 'aggregator_item' entity to a text field
- taxonomy term #569434: Remove taxonomy term description field; provide description field for forum taxonomy
- user #1548204: Remove user signature and move it to contrib

@Wim Leers+++++++++++++++++++++++++++++++++

Meta, so active. :)

The summary is a little confusing. We're not making them configurable fields -- right? Just converting the custom code for rendering the elements to widgets and formatters?

Also, is it really true that base fields aren't translatable? I thought there was lots of work to make that possible.

@xjm : yeah, it seems most of those "apply formatters / widgets to [entity_type] base fields" were created with a shaky phrasing about "making base fields configurable" :-). Some of them have been corrected, but not all of them it seems.

So yes, those tasks really are about making existing base fields in existing entity types move from one-off ad-hoc render / form API elements to widgets and formatters - and not even necessarily in a "configurable way": in some cases the widget / formatter will just be hardcoded and not changeable in "Manage display" screens.

Adjusted the summary above.

+ Adjusted the summaries of the child issues that are still open to simply refer to this one.

Completing this issue might be the best way to resolve #2002180: Entity forms skip validation of fields that are edited without widgets, so adding it as a related issue, but see #2002180-45: Entity forms skip validation of fields that are edited without widgets for why I'm not yet promoting this issue to critical.

Just unassigning issues that haven't been developed for a bit in the D8 queue.