-
Notifications
You must be signed in to change notification settings - Fork 31
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: allow themes to use any srgb color for definitions #1756
Conversation
Themes were previously limited to HSL raw value colors such that transparency could be created using HSLA. This made theme creation somewhat annoying and limiting. This change adopts color-mix (requires chrome 111, firefox 113 -- both older than 6 months) to create any necessary transparency. - replaces hsl raw values with hex (allows skipping color normalization) - replace hsla usage with color-mix - fix miss-named legend-color - created utils that use color-mix for easier specification - move utils.scss to be auto-imported with custom.scss - re-wrote color normilzation and resolving to resolve color-mix based colors - changed iris grid theme context to not accept a paritial Breaking change: IrisGridThemeContext no longer accepts a paritial theme. By guaranteeing the provider is a full theme we can resolve the CSS variables and normailze the colors only once per theme load globally, rather than having to do it once per grid.
): T { | ||
const perfStart = performance.now(); | ||
|
||
// Add a temporary div to attach temp css variables to | ||
const tmpPropEl = document.createElement('div'); | ||
targetElement.appendChild(tmpPropEl); | ||
|
||
const varExpressions = [...extractDistinctCssVariableExpressions(record)]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still able to resolve strings containing multiple expressions?
var(--bbb1) var(--bbb2)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
const el = document.createElement('div');
el.style.setProperty("--a", "#aaa");
el.style.setProperty("--b", "#bbb");
el.style.setProperty("--c", "var(--a) var(--b)");
document.body.appendChild(el);
window.getComputedStyle(el).getPropertyValue("--c");
// '#aaa #bbb'
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1756 +/- ##
==========================================
+ Coverage 46.03% 46.06% +0.03%
==========================================
Files 626 626
Lines 37576 37593 +17
Branches 9447 9458 +11
==========================================
+ Hits 17298 17318 +20
+ Misses 20223 20220 -3
Partials 55 55
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
…e, it used to be 0.09 in the old theme)
tests/styleguide.spec.ts-snapshots/inputs-focus-col3-row6-text-firefox-linux.png
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. Verified that local build + preview is working as well
Themes were previously limited to HSL raw value colors such that transparency could be created using HSLA. This made theme creation somewhat annoying and limiting. This change adopts color-mix (requires chrome 111, firefox 113 -- both older than 6 months) to create any necessary transparency.
Style changes:
BREAKING CHANGE: