Problem/Motivation

In #2348543: [meta] Consensus Banana Phase 2, transition templates to the starterkit theme we moved a lot of templates from modules to Classy, the CSS that relies on the classes in Classy is still in the modules files

Proposed resolution

Move the CSS that relies on the HTML classes for every module. Ensure that the classes aren't being added in the module.

Remaining tasks

User interface changes

None for Classy, Stark will be more Stark

API changes

None

CommentFileSizeAuthor
#36 Screenshot 2015-09-27 20.20.06.jpg791.38 KBLewisNyman
#35 move_image_theme_css_to-2489574-35.patch2.14 KBLewisNyman
#32 2489574-interdiff-32-23.txt365 bytesjaxxed
#32 core-make_classy_imagewidget_a_component-2489574-32.patch2.13 KBjaxxed
#29 core-make_classy_imagewidget_a_component-2489574-20.patch2.13 KBjaxxed
#29 2489574-interdiff-29-23.txt365 bytesjaxxed
#29 core-make_classy_imagewidget_a_component-2489574-20.patch2.13 KBjaxxed
#24 move_image_theme_css_to-2489574-23.patch2.13 KBLewisNyman
#24 interdiff.txt1.23 KBLewisNyman
#18 move_image_theme_css_to-2489574-18.patch2.11 KBstar-szr
#14 move_image_theme_css_to-2489574-14.patch2.85 KBChernous_dn
#6 image-css-to-classy-2489574-6.patch3 KBManjit.Singh
#2 image-css-to-classy-2489574-2.patch2.27 KBManjit.Singh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Material commented
Assigned: Unassigned » Manjit.Singh
Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Material commented
FileSize
image-css-to-classy-2489574-2.patch2.27 KB

moving images css to classy :)

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Material commented
Component: forum.module » image.module
Assigned: Manjit.Singh » Unassigned
Status: Active » Needs review
LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Status: Needs review » Needs work 
core/modules/image/src/Plugin/Field/FieldWidget/ImageWidget.php:
  163  
  164      $element['#theme'] = 'image_widget';
  165:     $element['#attached']['library'][] = 'image/form';
  166  
  167      // Add the image preview.

We need to remove this attachment here.

The last submitted patch, 2: image-css-to-classy-2489574-2.patch, failed testing.

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Material commented
FileSize
image-css-to-classy-2489574-6.patch3 KB

removed code as per suggestion in #4

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Material commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 6: image-css-to-classy-2489574-6.patch, failed testing.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Issue tags: +Needs screenshots, +Seven

Nice, thanks. It looks like the test shouldn't be too hard to fix. We need to take screenshots before/after in Classy and Seven.

Status: Needs work » Needs review

Chernous_dn queued 6: image-css-to-classy-2489574-6.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 6: image-css-to-classy-2489574-6.patch, failed testing.

Status: Needs work » Needs review

Manjit.Singh queued 6: image-css-to-classy-2489574-6.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 6: image-css-to-classy-2489574-6.patch, failed testing.

Chernous_dn’s picture

Chernous_dn CreditAttribution: Chernous_dn at FFW for FFW commented
Status: Needs work » Needs review
FileSize
move_image_theme_css_to-2489574-14.patch2.85 KB

Update patch #6 I think that we need attach library to /core/themes/classy/templates/content-edit/image-widget.html.twig

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr as a volunteer commented
Related issues: +#2511036: image-widget.html.twig never gets used (image_widget gets overridden by FieldWidget::process)

Manually testing (or committing) this may be blocked on #2511036: image-widget.html.twig never gets used (image_widget gets overridden by FieldWidget::process).

lauriii queued 14: move_image_theme_css_to-2489574-14.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 14: move_image_theme_css_to-2489574-14.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr as a volunteer commented
Status: Needs work » Needs review
FileSize
move_image_theme_css_to-2489574-18.patch2.11 KB

Rerolled.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii as a volunteer and at Druid commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs screenshots

No visual changes so won't be uploading screenshots

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez as a volunteer commented

Why is the library called drupal.image?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii as a volunteer and at Druid commented
Status: Reviewed & tested by the community » Needs review

Actually to get this align with other issues we probably should remove that library

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented

@lauriii This was originally a separate library in the module, so I think it should stay a separate library in Classy?

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii as a volunteer and at Druid commented

Oh yeah and its being loaded conditionally. We still should rename the drupal.image to only image.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
FileSize
interdiff.txt1.23 KB
move_image_theme_css_to-2489574-23.patch2.13 KB

I renamed this library and the file to image-widget to better describe the contents.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii as a volunteer and at Druid commented
Status: Needs review » Reviewed & tested by the community
davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez as a volunteer commented

Should this go in a css/theme folder, since it is being added to the theme category?

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/themes/classy/classy.libraries.yml
@@ -27,3 +27,9 @@ search.results:
+    theme:
+      css/components/image-widget.css: {}

Nope this is a component so we should switch to component.

jaxxed’s picture

jaxxed CreditAttribution: jaxxed at Wunder commented
Assigned: Unassigned » jaxxed

looks like you only need the 1 line change, I'll add that to your patch.

jaxxed’s picture

jaxxed CreditAttribution: jaxxed at Wunder commented
FileSize
core-make_classy_imagewidget_a_component-2489574-20.patch2.13 KB
2489574-interdiff-29-23.txt365 bytes
core-make_classy_imagewidget_a_component-2489574-20.patch2.13 KB

just re-rolled with the small change. Couldn't find the right place to test it properly. Isn't this template supposed to load on any image field edit?

jaxxed’s picture

jaxxed CreditAttribution: jaxxed at Wunder commented

doh! double uploaded.

jaxxed’s picture

jaxxed CreditAttribution: jaxxed at Wunder commented

OK, I made a mess out of the last upload, so I am going to just upload it again.

jaxxed’s picture

jaxxed CreditAttribution: jaxxed at Wunder commented
Status: Needs work » Needs review
FileSize
core-make_classy_imagewidget_a_component-2489574-32.patch2.13 KB
2489574-interdiff-32-23.txt365 bytes

just re-rolled with the small change. Couldn't find the right place to test it properly. Isn't this template supposed to load on any image field edit?

(2nd attempt at upload, repeating #29)

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Status: Needs review » Postponed

It looks like this can't be tested until #2511036: image-widget.html.twig never gets used (image_widget gets overridden by FieldWidget::process) is committed so setting to postponed for now.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Status: Postponed » Needs review

Now we can test this patch

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Assigned: jaxxed » Unassigned
FileSize
move_image_theme_css_to-2489574-35.patch2.14 KB

Reroll.

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Issue summary: View changes
FileSize
Screenshot 2015-09-27 20.20.06.jpg791.38 KB

It loads! Setting to RTBC/postponed.

davidhernandez’s picture

davidhernandez
Primary language English
Location USA
CreditAttribution: davidhernandez as a volunteer commented

Did you mean to change the status?

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Status: Needs review » Postponed

Yes :P Thanks

akalata’s picture

akalata CreditAttribution: akalata commented
Status: Postponed » Closed (fixed)