Add hierarchical color palettes and theme-aware assets#121
Conversation
Sensitive Files DetectedThis is an automated notice. A maintainer will review after import. |
🎉 Bazel & CI Test Results
All Bazel configuration and CI tests passed! ✨ The build system and core tooling are working correctly. 🚀 Bazel remote cache is now enabled - future builds will be faster! Workflow: Valdi CI |
|
Thanks for the contribution! While bringing this into Snapchat's build a few issues came up in review that are worth addressing here. I've verified each against the code and grouped them by confidence. References are by function name. Confirmed functional bugs1.
Result<Value> postprocessBoxShadow(ViewNode& viewNode, const Value& in) {
if (!in.isArray()) {
return in;
}
constexpr size_t kBoxShadowColorIndex = 4;
auto resolvedBoxShadow = resolveColorAtIndexInArray(viewNode, in, kBoxShadowColorIndex);
...
}2. The new path parses each value with Thread-safety (please sanity-check against Valdi's threading model, you know it best)The multi-palette design introduces cross-thread access patterns that the single fixed palette did not have. Writes are dispatched to the main thread (
If Valdi guarantees these accesses are serialized onto one thread, this is a non-issue and can be dismissed. If not, the fix is a mutex in Defensive / robustness (lower urgency)3. The ctor does 4. In the non-empty-name branch, if (_viewNodeTree != nullptr) {
if (const auto& viewManagerContext = _viewNodeTree->getViewManagerContext()) {
colorPalette =
viewManagerContext->getAttributesManager().getColorPaletteManager()->getColorPalette(colorPaletteName);
}
}
setHasOveriddenColorPalette(true);5. The child->setInheritedColorPalette(viewTransactionScope, _colorPalette);(This is largely theoretical since in-tree nodes normally resolve to the non-null active palette, but the invariant is cleaner without the guard.) |
Description
This change adds support for overriding colorPalette on a per node level. This allows to turn a subtree into light or dark mode, regardless of the system configuration. In addition, it adds support for themeable assets, which makes it possible for an asset to have a different variant depending on the resolved color palette. One can use this to make a dark and light asset, similar to how we can make an asset that has a direction version for LTR and RTL.
Type of Change
Testing
bazel test //...)Testing Details
Checklist
Related Issues
Additional Context