Olivero table.css should be in its own library and #attached to tables

I am working on it.

Please review the changes.

Seems pretty straight forward

@sandip poddar looking at your post history, could probably elevate past novice tagged issues. Thanks!

Thanks @smustgrave! I'll definitely start looking into more advanced issues.

The css is not added to pages with a table, something doesn't work here. I checked /admin/content with olivero as admin theme.

The file needs to have the deprecation dance: see https://www.drupal.org/about/core/policies/core-change-policies/how-to-d... → for an example you can check 📌 Move resize CSS into its own library Active

Hi @nod_ thanks for the review. I wanted to clarify that I can see the table.css file being added correctly. For testing purpose, I intentionally added background-color: blue to the table tag, and I verified in the Network tab also that the stylesheet is loading as expected. Can you again have a look at it please.

For reference, I’ve attached a screenshot:

Thanks again for your guidance and for providing these valuable resources. I haven’t try any recordable changes yet, but I’m going through the documentation you shared and trying to implement the same here.

Sorry my bad! that thing is only works for table.html.twig not working for view-view-table.html.twig. Sorry for the above noice i did not notice it before. I am fixing it.

Here is the change record: https://www.drupal.org/node/3517675 →

Please review the changes.

Cleaned up the CR a bit. Don't think we needed to include the MR as a patch, and tweaked the versions.

nod_ → closed merge request !11620

Committed ab9823e and pushed to 11.x. Thanks!

Somehow I missed this.

Tables can also be added via CKEditor, which is why we had it as a global library. You can see at https://git.drupalcode.org/project/drupal/-/blob/11.2.x/core/themes/oliv..., we're also targeting tables that appear in .text-content (which is formatted text).

@mherchel - best to re-open issues if there's a problem or create a follow-up, it's easy for comments on fixed issues to go missing.

I've reverted this from 11.x and 11.2.x for more discussion. We could potentially add the library when text fields are rendered too.

Hi @catch, @mherchel
Can we try something like this below. Is it the right approach for it?

diff --git a/core/themes/olivero/olivero.theme b/core/themes/olivero/olivero.theme
index b2f3bff2684..0ee7fed1c81 100644
--- a/core/themes/olivero/olivero.theme
+++ b/core/themes/olivero/olivero.theme
@@ -365,6 +365,7 @@ function olivero_preprocess_field(&$variables): void {
 
   if (in_array($variables['field_type'], $rich_field_types, TRUE)) {
     $variables['attributes']['class'][] = 'text-content';
+    $variables['#attached']['library'][] = 'olivero/olivero.table';
   }
 
   if ($variables['field_type'] == 'image' && $variables['element']['#view_mode'] == 'full' && !$variables["element"]["#is_multiple"] && $variables['field_name'] !== 'user_picture') {

New change:

/**
 * Implements hook_preprocess_HOOK().
 */
function olivero_preprocess_field(&$variables): void {
  $rich_field_types = ['text_with_summary', 'text', 'text_long'];

  if (in_array($variables['field_type'], $rich_field_types, TRUE)) {
    $variables['attributes']['class'][] = 'text-content';
    $variables['#attached']['library'][] = 'olivero/olivero.table';
  }

  if ($variables['field_type'] == 'image' && $variables['element']['#view_mode'] == 'full' && !$variables["element"]["#is_multiple"] && $variables['field_name'] !== 'user_picture') {
    $variables['attributes']['class'][] = 'wide-content';
  }
}

If you agree i am updating this in the MR so.

that looks reasonable

sandip → reopened merge request !11620

@nod_ sorry for the delay. I've raised the MR and attached some images for clarity. Moving the issue to NR.