Table style update

Attaching Seven screenshots as a reference.

Missing:
* Tabledrag (example: Field UI form)
* Available translation updates table (/admin/reports/translations)
* Extend / Uninstall table (/admin/modules, /admin/modules/uninstall)
* warning and error states of table rows (/admin/reports/updates)

https://www.drupal.org/files/issues/2018-12-03/table.png

Increased row height

An increased row height will display fewer items per screen and require more scrolling, creating a greater cognitive load for some users.

Recommendation:

Provide an affordance to choose alternative settings for table cell padding to vary the row height, like Gmail's Comfortable/Cozy/Compact settings.

1. +++ b/css/src/components/tablesort-indicator.css
   @@ -4,23 +4,43 @@
   +  margin-top: -8px;
   ...
   +  width: 14px;
   +  height: 16px;
   

   Shouldn't these values be converted in rem ?

2. +++ b/images/src/sort--asc.svg
   @@ -0,0 +1,9 @@
   +  width='14'
   +  height='12'
   +  viewBox='0 0 10 12'>
   
   +++ b/images/src/sort--desc.svg
   @@ -0,0 +1,9 @@
   +  width='14'
   +  height='12'
   +  viewBox='0 0 10 12'>
   

   Cannot the differences of values between the width and the viewport be a problem ? (alignments for instance)

#24.1: Done.

#24.2:

• About SVG viewBox
• About SVG preserveAspectRatio

Where has the hover effect come from? It is not described in the design system. We should either update the design system or change the implementation.

Here's a screenshot of the hover effect for anyone wondering:

From https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/Design-system?node-i...

Thank you for pointing that out. I must have missed it somehow 😇

There was some discussion on Slack and as a result, @saschaeggi and @ckrina changed the styles for hover and active states of the sortable table headers. We should implement that change before committing this.

@lauriii about the hover state, see the specification below:

Cheers

#33 #34 This is already implemented.

1. +++ b/claro.theme
   @@ -431,3 +433,63 @@ function claro_preprocess_radios(&$variables) {
   +      /* For 8.6.x and below. */
   

   Good job on testing this with both versions. We should add a @todo to remove this after 8.6.x is out of support.

2. +++ b/claro.theme
   @@ -431,3 +433,63 @@ function claro_preprocess_radios(&$variables) {
   +              $header_cell['attributes']->addClass('th--sortable');
   ...
   +          $header_cell['attributes']->addClass('th--sortable');
   ...
   +          $header_cell['attributes']->addClass('th--sortable');
   

   If we provide th--sortable class, we should have th class as well. Maybe it could be changed to something like sortable or sortable-heading instead?

3. +++ b/css/src/components/tables.css
   @@ -14,101 +19,137 @@ caption {
   +.th--sortable > a {
   ...
   +[dir="rtl"] .th--sortable > a {
   ...
   +.th--sortable > a::before {
   ...
   +.th--sortable > a::before {
   ...
   +.th--sortable > a::after {
   ...
   +.th--sortable > a:focus::before,
   +.th--sortable > a:hover::before {
   ...
   +.th--sortable > a:focus::after,
   +.th--sortable > a:hover::after {
   ...
   +.th--sortable.is-active > a {
   ...
   +.th--sortable.is-active > a::before {
   ...
   +.th--sortable.is-active > a::after {
   

   Did you consider adding class to the link itself? It seems like it could be done in the template.

4. +++ b/css/src/components/tables.css
   @@ -14,101 +19,137 @@ caption {
   +  border-bottom: 0.1875rem solid transparent;
   

   For the hover state, this should 2px.

#40.1 Done. I also switched comment styles.

#40.2 It's sortable-heading from now.

#40.3 It's theoretically possible.
The problem is that we should completely refactor Views table implementation (eg removecolumn.url and generate column.content as a link renderable) and since it's impossible to generate a query-only url what views provides there, I'm afraid of having issues in that case.

#40.4 I don't think so.

It seems like it wasn't updated everywhere. If you look at the component spec, it has a clearer explanation of the design:

omg... I always miss something :/

@huzooka there is another thing we should add. The Highlight color (hover) for the table row. See https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/Design-system?node-i...

Sorry for that!

Seven provides a bg color for a selected table row, don't we want to do something equivalent?

Yes we want to do something similar. I've added the active state.
See https://www.figma.com/file/OqWgzAluHtsOd5uwm1lubFeH/Design-system?node-i...

Hover + active state updated.

New 'after' screenshots attached.

The active column is missing background on active rows:

The recently applied autocomplete patch contains the improvements of this issue.

Removing the needs accessibility review tag, I think @modulist's proposal in #20 is great, and it has a follow-up issue filed.

A couple more feature requests to make the table styles more future-proof. The patch here is already quite big so I made them follow-ups to this.

Why needs work?

No idea, wasn't intended. I might have had the form open at #72

One more problem I could find is that the tabledrag handle is not aligned with text in a table row:

I don't really see the advantages of re-arranging the tabledrag handle until we miss Table drag styles (especially only the dragging and the last-dragged colors are missing if we can assume that the handle icon is final here).

Another follow-up for table drag: #3032365: Table drag style update

This looks good now! Thank you!

+++ b/css/src/components/tables.css
@@ -4,9 +4,14 @@
 table {
-  width: 100%;
-  margin: 0 0 10px;
+  min-width: 100%;
+  margin-top: var(--space-l);

This changes causes some admin modals to display over the full viewport width. I have filed #3021388: Table style update to fix this.