Simplify the HTML of field.html.twig

kostask → closed merge request !7416

At DrupalCon Prague we decided that field--has-multiple-items and field--has-single-item were not communicating the correct information. We need to communicate the field cardinality in the database. When a user sees has-multiple or has-single they will connect this information what is rendered. A multivalue field can have just one entry, but in the original patch it would get a has-multiple-items class.

Just for clarity, the way field.html.twig currently works is that if it's a multivalue field - even if it only has one item in it - it gets the .field__items wrapper. If it has only one item, that item is wrapped in a div and if it has more than one item it uses ul.

The class we are intending to add here field--multiple will denote that this is a multivalue field, not how many items are in the field.
Perhaps we could add another class field--has-N-items to denote how many items we actually have.

Suggestion for classes:

  'field--name-' ~ field_name|clean_class,
  'field--type-' ~ field_type|clean_class,
  'field--label-' ~ label_display,
  multiple ? 'field--multivalue-field' : 'field--single-value-field',
  multiple ? 'field--has' ~ items|length ~ 'items'|clean_class : 'field--has-1-item',
  label_display == 'inline' ? 'clearfix',

Note: we will need to update the issue summary with new notes for the change record.

bserem → changed the visibility of the branch 3308309-simplify-the-html to hidden.

LB related tests fixed, working on CKEditor ones (the ones we haven't solved in years)

In the meanwhile, I don't really like the naming conventions and some of the classes that we are introducing here

- field--multivalue-field -> field--multiple-value-field, or field--multiple which is what already exists in Core
- field--single-value-field -> as is, or field--single which is what already exists Core
- field--has-multiple-items
- field--has-single-item -> field--has-one-item
- field--items-count-X -> is this really needed? Do we care about something like that or do we just pollute the DOM?

PS, `field_items` class is introduced to single value fields with the work on the branch. Thinking about it too

Since it's a long standing issue I also add the "Needs Review Queue Initiative" tag for others to check the solution provided too.

Before reviewing think this needs a rebase, 600+ commits

Will need some screenshots during testing to make sure nothing has broke.

The tests that had to be updated to pass, surprised they are using starterkit_theme maybe follow ups are needed to correct that.

I went on with the rebase of the commits. I think the tests do need a check from someone who knows what he's doing + also check what @smustgrave said about the use of the starterkit_theme.

I believe we need to change the target branch to 11.1 or something. The commits are awfully wrong in Gitlab after the rebase.
It's been less than 10 commits, we are seeing things already merged in Core.