feat: add catppuccin theme support#59
Conversation
textfuel
left a comment
textfuel
left a comment
There was a problem hiding this comment.
Thanks for the PR! Huge refactor on the color system - really appreciate that. I have left a few comments to discuss
There was a problem hiding this comment.
There is a duplicated palette in authorcolor.go. Could we pick one source of truth between it and your theme.go? I would prefer keeping yours and dropping authorcolor.go
| func SetTheme(name string) error { | ||
| switch name { | ||
| case "", "default": | ||
| Default = defaultTheme() | ||
| case "catppuccin-latte": | ||
| Default = catppuccinLatte() | ||
| case "catppuccin-frappe": | ||
| Default = catppuccinFrappe() | ||
| case "catppuccin-macchiato": | ||
| Default = catppuccinMacchiato() | ||
| case "catppuccin-mocha": | ||
| Default = catppuccinMocha() | ||
| default: | ||
| return fmt.Errorf("unknown theme: %q", name) | ||
| } |
There was a problem hiding this comment.
Consider using https://github.com/catppuccin/go so we'd have all the Catppuccin flavors and no hardcoded colors.
Bonus: the lib also exposes contrast helpers, which solves the Latte readability issue (white-on-light-gray on selected items)
Not blocking
| ) | ||
| // These are functions so they pick up the active theme's colors. | ||
| func jqlFieldStyle() lipgloss.Style { return lipgloss.NewStyle().Foreground(theme.ColorBlue) } | ||
| func jqlKeywordStyle() lipgloss.Style { return lipgloss.NewStyle().Foreground(lipgloss.Color("5")) } |
There was a problem hiding this comment.
Looks like a few colors are missing from the palette.
Migrating to the catppuccin/go lib would cover this automatically.
Was there a reason for skipping them, or just oversight?
There was a problem hiding this comment.
A few package-level styles are evaluated at init time, before SetTheme runs, so they capture the default ANSI colors and never pick up the active theme:
- pkg/tui/components/createform.go:1098 — noneStyle
- pkg/tui/views/infofields.go:255 — noneStyle
- pkg/tui/views/detail.go:663 — urlStyle
Same root cause as the var to func conversion you already did in jqlhighlight.go. Could we apply the same fix here?
func noneStyle() lipgloss.Style {
return lipgloss.NewStyle().Foreground(theme.ColorGray)
}
|
Thanks for feedback! |
|
Addressed comments. |
these needed to be re-generated every time go.mod changed i'll add it into Contributing.md
textfuel
left a comment
textfuel
left a comment
There was a problem hiding this comment.
Looks awesome! One last minor comment and I'll release this!
Co-authored-by: textfuel <[email protected]>
|
Applied suggestion. |
2 participants
What
Add catppuccin (https://github.com/catppuccin) themes support.
Why
I'd like to have themes support so I can use my favourite theme.
How to test
Set gui.theme to one of supported catppuccin themes or
default.Closes #58