Field Cardinality Style Update

This issue still depends on #3032365: Table drag style update and table styles.

And I think that when those are ready, this wont need any further action.

Most of my efforts for this issue this were in the scope of #3032365: Table drag style update, which I didn't spot as I was filtering by the Code component. I added a patch to that issue, which has a few changes that are specific to the scope of this issue. I can pry those apart as needed.

Table drag has been committed. This is now unblocked.

Not sure if tables headings should be managed in this issue. So keeped it as is.

Looks great.

I tested it and it now looks like this:

1. Shouldn't it been better that there would be more space between the text and the arrow down ? (between -10 and the arrow at the screenshot) On screens lower than 600px you don't have this problem. It also appears on the other selectboxes as well like this :

and

2. when you use the handler to sort the records the background color will be yellow so you can easily see that the record have changed.
You do not have that feature when you change the option from the select box, buts thats maybe out of the scope of this issue.

@lot007 I think the negative values causing less space in the select box isn't covered by the design yet and needs a review/little rework in case of design. I tried to get the spacings out of Figma but it isn't that clear how the values should be positioned.

I think #14.1 has to do more with the pre-existing select implementation and designs. Maybe it should be discussed in another issue?

#14.2 also seems out of scope.

I'm wondering if we could make the rows that have been changed look a bit nicer. Currently, the asterisk is pushed on a new line which doesn't look too great:

Moving to needs work for #16.

The spacing for the select was defined but probably not highlighted enough, but just on the select component itself. There should be a distance of 8px/0.5em between the arrow and the text content.

And +1 to what @lauriii says on #16 about the asterisk. Everything should be center-aligned and nothing apart from text should jump into a second line.

I tried to make some progress on this but I think we have to make some adjustments to the designs.

Here's how this looks at the moment:

1. I fixed bugs related normalizer.css abbr styles. Normalizer adds a border to abbr[title] elements that leads into double border (border + underline). I added override to Claro to remove text underline so that only border exists. This border has been then overridden in our table drag styles since the border doesn't look too useful in this particular use case.
2. Adjusted the spacing so that the asterisk pointing to the message is rendered on a single line with the handle. I also added some spacing between the handle and the field to make space for the asterisk. This should be probably adjusted to the design system.
3. Made the add another item button use button small component. According to the design system, it should use button medium variation but that doesn't exist 🤷‍♂️

I found one more issue with the design which is that on entity reference fields, the loading animation doesn't have enough space to properly render the loading text above the input element. This becomes even worse after #3054689: Implement green focus ring on text field has been committed. We should update the designs to take this into account.

In review.

1. +++ b/css/src/components/form.css
   @@ -159,3 +159,38 @@ td > .form-item:only-child,
   +
   +.field-multiple-table {
   +  margin: var(--space-m) 0;
   +}
   

   Instead of making the form component too big, please create a standalone asset for theming multiple field markup.

2. +++ b/css/src/components/form.css
   @@ -159,3 +159,38 @@ td > .form-item:only-child,
   +.field-add-more-submit.field-add-more-submit {
   +  margin-top: 0;
   +  margin-bottom: 0;
   +}
   

   This will look really odd if we have some description for the field (btw design does not have an example for the description...)

3. Required mark is missing from the field title.
4. I'd try to prevent tabledrag's functionality on disabled multi-field widgets.

Update for the description: https://monosnap.com/direct/QA3pZe1O6y46B776RitGV9kV73C21T

Test module with multiple text fields added to CD Tools: https://github.com/zolhorvath/cd_tools/releases/tag/1.10.0

@huzooka Did somebody already taken #20.4 into account (missing space)? We have a fix for it here, but don't know if that's the right issue for that fix.

Patch of the current state attached.

What happened since #20:

• I had to add Modernizr for submits as well: If you used the test form provided by cd_tools:fieldcardinality without a Toolbar, the small button variant wasn't in action because the lib was added only if there was a <button> element on the page.
• We had a bug with our vertical-tabs.js replacement: on IE11, the '_slicedToArray' shim (added by the JS build process) for handling destructured arrays broke IE11. I disabled that rule ant went back to the inherited variable assignment style.
• The new tableddrag rearranges the content of the first table cell from now. (This is the cell where the drag handle and the indentation elements are added). The content of these cells are themed as table and table-cells. This was needed for keeping everything vertically-aligned.
• Tabledrag is disabled for disabled multiple value form widgets.
• #20 reduced the cell spacing for multi-item field widgets. I've moved this to a more global scope, so the cell spacing is reduced for every draggable table.
• Required multiple value form widgets have a required mark :)
• Multiple value form widgets have a styled description.

Since our tabledrag functionality has changes, I've tested this new patch on multiple pages:

• /contact/field_cardinality_test, provided by cd_tools:fieldcardinality.
• On a taxonomy overview page, where terms have some hierarchy: /admin/structure/taxonomy/manage/tags/overview.
• On a Field UI (form/display) edit form: l/admin/structure/types/manage/article/form-display (or l/admin/structure/types/manage/article/display).

