Problem/Motivation

See: #2489460: [Meta] Move module.theme.css files to Classy or Seven

Proposed resolution

Move the CSS file to classy
Create a library for the CSS file
Add the library in the Classy twig template

Remaining tasks

  • Add screenshots.

User interface changes

None for Classy, Stark will be more Stark

API changes

None

Stark:

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because theme system standards
Issue priority Not critical because theme standards
Unfrozen changes Unfrozen because it only changes CSS

Classy:

CommentFileSizeAuthor
#19 interdiff.txt2.92 KBstar-szr
#19 move_comment_theme_css-2489560-18.patch4.57 KBstar-szr
#16 comment.theme_.css-classy.png114.46 KBstar-szr
#15 move_comment_theme_css-2489560-15.patch2.59 KBmortendk
#13 Screenshot 2015-05-26 16.15.11.jpg695.71 KBLewisNyman
#13 Screenshot 2015-05-26 16.13.11.jpg757.09 KBLewisNyman
#11 2489560-11-comment-to-classy.patch2.82 KBManjit.Singh
#11 interdiff-2489560-4-11.txt851 bytesManjit.Singh
#10 comments-after-applying-patch.png151.56 KBManjit.Singh
#10 comments-before-applying-patch.png150.78 KBManjit.Singh
#4 2489560-4-comment-to-classy.patch2.81 KBManjit.Singh
#2 2489560-2-comment-to-classy.patch2.17 KBManjit.Singh
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mortendk’s picture

mortendk CreditAttribution: mortendk commented
Parent issue: » #2489460: [Meta] Move module.theme.css files to Classy or Seven
Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Srijan | A Material+ Company commented
Status: Active » Needs review
FileSize
2489560-2-comment-to-classy.patch2.17 KB

Moving comment css file to classy

LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Status: Needs review » Needs work 
+++ b/core/themes/classy/classy.libraries.yml
@@ -3,3 +3,9 @@ base:
+comment-threaded:
+  version: VERSION
+  css:
+    theme:
+      css/comment/comment.theme.css: {}
\ No newline at end of file

We need a newline at the end of this file

We still need to remove the reference to the removed library here:

core/modules/comment/src/CommentViewBuilder.php:
  184        // Add indentation div or close open divs as needed.
  185        if ($build['#comment_threaded']) {
  186:         $build['#attached']['library'][] = 'comment/drupal.comment.threaded';
  187          $prefix .= $build['#comment_indent'] <= 0 ? str_repeat('</div>', abs($build['#comment_indent'])) : "\n" . '<div class="indented">';
  188        }

It also looks like we still have the indented classes here that should be in classy?

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Srijan | A Material+ Company commented
FileSize
2489560-4-comment-to-classy.patch2.81 KB

Done with the changes mentioned in #3. Please verify.

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Srijan | A Material+ Company commented
Status: Needs work » Needs review
LewisNyman’s picture

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

Nice, thanks. I think we need before/after screenshots of Classy.

sqndr’s picture

sqndr CreditAttribution: sqndr as a volunteer commented
Issue tags: +Novice

Added novice tag.

sqndr’s picture

sqndr CreditAttribution: sqndr as a volunteer commented
Issue summary: View changes
lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Druid commented
Status: Needs review » Needs work

Needs work for needs tag

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Srijan | A Material+ Company commented
FileSize
comments-before-applying-patch.png150.78 KB
comments-after-applying-patch.png151.56 KB

CSS file is not calling in classy :(

comments-before-applying-patch.png

comments-after-applying-patch.png

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Srijan | A Material+ Company commented
FileSize
interdiff-2489560-4-11.txt851 bytes
2489560-11-comment-to-classy.patch2.82 KB

Based on the discussion at #2489460: [Meta] Move module.theme.css files to Classy, at this point library names in Classy should match what is in the core module. So, comment-threaded should be drupal.comment.threaded.

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Srijan | A Material+ Company commented
Status: Needs work » Needs review
Issue tags: -Needs screenshots, -Novice
LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
FileSize
Screenshot 2015-05-26 16.13.11.jpg757.09 KB
Screenshot 2015-05-26 16.15.11.jpg695.71 KB

I tested thread comments in both Stark and Classy and I can confirm that the comments indent in Classy but no longer indent in Stark. Thanks!

Stark:

Classy:

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Druid commented
Status: Reviewed & tested by the community » Needs work

Bartik overrides comment.html.twig so comment.theme.css won't be attached for Bartik.

mortendk’s picture

mortendk CreditAttribution: mortendk as a volunteer commented
Status: Needs work » Needs review
FileSize
move_comment_theme_css-2489560-15.patch2.59 KB

rerolled the patch so it adds in the library again

but we have a problem with attach_library as in bartik adding in {{ attach_library('classy/drupal.comment.threaded') }} dont attach the file neither does {{ attach_library('@classy/drupal.comment.threaded') }}
So how do we add from another lib ?

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr at Northern Commerce commented
FileSize
comment.theme_.css-classy.png114.46 KB

@mortendk seems fine here. I cleared caches and made a comment on a node and viewed that node:

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr at Northern Commerce commented

Would be nice to only attach when threaded though, that's what the logic before was doing…

mortendk’s picture

mortendk CreditAttribution: mortendk as a volunteer commented

oki well then i guess its ready to roll ;)
... me wonders wtf is up in my build

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr at Northern Commerce commented
FileSize
move_comment_theme_css-2489560-18.patch4.57 KB
interdiff.txt2.92 KB

Implemented #17.

mortendk’s picture

mortendk CreditAttribution: mortendk as a volunteer commented
Status: Needs review » Reviewed & tested by the community

good call on the variable set - comment.theme.css now only added if comments actually exist & in bartik as well

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Chapter Three commented
Status: Reviewed & tested by the community » Fixed

The is a non disruptive CSS change. Committed ae86ffd and pushed to 8.0.x. Thanks!

Can the meta issue have the proper beta evaluation added - thanks.

  • alexpott committed ae86ffd on 8.0.x 
    Issue #2489560 by Manjit.Singh, Cottser, mortendk, LewisNyman: move...
LewisNyman’s picture

LewisNyman
he/him
CreditAttribution: LewisNyman at Wunder commented
Issue summary: View changes

Evaluation added.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.