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.

Comments

Stockfoot created an issue. See original summary.

gauravvvv’s picture

Status: Active » Needs review
StatusFileSize
new3.38 KB

I have refactored the views_ui.admin.pcss.css file. I have improved nesting in the file.

Output: In the CSS fileviews_ui.admin.css there is only small change that too re-positioning on some CSS. Please review

bspeare’s picture

Version: 10.0.x-dev » 10.1.x-dev
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative
StatusFileSize
new497.53 KB

Reviewed #2

Change for nesting looks good. Attaching views ui screenshot to show nothing appears broken.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 3332737-2.patch, failed testing. View results

smustgrave’s picture

Status: Needs work » Reviewed & tested by the community

Random failure 1) Drupal\Tests\media\FunctionalJavascript\MediaSourceFileTest::testMediaFileSource

nod_’s picture

Status: Reviewed & tested by the community » Needs work

Nesting looks good, can we use logical properties? seems like it would help here

gauravvvv’s picture

StatusFileSize
new3.99 KB
new2.31 KB

Updated patch with logical properties. Attached interdiff for same.

gauravvvv’s picture

Status: Needs work » Needs review
santosh_verma’s picture

Tested the Patch #8,

Nesting and logical properties both are looking good to me.
patch applied cleanly, tested the UI manually nothing look like broken.

UI
patch

RTBC +

nod_’s picture

Status: Needs review » Reviewed & tested by the community

If you feel it's RTBC you can change the issues status to RTBC :)

  • nod_ committed d4ddbd2f on 10.1.x
    Issue #3332737 by Gauravvvv, Santosh_Verma, smustgrave: Refactor Claro's...
nod_’s picture

Status: Reviewed & tested by the community » Fixed

Committed d4ddbd2 and pushed to 10.1.x. Thanks!

Status: Fixed » Closed (fixed)

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