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.1.x/core/themes/claro/css/base/elements.pcss.css 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

User interface changes

None. There should be no visual differences.
Please post before/after screenshots and make sure they look the same.

Issue fork drupal-3355248

Command icon 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

Gauravvvv created an issue. See original summary.

gauravvvv’s picture

Issue summary: View changes
StatusFileSize
new205.85 KB

Some RTL styling of elements.pcss.css file is overriding the normal CSS.

Additional spacing is added on RTL direction, because we're not using logical properties in elements.pcss.css file

Before patch

gauravvvv’s picture

Status: Active » Needs review
santosh_verma’s picture

I have tested after applying the the 1st patch (3332729-32.patch)
https://www.drupal.org/project/drupal/issues/3332729
the issue appears
before patch
before

after patch
after

issue resolved with the current MR
Current MR

santosh_verma’s picture

Status: Needs review » Reviewed & tested by the community
bnjmnm’s picture

Status: Reviewed & tested by the community » Needs work

Looks good overall, but found one more nesting opportunity in the MR. See the comment there.

rpayanm made their first commit to this issue’s fork.

rpayanm’s picture

Status: Needs work » Needs review

Please review.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Additional nesting seems good.

santosh_verma’s picture

Reviewed the comment #9 MR,
Nothing changed into css file after nesting, it looks good to me

RTBC +1

Sorry for the duplicate comment @smustgrave, I was working on it posted the comment without refreshing the page.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

  • lauriii committed 539ad62b on 11.x
    Issue #3355248 by Gauravvvv, rpayanm, Santosh_Verma, bnjmnm, smustgrave...

lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed 539ad62 and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

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