Skip to content

Conversation

@ainsleyclark
Copy link
Contributor

Summary

This PR standardizes CSS variable naming conventions across the sveltekit-helper package to improve consistency and maintainability. Variable names now follow a uniform pattern using colour (British English spelling) and shortened property names like bg instead of background.

Key Changes

  • Sidebar component: Renamed --sidebar-background to --sidebar-bg and --sidebar-toggle-background to --sidebar-toggle-bg
  • PayloadForm component: Standardized form-related variables:
    • --form-input-text--form-input-colour
    • --form-error-color--form-error-colour
    • --form-success-color--form-success-colour
    • --form-button-text--form-button-colour
  • Documentation: Updated README.md with new variable names and inline style examples
  • Component code: Updated all CSS variable references in component styles and JSDoc comments

Implementation Details

  • All changes maintain backward compatibility in terms of functionality
  • Variable naming now consistently uses British English spelling (colour instead of color)
  • Abbreviated property names (bg instead of background) reduce verbosity while remaining clear
  • Updated both component implementations and documentation to reflect the new naming scheme

Standardized CSS custom property naming across all components:
- Changed 'background' to 'bg' (--sidebar-background → --sidebar-bg)
- Changed American 'color' to British 'colour' (--form-error-color → --form-error-colour)
- Changed generic 'text' to 'colour' (--form-input-text → --form-input-colour)

Updated files:
- Sidebar.svelte: background → bg
- PayloadForm.svelte: color → colour, text → colour
- README.md: Updated documentation examples

This ensures consistent naming patterns across the codebase:
- Use 'bg' for all background variables
- Use 'colour' (British spelling) for all color variables
@claude
Copy link

claude bot commented Jan 22, 2026

Review summary

  • Overall score: 9/10
  • Critical issues: 0
  • Warnings: 1
  • Suggestions: 2
  • Recommendation: ✅ Approve

This PR successfully standardises CSS variable naming conventions across the sveltekit-helper package, using consistent British English spelling and shortened property names. The changes are well-documented and maintain consistency with the project's AGENTS.md guidelines.

Critical issues 🔴

None

Warnings 🟡

Breaking change without migration guide

The CSS variable renamings are breaking changes for any consumers currently using these variables. While the PR body mentions "backward compatibility in terms of functionality", the variable names themselves are not backward compatible.

Impact: Any projects using the old variable names (--sidebar-background, --form-input-text, etc.) will experience style breakage after updating.

Recommendation: Consider one of the following:

  • Add a migration section to the README.md explaining the variable name changes
  • Bump the package version appropriately (major version bump for breaking changes)
  • Temporarily support both old and new variable names using CSS fallbacks:
    background-color: var(--sidebar-bg, var(--sidebar-background, var(--colour-base-black)));

Files affected:

  • packages/sveltekit-helper/src/components/Sidebar.svelte:194
  • packages/sveltekit-helper/src/components/Sidebar.svelte:213
  • packages/sveltekit-helper/src/components/Sidebar.svelte:264
  • packages/sveltekit-helper/src/components/payload/PayloadForm.svelte:205-280

Suggestions 🟢

Consider documenting the naming convention pattern

The README.md now demonstrates the new pattern (bg for background, colour spelling), but doesn't explicitly document the naming convention rules. This would help future contributors maintain consistency.

Example addition to README.md:

### CSS variable naming conventions
- Use British English spelling (`colour` not `color`)
- Use abbreviated property names where clear (`bg` for `background`)
- Follow kebab-case (`--component-property-modifier`)

Verify JSDoc comments match implementation

In Sidebar.svelte:141-151, the JSDoc comments correctly document the new variable names. Consider doing a codebase-wide search to ensure no other documentation or examples reference the old variable names.

Command to check:

rg "(sidebar-background|sidebar-toggle-background|form-input-text|form-error-color|form-success-color|form-button-text)" --type md --type ts --type svelte

@codecov
Copy link

codecov bot commented Jan 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.80%. Comparing base (7f6b060) to head (887a2bc).
⚠️ Report is 464 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #382      +/-   ##
==========================================
+ Coverage   64.59%   69.80%   +5.21%     
==========================================
  Files         154      185      +31     
  Lines        6064     7359    +1295     
==========================================
+ Hits         3917     5137    +1220     
+ Misses       2064     2025      -39     
- Partials       83      197     +114     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ainsleyclark ainsleyclark merged commit d0de682 into main Jan 22, 2026
6 checks passed
@ainsleyclark ainsleyclark deleted the claude/fix-css-var-naming-ZY3ii branch January 22, 2026 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants