Problem/Motivation
This is a child of #3324398: [META] Update Claro CSS with new coding standards and part of #3254529: [PLAN] Drupal CSS Modernization Initiative.
Steps to reproduce
The stylesheet at https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/themes/clar... needs to be refactored to make use of modern CSS and Drupal core's PostCSS tooling.
@todo: Add clear testing instructions to test this manually on the UI.
Proposed resolution
- Use CSS Logical Properties where appropriate.
- Use CSS nesting where appropriate.
- Use existing variables (
variables.pcss.css) where appropriate. Follow the proposed Drupal CSS coding standards to name the variables. - Add a comment when there's a value where there is not a variable like
font-size: 1.23rem; /* @todo One off value. */ - When possible, set variables at the root of the component and then map them to global theme variables:
.entity-meta { --entity-meta-title-font-size: var(--font-size-h5); ... more style } .entity-meta__title { font-size: var(--entity-meta-title-font-size); }
Out of scope
- Changing CSS classes
- Drupal 9 patches
User interface changes
None. There should be no visual differences.
Please post before/after screenshots and make sure they look the same.
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | Screen Shot 2023-03-07 at 8.53.34 PM.png | 234.65 KB | smustgrave |
| #12 | Screen Shot 2023-02-28 at 2.36.04 PM.png | 26.82 KB | smustgrave |
| #12 | Screen Shot 2023-02-28 at 2.36.08 PM.png | 22.25 KB | smustgrave |
| #12 | Screen Shot 2023-02-28 at 2.36.22 PM.png | 5.08 KB | smustgrave |
| #5 | Screenshot 2023-02-22 at 2.56.49 PM.png | 26.09 KB | gauravvvv |
Issue fork drupal-3332430
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
hot_sauce commentedI am working on this.
Comment #4
gauravvvv commentedComment #5
gauravvvv commentedI have attached the patch and before/after screenshots for reference. please review

Before patch:
After patch:

Comment #6
aziza_a commentedChecked changes - looks good
+RTBC
Comment #8
smustgrave commentedPatch #5 appears good.
Thanks for the screenshots!
Comment #10
hot_sauce commented@Gauravvv
Thank you for the work on your patch. I was working on an MR as part of Florida DrupalCamp last week and got around to finally pushing it up today.
In comparing my MR with your patch, I think there is still room for improvement on this to help modernize the CSS more.
- For
.pager__link, .pager__item--currentwe can do more conversion to CSS Logical properties by changingmin-widthandheighttomin-inline-sizeandblock-sizerespectively- We can move the CSS variables out of
:rootand into the.pagerclass since all of these will apply to anything within pager since it's best practice to not use:rootif we don't have to- In addition, the naming conventions for the
pagervariables wasn't consistent. Some used one - and some used two, so I adjusted that for consistency sake.- We can also make CSS variables out of the background images for the
.pager__itemclasses, and then further consolidate the@media (forced-colors: active)query to use these for themask-imageas well like thisThis is, admittedly, my first contribution to Drupal directly but I think that with some tweaking between the patch and the MR we can find the best solutions here.
Comment #11
gauravvvv commentedHi @hotsaucedesign, You have assigned the issue to yourself and there were no updates for the last 5 days. So I picked up the issue. I will make sure to ask first then only I will pick issues. thankyou
Comment #12
smustgrave commentedNesting looks good. Adding some screenshots to show they have not broken anything (that I see).
Comment #13
lauriiiComment #14
hot_sauce commented@laurii
Thank you for the review, I've pushed up the fixes and commented on a couple of things, please let me know if this warrants any further work!
Comment #15
smustgrave commentedRTL seems working
Comment #17
ckrinaCommitted 30a1e23 and pushed to 10.1.x. Thanks!
Note about the unresolved comment by @lauriii about the background image: the answer given by @hot_sauce about making it easier to theme
forced-colorsmade sense to me, plus discussed with @lauriii and he's OK with the existing state.