DDEV is the official local development tool of Drupal. And like Drupal, DDEV depends on the support of the open source community.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 |
|---|---|---|---|
| #14 | 3332462-14-after.png | 51.92 KB | starshaped |
| #14 | 3332462-14-before.png | 50.95 KB | starshaped |
| #6 | Screenshot 2023-02-22 at 11.44.48 AM.png | 127.65 KB | gauravvvv |
| #6 | Screenshot 2023-02-22 at 11.51.15 AM.png | 128.55 KB | gauravvvv |
Issue fork drupal-3332462
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:
- 3332462-refactor-claros-system-status-counter
changes, plain diff MR !3509
Comments
Comment #2
starshapedActively working on this at Florida Drupal Camp 2023.
Comment #3
starshapedComment #4
gauravvvv CreditAttribution: gauravvvv at Axelerant for Drupal India Association commentedComment #6
gauravvvv CreditAttribution: gauravvvv at Axelerant for Drupal India Association commentedHere are the before/after patch screenshots. please review
Before patch:
After patch:
Comment #7
smustgrave CreditAttribution: smustgrave at Mobomo commentedThanks for the screenshots
Appears there are failures in the MR.
Comment #8
starshapedI'm going to be working on this tonight and tomorrow to further clean up the CSS and fix the MR failures.
Comment #9
smustgrave CreditAttribution: smustgrave at Mobomo commentedSounds good! But needs review isn’t the right status if there’s work to still be done.
Comment #10
starshapedYeah, that was a mistake on my end, I didn't realize I had changed the status! I'll set it to needs review once I get my work done.
Comment #11
smustgrave CreditAttribution: smustgrave at Mobomo commentedWill keep an eye out to re-review
Comment #12
starshapedCode has been cleaned up and tests are passing. Ready for review!
Comment #13
smustgrave CreditAttribution: smustgrave at Mobomo commentedSince you made additional changes since the last set of screenshots. Think we are going to need new ones since the .css file has changed.
Comment #14
starshapedReadded the RTL styles as they only apply on an RTL layout, and added before and after screenshots.
Comment #15
smustgrave CreditAttribution: smustgrave at Mobomo commentedFrom what I can see everything has been covered with the nesting, variables, etc.
Comment #18
lauriiiCommitted 138349b and pushed to 10.1.x. Thanks!
Discovered some pre-existing inconsistencies with the RTL styles. Opened a follow-up to address those #3344765: Inconsistencies in system-status-counter RTL styles.