Contrib.social

Refactor Claro's node stylesheet

Open on Drupal.org →
Created on 10 January 2023, almost 2 years ago
Updated 22 February 2023, almost 2 years ago

Problem/Motivation

This is a child of 🌱 [META] Update Claro CSS with new coding standards Active and part of 🌱 [PLAN] Drupal CSS Modernization Initiative Active .

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.

📌 Task
Status

Closed: works as designed

Version

10.1 ✨

Component
Claro  →

Last updated about 10 hours ago

Created by

🇺🇸United States Stockfoot

Live updates comments and jobs are added and updated live.
  • CSS

    It involves the content or handling of Cascading Style Sheets.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment almost 2 years ago →
    bspeare
  • Comment almost 2 years ago →
    🇮🇳India gauravvvv Delhi, India

    Gauravvv → made their first commit to this issue’s fork.

  • First commit to issue fork.
  • @royalpinto007 opened merge request.
  • Status changed to Needs work almost 2 years ago
  • Comment almost 2 years ago →
    🇮🇳India royalpinto007

    This change ensures that the margin of the node__submitted class only affects the top margin, which can improve consistency and readability. By using margin-top instead of margin, we can also reduce the potential for unintended side effects or conflicting styles that may arise from using the shorthand margin property.

  • Comment almost 2 years ago →
    🇺🇸United States hot_sauce

    @royalpinto007

    Switching to margin-top negates the bottom margin being set, which is needed, and also sets an inherited margin for the side margins, which may be need to be set as 0.

    We could refactor this to use:

    margin-block: 1em;
margin-inline: 0;

    But that is the same as what this file has currently of margin: 1em 0

    I think the shorthand for this file is fine and there may not be any actual changes needed here.

  • Comment almost 2 years ago →
    🇮🇳India gauravvvv Delhi, India

    I think the shorthand for this file is fine and there may not be any actual changes needed here.

    I agree with @hotsaucedesign here. This file doesn't need any modifications. It's already using shorthand property.

  • Status changed to Closed: works as designed almost 2 years ago
  • Comment almost 2 years ago →
    🇺🇸United States hot_sauce

    Thank you @Gauravvv, will mark this one as Closed (works as designed)

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024