YeonShin commented on code in PR #67595:
URL: https://github.com/apache/airflow/pull/67595#discussion_r3426340710
##########
airflow-core/src/airflow/ui/src/pages/Dag/Calendar/calendarUtils.ts:
##########
@@ -258,7 +275,29 @@ export const createCalendarScale = (
return PLANNED_COLOR;
}
- return actualCount === 0 ? EMPTY_COLOR : singleColor;
+ if (hasActual) {
+ if (failedCount > 0 && runningCount > 0) {
+ return { actual: RUNNING_COLOR, planned: failedColor };
+ }
+ if (failedCount > 0 && successCount > 0) {
+ return { actual: successColor, planned: failedColor };
+ }
+ if (runningCount > 0 && successCount > 0) {
+ return { actual: successColor, planned: RUNNING_COLOR };
+ }
+
+ if (failedCount > 0) {
+ return failedColor;
+ }
+ if (runningCount > 0) {
+ return RUNNING_COLOR;
+ }
+ if (successCount > 0) {
+ return successColor;
+ }
+ }
+
+ return EMPTY_COLOR;
Review Comment:
I completely agree. Honestly, I had the exact same concern while working on
this part because reusing those semantic keys felt like a trap for future
readers. Thank you for pointing it out—I have refactored and renamed the keys
to `primary` (top-right triangle) and `secondary` (bottom-left triangle) to
make it strictly about visual mapping rather than functional assumptions.
##########
airflow-core/src/airflow/ui/src/pages/Dag/Calendar/calendarUtils.ts:
##########
@@ -291,24 +330,39 @@ export const createCalendarScale = (
actual: string | { _dark: string; _light: string };
planned: string | { _dark: string; _light: string };
} => {
- const actualCount = getActualRunCount(counts, viewMode);
+ const failedCount = counts.failed;
+ const runningCount = viewMode === "total" ? counts.running : 0;
+ const successCount = viewMode === "total" ? counts.success : 0;
+
const hasPending = getPendingRunCount(counts) > 0;
- const hasActual = actualCount > 0;
+ const hasActual = failedCount > 0 || runningCount > 0 || successCount > 0;
- if (hasPending && hasActual) {
- let actualColor = colorScheme[0] ?? EMPTY_COLOR;
+ type ColorValue = string | { _dark: string; _light: string };
+ const getIntensityColor = (count: number, scheme: Array<ColorValue>) => {
Review Comment:
Thanks for the guidance! I understood that the bottom legend lacked
indicators for the actual run states (Success, Running, Failed) since it only
originally showed Planned and Mixed. To fix this, I extracted a reusable
`<LegendIcon/>` and expanded the bottom legend to explicitly map out Success
(green), Running (cyan), Failed (red), and Planned (gray) samples. This ensures
any status color or mixed blend on the grid can be instantly decoded by the
user.
##########
airflow-core/src/airflow/ui/src/pages/Dag/Calendar/calendarUtils.ts:
##########
@@ -321,21 +375,40 @@ export const createCalendarScale = (
return PLANNED_COLOR;
}
- const targetCount = actualCount;
+ if (hasActual) {
+ if (failedCount > 0 && runningCount > 0) {
+ return {
+ actual: RUNNING_COLOR,
+ planned: getIntensityColor(failedCount, FAILURE_COLOR_INTENSITIES),
+ };
+ }
- if (targetCount === 0) {
- return colorScheme[0] ?? EMPTY_COLOR;
- }
+ if (failedCount > 0 && successCount > 0) {
+ return {
+ actual: getIntensityColor(successCount, TOTAL_COLOR_INTENSITIES),
+ planned: getIntensityColor(failedCount, FAILURE_COLOR_INTENSITIES),
+ };
+ }
- for (let index = uniqueThresholds.length - 1; index >= 1; index -= 1) {
- const threshold = uniqueThresholds[index];
+ if (runningCount > 0 && successCount > 0) {
+ return {
+ actual: getIntensityColor(successCount, TOTAL_COLOR_INTENSITIES),
+ planned: RUNNING_COLOR,
+ };
+ }
- if (threshold !== undefined && targetCount >= threshold) {
- return colorScheme[Math.min(index, colorScheme.length - 1)] ??
EMPTY_COLOR;
+ if (failedCount > 0) {
+ return getIntensityColor(failedCount, FAILURE_COLOR_INTENSITIES);
+ }
+ if (runningCount > 0) {
Review Comment:
Done! I have extracted the priority mapping logic into a single helper
function named `resolveCellColor`. Both the single-value and gradient color
paths now route through this helper, eliminating duplication and ensuring the
two paths won't drift apart in the future.
--
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]
