Skip to content

DRAFT 830-refactor: Move style params to scss variables #891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

YuliaDemir
Copy link
Collaborator

What type of PR is this? (select all that apply)

  • ๐Ÿ• Feature
  • ๐Ÿ› Bug Fix
  • ๐Ÿšง Breaking Change
  • ๐Ÿง‘โ€๐Ÿ’ป Code Refactor
  • ๐Ÿ“ Documentation Update

Description

  • add consts for border-radius
  • add consts for font-size
  • add consts for gap

Related Tickets & Documents

Screenshots, Recordings

Please replace this line with any relevant images for UI changes.

Added/updated tests?

  • ๐Ÿ‘Œ Yes
  • ๐Ÿ™…โ€โ™‚๏ธ No, because they aren't needed
  • ๐Ÿ™‹โ€โ™‚๏ธ No, because I need help

[optional] Are there any post deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

@github-actions github-actions bot changed the title 830-refactor: move style params to scss variables 830-refactor: Move style params to scss variables May 13, 2025
@YuliaDemir YuliaDemir changed the title 830-refactor: Move style params to scss variables DRAFT 830-refactor: Move style params to scss variables May 13, 2025
@SpaNb4
Copy link
Collaborator

SpaNb4 commented May 15, 2025

run visual now

@SpaNb4
Copy link
Collaborator

SpaNb4 commented May 15, 2025

@coderabbitai, please create an issue about enforcing variable usage in SCSS, for example using https://github.com/AndyOGo/stylelint-declaration-strict-value

Copy link
Contributor

coderabbitai bot commented May 15, 2025

Tip

For best results, initiate chat on the files or code changes.

๐Ÿงฉ Analysis chain

๐Ÿ Script executed:

#!/bin/bash
# Check if the repository has any Stylelint configuration files
echo "Checking for Stylelint configuration files..."
fd -e json -e js -e yaml -e yml stylelint

# Check for any package.json files to see if stylelint is already a dependency
echo "Checking package.json for stylelint dependencies..."
cat package.json 2>/dev/null | grep -i stylelint || echo "No stylelint in root package.json or file not found"

# Look for SCSS files to confirm they're used in the project
echo "Checking for SCSS files in the project..."
fd -e scss | head -5

# Check if there are any linting rules or mentions of variable usage enforcement in documentation
echo "Checking for linting documentation or guidelines..."
fd -t f -e md -e txt | xargs grep -l "lint" 2>/dev/null | head -5

Length of output: 1157


๐Ÿ Script executed:

#!/bin/bash
# Check for any existing stylelint configuration files
echo "Looking for stylelint configuration files..."
fd "\.stylelintrc" || fd ".stylelint.config" || echo "No stylelint config files found with common names"

# Check for any SCSS variable files to understand current variable structure
echo "Looking for SCSS variable files..."
fd -e scss | grep -i "var\|constants\|colors\|theme" | head -5

# Let's examine the first few lines of a constants file to see how variables are defined
echo "Examining SCSS variables..."
cat src/core/styles/_constants.scss | head -15

Length of output: 867


๐Ÿ Script executed:

#!/bin/bash
# Create a new GitHub issue about enforcing SCSS variable usage
gh issue create --title "Enforce SCSS variable usage with stylelint-declaration-strict-value" --body "## Overview
Currently, the project uses Stylelint for SCSS linting (as seen in package.json), but we should enhance the configuration to enforce the use of variables for specific CSS properties instead of hardcoded values.

## Recommendation
Integrate [stylelint-declaration-strict-value](https://github.com/AndyOGo/stylelint-declaration-strict-value) plugin to enforce the use of variables, functions, or calculations for properties like:
- Colors
- Font sizes
- Font weights 
- Z-indices
- Spacing values (margins, paddings)

## Benefits
- Ensures consistency across the entire codebase
- Makes global styling changes easier to manage
- Prevents hardcoded values that might not follow the design system
- Improves maintainability and prevents visual regressions

## Implementation
1. Install the plugin:
\`\`\`bash
npm install --save-dev stylelint-declaration-strict-value
\`\`\`

2. Add to stylelint configuration:
\`\`\`json
{
  \"plugins\": [\"stylelint-declaration-strict-value\"],
  \"rules\": {
    \"scale-unlimited/declaration-strict-value\": [
      [\"color\", \"z-index\", \"font-size\", \"font-weight\"],
      {
        \"ignoreValues\": [\"transparent\", \"inherit\", \"initial\", \"currentColor\", \"0\"]
      }
    ]
  }
}
\`\`\`

3. Gradually update existing code to use variables where hardcoded values are currently used.

## Related PR
Requested by @SpaNb4 in https://github.com/rolling-scopes/site/pull/891
"

Length of output: 1546


@SpaNb4 I've created a GitHub issue about enforcing variable usage in SCSS using the stylelint-declaration-strict-value plugin as requested. The issue is available at: #895

The issue includes:

  • Overview of the current setup
  • Recommendation to use the plugin
  • Benefits of enforcing SCSS variables
  • Implementation steps with example configuration
  • Reference back to this PR

@YuliaDemir
Copy link
Collaborator Author

I can't view the 19 visual changes that require verification. percy returns an error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move style params from code to scss-variables
3 participants