Skip to content

Conversation

@fehmer
Copy link
Member

@fehmer fehmer commented Jan 14, 2026

  • move all colors to constants/themes
  • remove now empty css files
  • add Theme component to apply css variables and load additional css files if needed
  • rework related code

@monkeytypegeorge monkeytypegeorge added frontend User interface or web stuff assets Languages, themes, layouts, etc. labels Jan 14, 2026
@fehmer fehmer force-pushed the feature/remove-theme-color-duplicates branch from fb1b6f4 to 1bbea5d Compare January 15, 2026 13:10
@Miodec
Copy link
Member

Miodec commented Jan 15, 2026

@copilot only serika dark, darling and dots are converted on this pr. convert the rest

Copy link
Contributor

Copilot AI commented Jan 15, 2026

@Miodec I've opened a new pull request, #7371, to work on those changes. Once the pull request is ready, I'll request review from you.

@fehmer fehmer force-pushed the feature/remove-theme-color-duplicates branch from 2ec3e67 to c4bd55f Compare January 15, 2026 18:21
@fehmer fehmer marked this pull request as ready for review January 15, 2026 18:27
@github-actions github-actions bot added the waiting for review Pull requests that require a review before continuing label Jan 15, 2026
@Miodec Miodec requested a review from Copilot January 15, 2026 18:39
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors theme handling by consolidating all color definitions from individual CSS files into a centralized TypeScript constants file. It removes now-empty CSS files, introduces a new Theme SolidJS component to dynamically apply CSS variables, and updates all related code to work with the new architecture.

Changes:

  • Consolidated all theme color definitions into frontend/src/ts/constants/themes.ts
  • Removed duplicate color CSS variable definitions from individual theme CSS files
  • Created new Theme SolidJS component to dynamically apply CSS variables and load theme-specific CSS files
  • Updated theme-related code throughout the application to use the new centralized theme colors

Reviewed changes

Copilot reviewed 215 out of 216 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
frontend/static/themes/*.css Removed :root CSS variable blocks from all theme files (keeping only additional CSS rules)
frontend/src/ts/constants/themes.ts Would need to verify this file contains all theme definitions
frontend/src/ts/signals/theme.ts Updated to export Theme type and removed ThemeColors duplicate definition
frontend/src/ts/components/layout/Theme.tsx New SolidJS component for managing theme CSS variables and loading theme-specific CSS
frontend/src/ts/elements/theme-colors.ts Deleted file (functionality moved to signals)
frontend/src/ts/controllers/theme-controller.ts Refactored to work with centralized theme definitions
frontend/src/ts/elements/settings/theme-picker.ts Updated to use new theme color management approach
frontend/src/html/pages/settings.html Updated color picker IDs to use data-key attributes
frontend/src/index.html Removed hardcoded theme link
frontend/src/html/head.html Added Theme component mount point
Multiple TS files Updated imports and function calls to use new theme color getters
Comments suppressed due to low confidence (1)

frontend/src/html/pages/settings.html:1606

  • Inconsistent ID naming. This input has id='--sub-color' but should be id='colorPickerSub' to match the naming pattern used by all other color picker inputs in the same settings panel.
                  id="--sub-color"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

end: ")",
},
{
rule: /\b[A-Z, a-z, 0-9]{6}\b/,
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Character ',' is repeated in the same character class.

Suggested change
rule: /\b[A-Z, a-z, 0-9]{6}\b/,
rule: /\b[A-Za-z0-9]{6}\b/,

Copilot uses AI. Check for mistakes.
end: ")",
},
{
rule: /\b[A-Z, a-z, 0-9]{6}\b/,
Copy link

Copilot AI Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Character ' ' is repeated in the same character class.

Suggested change
rule: /\b[A-Z, a-z, 0-9]{6}\b/,
rule: /\b[A-Za-z0-9]{6}\b/,

Copilot uses AI. Check for mistakes.
@monkeytypegeorge monkeytypegeorge added the docs Related to Markdown files and documentation label Jan 15, 2026
@monkeytypegeorge monkeytypegeorge added the packages Changes in local packages label Jan 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

assets Languages, themes, layouts, etc. docs Related to Markdown files and documentation frontend User interface or web stuff packages Changes in local packages waiting for review Pull requests that require a review before continuing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants