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 |
---|---|---|---|
#7 | Screenshot 2023-02-21 at 12.41.31 PM.png | 36.82 KB | Gauravvvv |
#7 | Screenshot 2023-02-21 at 12.40.11 PM.png | 46.02 KB | Gauravvvv |
Issue fork drupal-3332441
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 #5
Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commentedremoved some RTL specific styling. Added marign-inline to avoid RTL. Improved some nesting. Please review
Comment #6
smustgrave CreditAttribution: smustgrave at Mobomo commentedSince the .css file did change can we get before/after screenshots please.
Comment #7
Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commentedAdded before and after patch screenshot, please review
Before patch
After patch
Comment #8
smustgrave CreditAttribution: smustgrave at Mobomo commentedThanks
Comment #9
lauriiiAdded comment to the MR
Comment #10
Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commentedAddressed comment in MR.
Comment #11
smustgrave CreditAttribution: smustgrave at Mobomo commentedMore nesting was added and didn't seem to break anything.
Comment #12
nod_Need to remove @nest
Comment #14
Gauravvvv CreditAttribution: Gauravvvv at Axelerant for Drupal India Association commentedRestoring status.
@nest is needed here because the ampersand & comes after the rule. If it becomes before the rule, then it's not needed
Comment #15
nod_Comment #17
nod_Patch fixes some spacing in RTL, nice.
Committed 99c88f9 and pushed to 10.1.x. Thanks!