feat: introduce theme helpers#226
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #226 +/- ##
=======================================
Coverage 99.00% 99.00%
=======================================
Files 44 44
Lines 1304 1307 +3
Branches 338 338
=======================================
+ Hits 1291 1294 +3
Misses 13 13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
|
||
| function applyThemeClassName(theme: Theme) { | ||
| if (typeof document !== 'undefined' && !hasThemeClassName(theme) && themeDefinitions[theme].isFlagActive()) { | ||
| document.body.classList.add(themeDefinitions[theme].className); |
There was a problem hiding this comment.
What if multiple flags are active?
There was a problem hiding this comment.
That's ok, it will just add the class to the body no matter if there is another flag present.
There was a problem hiding this comment.
If components use both classnames, the winning styles depend on stylesheet source order? It is implicit. What about having a precedence list, add only the highest-priority one?
There was a problem hiding this comment.
I’m not sure that’s ideal, as it could hide the fact that other services are having multiple flags enabled instead of cleaning them up.
That said, if you feel strongly about it, I’m happy to add it, it’s a small change.
There was a problem hiding this comment.
Maybe another solution is having that priority list, but show a dev warning if multiple flags are enabled.
There was a problem hiding this comment.
I think themes are mutually exclusive. Rather than silently applying 2 themes and rely on CSS order, I prefer to apply priority list, also console.warn if 2 themes are active.
There was a problem hiding this comment.
If we allow both classes to coexist, we'd need tests ensuring the CSS precedence holds.
2 participants
When the One Theme global flag is set, the toolkit now adds the
awsui-one-themeclass to<body>.Changes made:
themeDefinitionsregistry. Adding a future theme is now a single registry entry.initThemes()that applies the body class for every flag-activatedtheme. In the future, this can replace `useRuntimeVisualRefresh().
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.