A stylesheet from the Classy theme is attached in templates/content/node.html.twig on line 84

{{ attach_library('classy/node') }}

I suppose it’s a copy-and-past leftover.

CommentFileSizeAuthor
#13 3157308-13.patch1.31 KBhansa11
#11 3157308-11.patch1.3 KBhansa11
#8 3157308-8.patch502 bytessteinmb
#5 interdiff_2-5.txt763 byteskishor_kolekar
#5 3157308-5.patch540 byteskishor_kolekar
#2 3157308-2.patch454 bytesriccardor

Comments

riccardoR created an issue. See original summary.

riccardor’s picture

riccardor commented
StatusFileSize
new 3157308-2.patch454 bytes

Provided patch changes the line as follows:

{{ attach_library('olivero/node') }}

steinmb’s picture

steinmb commented
Status: Needs review » Reviewed & tested by the community

Does not seem right. Patch apply cleanly

 git apply -v 3157308-2.patch
Checking patch templates/content/node.html.twig...
Applied patch templates/content/node.html.twig cleanly.

And addresses the issue. RTBC

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Title: Classy stylesheet attached from twig template for node » Classy stylesheet attached from Olivero twig template for node
Project: Olivero » Drupal core
Version: 8.x-1.x-dev » 9.1.x-dev
Component: Code » Olivero theme
Status: Reviewed & tested by the community » Needs work

This still needs to be addressed.

kishor_kolekar’s picture

kishor_kolekar commented
Status: Needs work » Needs review
StatusFileSize
new 3157308-5.patch540 bytes
new interdiff_2-5.txt763 bytes

please review the patch.

steinmb’s picture

steinmb commented

The patch apply correctly though I have a question:
When I look in core/themes/olivero/olivero.libraries.yml I do not see node: library defined. Am I confused (as usual)?

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

You are correct. There is no olivero/node library.

steinmb’s picture

steinmb commented
Status: Needs work » Needs review
StatusFileSize
new 3157308-8.patch502 bytes

Quick one liner.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

This is correct, however the classy/node library references one CSS file which contains one CSS rule that we need to duplicate in Olivero:

.node--unpublished {
  background-color: #fff4f4;
}

We need to add this to our node.pcss.css file, and add a comment stating that there is no variable for this color.

hansa11’s picture

hansa11 commented
Assigned: Unassigned » hansa11
hansa11’s picture

hansa11 commented
Assigned: hansa11 » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3157308-11.patch1.3 KB

Patch details:
Removed the classy library from Olivero's node.html.twig
Added the required CSS in node.pcss.css

Please review.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Status: Needs review » Needs work

This is looking soooo close! 

+++ b/core/themes/olivero/css/components/node.pcss.css
@@ -69,3 +69,8 @@
+  /*There is no variable for the color - #fff4f4*/

Need to insert a space before the beginning and after the end of the comment. Also need to add a period at the end of the sentence.

We need to apply coding standards to the comment (https://www.drupal.org/docs/develop/standards/css/css-formatting-guideli...)

hansa11’s picture

hansa11 commented
Version: 9.1.x-dev » 9.2.x-dev
StatusFileSize
new 3157308-13.patch1.31 KB

@mherchel: Thank you for the review.
Updated the patch as per your suggestion, please review.

Thanks!

hansa11’s picture

hansa11 commented
Status: Needs work » Needs review
mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Version: 9.2.x-dev » 9.1.x-dev
Status: Needs review » Reviewed & tested by the community

This looks good to me. Thanks!

alexpott’s picture

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

Committed and pushed bf29675611 to 9.2.x and 7d8ea2dccb to 9.1.x. Thanks!

  • alexpott committed bf29675 on 9.2.x 
    Issue #3157308 by hansa11, kishor_kolekar, riccardoR, steinmb, mherchel...

  • alexpott committed 7d8ea2d on 9.1.x 
    Issue #3157308 by hansa11, kishor_kolekar, riccardoR, steinmb, mherchel...

Status: Fixed » Closed (fixed)

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