Skip to content

Improvement: Value colour scheme improvements#5579

Open
georgemac-labs wants to merge 3 commits intoportfolio-performance:masterfrom
georgemac-labs:improvement/color-value-scheme-improvements
Open

Improvement: Value colour scheme improvements#5579
georgemac-labs wants to merge 3 commits intoportfolio-performance:masterfrom
georgemac-labs:improvement/color-value-scheme-improvements

Conversation

@georgemac-labs
Copy link
Contributor

@georgemac-labs georgemac-labs commented Mar 19, 2026

Like approx 1 in 25 people worldwide, I have (minor) issues with shades of red and green!

The standard value colour scheme in PP is not good for me. The tones are dull, colours are recognisable if I look carefully, but you don't get the intended UI benefit of recognising them at a glance. And I am a mild case: many people would not be able to recognise the coloration at all!

Thus this PR:

  1. Adds a "vivid" value colour scheme
  2. Improves the settings page preview:
    • Value colours are now previewed against the actual bg colour used in app views
    • ... based on the theme selected in the dropdown
    • ... and a bug is fixed where the preview texts were not coloured in Dark mode (they were just white or grey)
  3. For all views, applies update colours immediately when apply is clicked (at least Trades previously required a view switch)

TBD:

  • Is this the right fix? Are the current standard colours actually a good choice as standard? To me, they seem dull compared to other apps. But it's hard for me to judge as I have non-standard vision!
  • I am doing this as an accessibility amateur based on my own experience. Open to other opinions about what the exact colour values should be.

- Use ColoredLabel with setTextColor/setBackdropColor instead of plain
  Label with setForeground, which was being overridden by the dark mode
  CSS theme engine
- Set DISABLE_CSS_STYLING on all preview widgets to prevent the theme
  from clobbering manually set colors
- Compute preview background based on the selected theme (not the
  currently active one) so the preview accurately reflects how colors
  will appear after applying
@georgemac-labs georgemac-labs changed the title Improvement: Color value scheme improvements Improvement: Value colour scheme improvements Mar 19, 2026
@buchen
Copy link
Member

buchen commented Mar 21, 2026

Value colours are now previewed against the actual bg colour used in app views

Yes, but... The text colors differ between dark and light mode. Now the preview is changing the background, but still uses the text color from the current theme. I think this is not given the right impression. The "blue text color" of the light theme does not work with the dark background color.

Unfortunately, the text color is not directly available because it is injected into the ValueColorScheme when the theme is chanted. But maybe there are ways to read the color values and apply them to the text widget? Otherwise I think it is okay to see the preview only for the current theme (light/dark) because this is what the user is using.

I am doing this as an accessibility amateur based on my own experience. Open to other opinions about what the exact colour values should be.

Me neither. For me, the bright green in the vivid light color scheme is hard to read.
But I also think that the green of the standard theme is a little bit too dark.

I am also okay to add another 4th scheme. I would need very good arguments to add a fifth.

@buchen
Copy link
Member

buchen commented Mar 21, 2026

Claude says there is no easy way to get the CSS colors of the text. It recommends parsing the CSS. I am not sure it is worth the effort (and future maintenance).

  1. Parse the CSS files directly (recommended)

The preference page already reads CSS files (for font size in readFontSizeFromCSS()). The ValueColorScheme CSS > format is simple and stable:

ValueColorScheme.standard {
positive-foreground: #02f500;
negative-foreground: #fe2b05;
}

You could add a small method that reads dark.css / light.css and extracts the foreground colors per scheme identifier >using a regex, similar to the
existing font-size parsing. Then createSchemeOption would use the current scheme's colors OR the alternate-theme >colors depending on what
getSelectedTheme() returns. The CSS files are bundled in the plugin (css/shared/dark.css, css/shared/light.css), so > they're always available.

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.

2 participants