pierrejeambrun commented on code in PR #64232:
URL: https://github.com/apache/airflow/pull/64232#discussion_r2996159854
##########
airflow-core/src/airflow/api_fastapi/common/types.py:
##########
@@ -188,19 +188,35 @@ def serialize_model(self) -> str:
return f"oklch({self.lightness} {self.chroma} {self.hue})"
+# Private type aliases for theme token shapes
+_ColorShade = dict[Literal["value"], OklchColor]
+_SHADE_LITERAL = Literal["50", "100", "200", "300", "400", "500", "600",
"700", "800", "900", "950"]
+_ColorScale = dict[_SHADE_LITERAL, _ColorShade]
+
+
+class ThemeColors(BaseModel):
+ """Color tokens for the UI theme. All fields are optional; at least one
must be provided."""
+
+ brand: _ColorScale | None = None
+ gray: _ColorScale | None = None
+ black: _ColorShade | None = None
+ white: _ColorShade | None = None
+
+ @model_validator(mode="after")
+ def check_at_least_one_color(self) -> ThemeColors:
+ if not any([self.brand, self.gray, self.black, self.white]):
+ raise ValueError("At least one color token must be provided:
brand, gray, black, or white")
+ return self
+
+ @model_serializer(mode="wrap")
+ def serialize_model(self, handler: Any) -> dict:
+ return {k: v for k, v in handler(self).items() if v is not None}
+
+
class Theme(BaseModel):
"""JSON to modify Chakra's theme."""
- tokens: dict[
- Literal["colors"],
- dict[
- Literal["brand"],
- dict[
- Literal["50", "100", "200", "300", "400", "500", "600", "700",
"800", "900", "950"],
- dict[Literal["value"], OklchColor],
- ],
- ],
- ]
+ tokens: dict[Literal["colors"], ThemeColors]
Review Comment:
Not in this PR but as a follow up I wold love to have the `tokens` become
optional. In case someone doesn't want to update the themecolors, but want to
provide a custom icon or css. (I don't think it's possible yet)
##########
airflow-core/tests/unit/api_fastapi/common/test_types.py:
##########
@@ -74,3 +74,136 @@ def test_invalid_oklch(self, input_str, error_message):
with pytest.raises(ValidationError) as exc_info:
OklchColor.model_validate(input_str)
assert error_message in str(exc_info.value)
+
+
+# Shared test data for Theme/ThemeColors tests
+_BRAND_SCALE = {
+ "50": {"value": "oklch(0.975 0.007 298.0)"},
+ "100": {"value": "oklch(0.950 0.014 298.0)"},
+ "200": {"value": "oklch(0.900 0.023 298.0)"},
+ "300": {"value": "oklch(0.800 0.030 298.0)"},
+ "400": {"value": "oklch(0.680 0.038 298.0)"},
+ "500": {"value": "oklch(0.560 0.044 298.0)"},
+ "600": {"value": "oklch(0.460 0.048 298.0)"},
+ "700": {"value": "oklch(0.390 0.049 298.0)"},
+ "800": {"value": "oklch(0.328 0.050 298.0)"},
+ "900": {"value": "oklch(0.230 0.043 298.0)"},
+ "950": {"value": "oklch(0.155 0.035 298.0)"},
+}
+_GRAY_SCALE = {
+ "50": {"value": "oklch(0.975 0.002 264.0)"},
+ "100": {"value": "oklch(0.950 0.003 264.0)"},
+ "200": {"value": "oklch(0.880 0.005 264.0)"},
+ "300": {"value": "oklch(0.780 0.008 264.0)"},
+ "400": {"value": "oklch(0.640 0.012 264.0)"},
+ "500": {"value": "oklch(0.520 0.015 264.0)"},
+ "600": {"value": "oklch(0.420 0.015 264.0)"},
+ "700": {"value": "oklch(0.340 0.012 264.0)"},
+ "800": {"value": "oklch(0.260 0.009 264.0)"},
+ "900": {"value": "oklch(0.200 0.007 264.0)"},
+ "950": {"value": "oklch(0.145 0.005 264.0)"},
+}
+_BLACK_SHADE = {"value": "oklch(0.220 0.025 288.6)"}
+_WHITE_SHADE = {"value": "oklch(0.985 0.002 264.0)"}
+
+
+class TestThemeColors:
+ def test_brand_only(self):
+ colors = ThemeColors.model_validate({"brand": _BRAND_SCALE})
+ assert colors.brand is not None
+ assert colors.gray is None
+ assert colors.black is None
+ assert colors.white is None
+
+ def test_gray_only(self):
+ colors = ThemeColors.model_validate({"gray": _GRAY_SCALE})
+ assert colors.gray is not None
+ assert colors.brand is None
+
+ def test_black_and_white_only(self):
+ colors = ThemeColors.model_validate({"black": _BLACK_SHADE, "white":
_WHITE_SHADE})
+ assert colors.black is not None
+ assert colors.white is not None
+ assert colors.brand is None
+ assert colors.gray is None
+
+ def test_all_tokens(self):
+ colors = ThemeColors.model_validate(
+ {"brand": _BRAND_SCALE, "gray": _GRAY_SCALE, "black":
_BLACK_SHADE, "white": _WHITE_SHADE}
+ )
+ assert colors.brand is not None
+ assert colors.gray is not None
+ assert colors.black is not None
+ assert colors.white is not None
+
+ def test_empty_colors_rejected(self):
+ with pytest.raises(ValidationError) as exc_info:
+ ThemeColors.model_validate({})
+ assert "At least one color token must be provided" in
str(exc_info.value)
+
+ def test_invalid_shade_key_rejected(self):
+ with pytest.raises(ValidationError):
+ ThemeColors.model_validate({"gray": {"999": {"value": "oklch(0.5
0.1 264.0)"}}})
+
+ def test_serialization_excludes_none_fields(self):
+ colors = ThemeColors.model_validate({"brand": _BRAND_SCALE})
+ dumped = colors.model_dump()
+ assert "brand" in dumped
+ assert "gray" not in dumped
+ assert "black" not in dumped
+ assert "white" not in dumped
+
+
+class TestTheme:
+ def test_brand_only_theme(self):
+ """Backwards-compatible: existing configs with only brand still
work."""
+ theme = Theme.model_validate({"tokens": {"colors": {"brand":
_BRAND_SCALE}}})
+ assert theme.tokens["colors"].brand is not None
+ assert theme.tokens["colors"].gray is None
+ assert theme.globalCss is None
+
+ def test_gray_only_theme(self):
+ """New: brand is no longer required."""
+ theme = Theme.model_validate({"tokens": {"colors": {"gray":
_GRAY_SCALE}}})
+ assert theme.tokens["colors"].gray is not None
+ assert theme.tokens["colors"].brand is None
+
+ def test_black_white_theme(self):
+ theme = Theme.model_validate({"tokens": {"colors": {"black":
_BLACK_SHADE, "white": _WHITE_SHADE}}})
+ assert theme.tokens["colors"].black is not None
+ assert theme.tokens["colors"].white is not None
+
+ def test_all_tokens_theme(self):
+ theme = Theme.model_validate(
+ {
+ "tokens": {
+ "colors": {
+ "brand": _BRAND_SCALE,
+ "gray": _GRAY_SCALE,
+ "black": _BLACK_SHADE,
+ "white": _WHITE_SHADE,
+ }
+ }
+ }
+ )
+ colors = theme.tokens["colors"]
+ assert colors.brand is not None
+ assert colors.gray is not None
+ assert colors.black is not None
+ assert colors.white is not None
+
+ def test_empty_colors_rejected(self):
+ with pytest.raises(ValidationError) as exc_info:
+ Theme.model_validate({"tokens": {"colors": {}}})
Review Comment:
Like here if `"tokens": {}` and we provide a `globalCss` entry, I don't
think it will work either, but is a valid use case probably.
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/ui/test_config.py:
##########
@@ -92,6 +92,59 @@ def mock_config_data():
yield
+THEME_WITH_ALL_COLORS = {
+ "tokens": {
+ "colors": {
+ "brand": {
+ "50": {"value": "oklch(0.975 0.008 298.0)"},
+ "100": {"value": "oklch(0.950 0.020 298.0)"},
+ "200": {"value": "oklch(0.900 0.045 298.0)"},
+ "300": {"value": "oklch(0.800 0.080 298.0)"},
+ "400": {"value": "oklch(0.680 0.120 298.0)"},
+ "500": {"value": "oklch(0.560 0.160 298.0)"},
+ "600": {"value": "oklch(0.460 0.190 298.0)"},
+ "700": {"value": "oklch(0.390 0.160 298.0)"},
+ "800": {"value": "oklch(0.328 0.080 298.0)"},
+ "900": {"value": "oklch(0.230 0.050 298.0)"},
+ "950": {"value": "oklch(0.155 0.030 298.0)"},
+ },
+ "gray": {
+ "50": {"value": "oklch(0.975 0.002 264.0)"},
+ "100": {"value": "oklch(0.950 0.003 264.0)"},
+ "200": {"value": "oklch(0.880 0.005 264.0)"},
+ "300": {"value": "oklch(0.780 0.008 264.0)"},
+ "400": {"value": "oklch(0.640 0.012 264.0)"},
+ "500": {"value": "oklch(0.520 0.015 264.0)"},
+ "600": {"value": "oklch(0.420 0.015 264.0)"},
+ "700": {"value": "oklch(0.340 0.012 264.0)"},
+ "800": {"value": "oklch(0.260 0.009 264.0)"},
+ "900": {"value": "oklch(0.200 0.007 264.0)"},
+ "950": {"value": "oklch(0.145 0.005 264.0)"},
+ },
+ "black": {"value": "oklch(0.220 0.025 288.6)"},
+ "white": {"value": "oklch(0.985 0.002 264.0)"},
+ }
+ },
+}
+
+
[email protected]
+def mock_config_data_all_colors():
+ with conf_vars(
+ {
+ ("api", "instance_name"): "Airflow",
+ ("api", "enable_swagger_ui"): "true",
+ ("api", "hide_paused_dags_by_default"): "true",
+ ("api", "fallback_page_limit"): "100",
+ ("api", "default_wrap"): "false",
+ ("api", "auto_refresh_interval"): "3",
+ ("api", "require_confirmation_dag_change"): "false",
Review Comment:
This can be factorized into a common 'fixture' used by both
`mock_config_data_all_colors` and `mock_config_data`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
