Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Improve changing palette and extended palette #121

Closed
AlexvZyl opened this issue Mar 23, 2024 · 4 comments · Fixed by #144
Closed

Feat: Improve changing palette and extended palette #121

AlexvZyl opened this issue Mar 23, 2024 · 4 comments · Fixed by #144
Assignees
Labels
Enhancement New feature, request or suggestion Fixed on Dev This issue will be fixed in the next release

Comments

@AlexvZyl
Copy link
Owner

We need a better way to reliably override the palette and extended palette.

We should take inspiration from how tokyonight does it:

https://github.com/folke/tokyonight.nvim/blob/fbe3a27378fdd51a8ddd04f57012455436916a62/lua/tokyonight/theme.lua#L873
https://github.com/folke/tokyonight.nvim/blob/fbe3a27378fdd51a8ddd04f57012455436916a62/lua/tokyonight/colors.lua#L158

@AlexvZyl AlexvZyl added the Enhancement New feature, request or suggestion label Mar 23, 2024
@5-pebbles
Copy link
Collaborator

Questions:

  1. what should be the functions be named? I will go with on_palette & on_highlight for now.

  2. should the arguments passed to on_palette & on_highlight be stateful? So should changes made in a first call to on_palette be reflected in the arguments passed to a second call?

So I will start implmenting:

-- This callback can be used to override the colors used in the palette.
    on_palette = function(palette) return palette end,
-- This callback can be used to override highlights before they are applied.
    on_highlight = function(highlights, palette) return highlights end,

@5-pebbles
Copy link
Collaborator

I think I am going to have to think on this one for a few days.

override allows you to set any highlights like tokyonight's on_highlights... but it would be nice if users could set colors like bg which are used internally.

What about using more highlight linking? So make a highlight group for bg and then all backgrounds link to that?
Then in override / on_highlight(or whatever I call it) you could change the Background, Visual, etc... highlight and those changes would be reflected everywhere.

@5-pebbles
Copy link
Collaborator

This is my best idea so far: 26f9c4a

@5-pebbles
Copy link
Collaborator

5-pebbles commented Mar 25, 2024

I think this is the best solution changing_palette

using the base palette works the same as normal, and you can set values in the extended palette just as you would the base palette.

You can even use values in the extended palette but they will be nil for the fist call:

#100 can be solved with 3 lines:

            on_palette = function(palette)
                -- the if is needed because on_palette is called twice and one time palette.bg is nil which breaks blend
                if palette.bg ~= nil then
                    palette.bg_visual = require("nordic.utils").blend(palette.orange.base, palette.bg, 0.15)
                end
                return palette
            end,

25-03-24@19:33:46

 

I am not sure about on_highlight but I think it is better for the future (because the plugin provides the palette).

I will re add support for override + add a deprecated warning on launch... then I will open a PR.

@AlexvZyl AlexvZyl mentioned this issue Jun 16, 2024
@5-pebbles 5-pebbles added the Fixed on Dev This issue will be fixed in the next release label Sep 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature, request or suggestion Fixed on Dev This issue will be fixed in the next release
Projects
None yet
2 participants