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

Add configurable colored systems #16

Merged

Conversation

doup
Copy link
Contributor

@doup doup commented Mar 3, 2023

  • Add Settings.system_style option, which allows to pass a System => Style mapper to decide how to style a single system node.
  • If None is given (default behavior) a predefined set of colors are applied

Fixes #13

Examples

colored-light

colored-dark

Current color palette

I've it on a Figma file, I can create a standalone file and share it if you want.

image

@doup doup marked this pull request as ready for review March 3, 2023 20:09
Copy link
Owner

@jakobhellermann jakobhellermann left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this! I agree that this looks a lot better and more readable. I left a few comments the PR looks mostly good to go.

src/schedule_graph/settings.rs Outdated Show resolved Hide resolved
src/schedule_graph/settings.rs Outdated Show resolved Hide resolved
src/schedule_graph/system_style.rs Outdated Show resolved Hide resolved
src/schedule_graph/system_style.rs Outdated Show resolved Hide resolved
src/schedule_graph/settings.rs Outdated Show resolved Hide resolved
let is_dark = l < 0.6;

// Calculate text color based on bg
let text_color = style
Copy link
Owner

Choose a reason for hiding this comment

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

This looks mostly fine but there are a few instances in which the contrast seems a little low:
image
Do you have an idea on how to fix this? Maybe just white or black text depending on the lightness?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed the values for light/dark text (see 5060fad). I'm not going full white/black to preserve some of the hue from the background. This should improve readability a little bit while still preserving the tint.

Although, the real issue is to find a good way to decide if I should use white or black text. Right now I'm doing it based on the background luminance (is_dark = luminance < 0.6); which is probably a naive approach, see: https://ux.stackexchange.com/q/107318

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For reference, this is how it looks like after the change:
image

doup and others added 7 commits March 6, 2023 18:14
Co-Authored-By: Jakob Hellermann <jakob.hellermann@protonmail.com>
Co-Authored-By: Jakob Hellermann <jakob.hellermann@protonmail.com>
Co-Authored-By: Jakob Hellermann <jakob.hellermann@protonmail.com>
@jakobhellermann jakobhellermann merged commit 731c470 into jakobhellermann:bevy-main Mar 6, 2023
@doup doup deleted the feature/colored-systems branch March 7, 2023 09:53
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