Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces CSS custom properties (CSS variables) for theming support in the folder pane, establishing a foundation for the solid-ui themes system. The changes replace hardcoded CSS values with theme-aware CSS variables that include appropriate fallback values.
Changes:
- Replaced hardcoded CSS values with CSS custom properties (variables) following the
--sui-*naming convention - Updated peer dependency from
solid-ui@^3.0.0tosolid-ui@^3.0.1-11be53bto support the new theme variables
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/folderPane.ts | Introduces CSS variables for spacing, colors, and sizing properties with fallback values to maintain backward compatibility |
| package.json | Updates solid-ui peer dependency to pre-release version that includes theme variable support |
| package-lock.json | Updates lockfile entries to reflect the new solid-ui and solid-logic peer dependency versions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const tr = outliner.propertyTR(dom, st, false) | ||
| tr.firstChild.textContent = '' // Was initialized to 'Contains' | ||
| tr.firstChild.style.cssText += 'min-width: 3em;' | ||
| tr.firstChild.style.cssText += 'min-width: var(--sui-space-2xl, 3em);' |
There was a problem hiding this comment.
Using a spacing CSS variable (--sui-space-2xl) for a width property is semantically inconsistent. The min-width property defines a minimum dimension/size, not spacing. Consider using a size-specific variable like --sui-size-2xl or --sui-width-sm instead to maintain clear semantic separation between spacing (margins, padding, gaps) and sizing (width, height) properties in the theme system.
| tr.firstChild.style.cssText += 'min-width: var(--sui-space-2xl, 3em);' | |
| tr.firstChild.style.cssText += 'min-width: var(--sui-size-2xl, 3em);' |
No description provided.