1. +++ b/js/claro.tabledrag.es6.js
   @@ -2,7 +2,11 @@
   + * The '_slicedToArray' shim added for handling destructured arrays breaks IE11,
   + * that is why the 'prefer-destructuring' rule is disabled.
   
   @@ -469,16 +472,27 @@
   +        .wrapInner('<div class="tabledrag-cell-content__item"/>')
   
   @@ -490,25 +504,28 @@
   -          [event] = event.originalEvent.touches;
   
   +++ b/js/claro.tabledrag.js
   @@ -6,8 +6,6 @@
   -var _slicedToArray = function () { function sliceIterator(arr, i) { var _arr = []; var _n = true; var _d = false; var _e = undefined; try { for (var _i = arr[Symbol.iterator](), _s; !(_n = (_s = _i.next()).done); _n = true) { _arr.push(_s.value); if (i && _arr.length === i) break; } } catch (err) { _d = true; _e = err; } finally { try { if (!_n && _i["return"]) _i["return"](); } finally { if (_d) throw _e; } } return _arr; } return function (arr, i) { if (Array.isArray(arr)) { return arr; } else if (Symbol.iterator in Object(arr)) { return sliceIterator(arr, i); } else { throw new TypeError("Invalid attempt to destructure non-iterable instance"); } }; }();
   

   Let's reference to https://github.com/babel/babel/pull/8947 here.

2. 
   We should probably remove the hover effect from the disabled table drag.
3. 
   The table drag handle column doesn't look great when viewed on a wide screen.

Addressing #30.

1. +++ b/claro.theme
   @@ -814,6 +817,65 @@ function claro_preprocess_fieldset(&$variables) {
   +      '#tag' => 'h4',
   

   I'm not sure if we actually want to make this a heading since this affects accessibility.

   This is already a heading 😲 Let's not do anything for this now but we should figure out if this actually should be a heading.

2. +++ b/claro.theme
   @@ -962,13 +962,29 @@ function claro_preprocess_table(&$variables) {
   +        if (is_array($first_cell)) {
   +          $row['cells'][$first_cell_key]['attributes']->addClass('tabledrag-cell');
   

   Should we check if the attributes object exists?

3. +++ b/css/src/components/tabledrag.css
   @@ -176,6 +179,15 @@ body.drag {
   +.tabledrag-cell:empty {
   

   What use case is this solving?

4. +++ b/css/src/components/tabledrag.css
   @@ -176,6 +179,15 @@ body.drag {
   +  padding-right: 0; /* LTR */
   ...
   +  padding-right: 0;
   +  padding-left: var(--space-m);
   

   It seems that the RTL styles are missing [dir="rtl"] from the selector and that the properties are a bit inconsistent.

5. +++ b/js/claro.tabledrag.es6.js
   @@ -7,6 +7,7 @@
   + * See https://github.com/babel/babel/issues/7597.
   

   🧐 Nit: let's use the @see syntax here.

6. 

   There's a regression on other table drags. Before the patch, the vocabulary name is displayed on one line, but after this patch, it's on two lines.

Just did some very quick testing with the latest patch. It seems to work more as I expected, but when the table drag has been moved, the asterisk is rendered very close to the field:

Re #36:

What is the expected spacing?
Since we have some issues because of our font sizes are big enough (it's a good and useful thing imho), I think that it is always a good idea to save some space wherever it is possible.

(BTW my recent patch addresses everything except this, but I will add my reactions for #33 a bit later, from home - I'm on a tram right now.)

@huzooka Maybe something similar to what I proposed in #20?

Re #38: What about something like this?
Visual diff on mobile:

Re #33:

1. Yes, it's the same what we inherit from core. I also think that it shouldn't be a heading.
2. Only if we cannot trust in core, see template_preprocess_table():
   • Line 966,
   • Line 1001, and
   • Line 1005
3. I've replaced this selector with a more-describing CSS class and added some comments. Basics: Multi field widget does not have anything in the first cell except the drag handle and maybe the changed mark. Field UI, taxonomy term overview have markup there (field label, term link).
4. Nice catch, thanks!!! Fixed.
5. Done.
6. This is also fixed, see point #3 above

There's a regression on the focus styles.

Before:

After:

I just checked this in IE11 and it lets the inline <svg> to be focused with keyboard.
I move this drag handle to a background-based solution.

If we use the background-image it results into no icon at all on the IE 11 high contrast which makes tabledrags unusable. 😕Can we think of any other approach?

Talking about the background image is striped out in IE11 HC-Mode: Maybe we can learn from this thread?
https://github.com/twbs/bootstrap/issues/21269

They discuss about replacing the svg with 'text' content (could also be a symbol?) in css. Doesn't work because isn't translatable.

Another possibility would be to deliver the svg (inline?) with content: url()

We can't really replace it with text in CSS since that needs to be translatable.

Re #44:
The previous drag handle was also the same on HC (screenshot with #20):

Let's open a follow-up to address this

+ I've to document my new additions to Claro's tabledrag.js replacement (and open the related FR to core).

Added the needed docs about the tabledrag-improvements added since #20.

Follow-ups:

• #3083051: Refactor tabledrag when core issues are resolved
• #3083048: Provide a tabledrag-handle that is visible in Windows high-contrast mode

Found some smaller bugs while I was generating the screenshots.

This looks good! Thank you for generating the screenshots and thanks to everyone who has helped on this.