Followup for #1788542: Use EntityFormController and EntityListController for image styles.

Convert core/modules/image/css/image.admin.css call from ImageStyleFormController into a library.

CommentFileSizeAuthor
#16 2049743-image_library-16.patch2.62 KBmondrake
#13 2049743-image_library-13.patch2.59 KBmondrake
#12 2049743-image_library-12.patch2.74 KBmondrake
#12 interdiff_11-12.txt344 bytesmondrake
#11 2049743-image_library-11.patch2.74 KBmondrake
#11 interdiff_9-11.txt2.61 KBmondrake
#9 2049743-image_library-9.patch2.71 KBmondrake
#5 2049743-image_library-5.patch7.28 KBmondrake
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Vasiliy Grotov’s picture

Vasiliy Grotov CreditAttribution: Vasiliy Grotov commented
Assigned: Unassigned » Vasiliy Grotov

Will work on this on the weekend.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea commented

Great. Thank you! But I still think you should wait for #1788542: Use EntityFormController and EntityListController for image styles to get committed first. There are big form changes there.

Vasiliy Grotov’s picture

Vasiliy Grotov CreditAttribution: Vasiliy Grotov commented

Ok, looking forward to #1788542: Use EntityFormController and EntityListController for image styles.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea commented
Status: Active » Needs work

@Vasiliy Grotov, as #1788542: Use EntityFormController and EntityListController for image styles is in, you can start with this one :)

I see 2 occurrences:

$ grep -nr image.admin.css core/modules/image
core/modules/image/lib//Drupal/image/Form/ImageEffectFormBase.php:63:    $form['#attached']['css'][drupal_get_path('module', 'image') . '/css/image.admin.css'] = array();
core/modules/image/lib//Drupal/image/Form/ImageStyleEditForm.php:61:    $form['#attached']['css'][drupal_get_path('module', 'image') . '/css/image.admin.css'] = array();
mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
2049743-image_library-5.patch7.28 KB

Let's give a try.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
Component: image system » image.module
mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
Title: Convert image.admin.css into a library » Convert image.admin.css and image.theme.css into a library
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Needs review » Needs work
Issue tags: +Needs screenshots, +Needs manual testing 
  1. --- /dev/null
+++ b/core/modules/image/css/admin.ui.css

index 3e1f901..0000000
--- a/core/modules/image/css/image.admin.css

    Why this name change? That doesn't fit our standards at all. 

  2. +++ /dev/null
deleted file mode 100644
index 3da51ef..0000000

index 3da51ef..0000000
--- a/core/modules/image/css/image.theme.css

+++ /dev/null
--- /dev/null
+++ b/core/modules/image/css/widget.css

    Same here.

Also aren't you missing the image.libraries.yml?

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
Assigned: Vasiliy Grotov » Unassigned
Status: Needs work » Needs review
FileSize
2049743-image_library-9.patch2.71 KB

Thanks for review.

1, 2 - OK, reverted. Not sure about the naming of the library elements though, then.

aren't you missing the image.libraries.yml?

? it was in the patch in #5.

Anyway - new patch here, no interdiff, the patch is just much simpler :)

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented 
  1. +++ b/core/modules/image/image.libraries.yml
@@ -0,0 +1,11 @@
+admin:
...
+widget:

    This needs to be image.admin and image.widget 

  2. +++ b/core/modules/image/image.libraries.yml
@@ -0,0 +1,11 @@
+    component:

    This should be theme, not component

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
FileSize
interdiff_9-11.txt2.61 KB
2049743-image_library-11.patch2.74 KB

Thanks. Here we go.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
FileSize
interdiff_11-12.txt344 bytes
2049743-image_library-12.patch2.74 KB

Sorry.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
FileSize
2049743-image_library-13.patch2.59 KB

Reroll for #2247991: [May 27] Move all module code from …/lib/Drupal/… to …/src/… for PSR-4

mondrake queued 13: 2049743-image_library-13.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 13: 2049743-image_library-13.patch, failed testing.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
FileSize
2049743-image_library-16.patch2.62 KB

Rerolled

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
Status: Needs work » Needs review
mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
Status: Needs review » Closed (duplicate)

This has been fixed in #2378095: Convert all remaining attached individual CSS/JS assets to attached asset libraries.