
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.
Issue fork drupal-3332426
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
bspeare CreditAttribution: bspeare commentedComment #6
royalpinto007This 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 #7
hot_sauce CreditAttribution: hot_sauce as a volunteer commented@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:
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 #8
gauravvvv CreditAttribution: gauravvvv at Axelerant for Drupal India Association commentedI agree with @hotsaucedesign here. This file doesn't need any modifications. It's already using shorthand property.
Comment #9
hot_sauce CreditAttribution: hot_sauce as a volunteer commentedThank you @Gauravvv, will mark this one as Closed (works as designed)