search

setzer22 / egui_node_graph

Build your node graph applications in Rust, using egui
MIT License
715 stars 134 forks source link

Color Matters (e.g. support light mode) #33

Closed kkngsm closed 2 years ago

kkngsm commented 2 years ago

Now, if change to light mode :

Light mode before modification

This makes the right-click popup text unreadable. So I made the following modifications to accommodate light mode.

Light mode after modification

I would like to hear your opinions on the colors.

This modified branch is kkngsm@fffdaebe8b5807528444056805429d1bb3c594cb

kkngsm commented 2 years ago

I would like to suggest other color-related functions.

setzer22 commented 2 years ago

This is pretty nice! Thanks for taking the time

Personally I like the colors, it feels like it has enough contrast and is similar to the original theme. Could you submit this as a pull request? :smile:

change the color of connections for each data type

This can be implemented in several ways. I think the most flexible option is to pass an immutable reference to the UserState in the data_type_color method here: https://github.com/setzer22/egui_node_graph/blob/c6c517644a6a104dcb52dc17ef80a776ad56b542/egui_node_graph/src/traits.rs#L10-L19 This would only allow customization beyond egui's light / dark theme, for users that have other kinds of needs.

make it possible to specify the background color of title for each node template

I think it's more flexible if we make the titlebar color a property of the node itself, not its template. This allows more custom behavior in case someone needs it (like, I don't know, a color picker node having the same color in the titlebar :thinking:. This would be possible by adding a titlebar_color method here: https://github.com/setzer22/egui_node_graph/blob/c6c517644a6a104dcb52dc17ef80a776ad56b542/egui_node_graph/src/traits.rs#L21-L26

Is this something you'd enjoy working on? I can take a look at it, but since you're already halfway there with the color changes I think it'd make sense if you want to tackle it :)

kkngsm commented 2 years ago

make it possible to specify the background color of title for each node template

I think it's more flexible if we make the titlebar color a property of the node itself, not its template.

Indeed, your idea is great. I am implementing this now.

setzer22 commented 2 years ago

Nice! :smile: Looking forward to it

setzer22 commented 2 years ago

With your latest changes, I think we can now close this. Thanks again! :tada: