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

Refactor design tokens to use a proper color table #8322

Merged
merged 9 commits into from
Dec 6, 2024

Conversation

abey79
Copy link
Member

@abey79 abey79 commented Dec 5, 2024

Related

What

This PR refactors the design token as follows:

  • All colors from design_tokens.json are now loaded in a big table at startup.
  • Colors are now referred to using the new the ColorToken, which is basically an index into this table.
  • Removed all of the color aliases stuff. This was only partially used and very cumbersome to update. The design_token.rs file is the de facto source of truth of semantic aliasing for colors.

@abey79 abey79 added ui concerns graphical user interface 🚜 refactor Change the code, not the functionality exclude from changelog PRs with this won't show up in CHANGELOG.md labels Dec 5, 2024
Copy link

github-actions bot commented Dec 5, 2024

Web viewer built successfully. If applicable, you should also test it:

  • I have tested the web viewer
Result Commit Link
a699c83 https://rerun.io/viewer/pr/8322

Note: This comment is updated whenever you push a commit.


impl ColorToken {
#[inline]
pub fn new(hue: Hue, shade: Shade) -> Self {
Copy link
Member Author

Choose a reason for hiding this comment

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

With enumn (which we already depend on) I could easy add support for u16 input, but then we must deal with error management. For that, we could either:

  • panic
  • have an invalid state for ColorToken (e.g. shade = None), which would return a random color when queried (so it flashes in the UI) and logs an error

Copy link
Member

@emilk emilk left a comment

Choose a reason for hiding this comment

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

Very nice to finally get this!

crates/viewer/re_ui/src/color_table.rs Outdated Show resolved Hide resolved
crates/viewer/re_ui/src/color_table.rs Outdated Show resolved Hide resolved
crates/viewer/re_ui/src/color_table.rs Outdated Show resolved Hide resolved
crates/viewer/re_ui/src/design_tokens.rs Outdated Show resolved Hide resolved
@abey79 abey79 merged commit c2304a0 into main Dec 6, 2024
31 checks passed
@abey79 abey79 deleted the antoine/color-table-and-token branch December 6, 2024 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md 🚜 refactor Change the code, not the functionality ui concerns graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants