Copilot commented on code in PR #64267:
URL: https://github.com/apache/airflow/pull/64267#discussion_r3066476123
##########
airflow-core/src/airflow/ui/src/pages/Dashboard/Stats/PluginImportErrors.tsx:
##########
@@ -38,42 +38,50 @@ export const PluginImportErrors = ({ iconOnly = false }: {
readonly iconOnly?: b
const importErrors = data?.import_errors ?? [];
if (isLoading) {
- return <Skeleton height="9" width="225px" />;
+ // Skeleton dimensions match the rendered component sizes to prevent
layout shift.
+ // iconOnly: 28px × 60px matches StateBadge (height={7} = 28px in Chakra
spacing scale).
+ // full: 42px × 175px matches the rendered StatsCard dimensions.
+ return iconOnly ? (
+ <Skeleton height="28px" width="60px" />
+ ) : (
+ <Skeleton height="42px" width="175px" />
+ );
}
if (Boolean(error) && (error as ExpandedApiError).status === 403) {
return undefined;
}
- if (importErrorsCount === 0) {
+ if (importErrorsCount === 0 && !error) {
return undefined;
}
return (
<Box alignItems="center" display="flex">
<ErrorAlert error={error} />
- {iconOnly ? (
- <StateBadge
- as={Button}
- colorPalette="failed"
- height={7}
- onClick={onOpen}
- title={translate("plugins.importError", { count: importErrorsCount
})}
- >
- <LuPlug size={8} />
- {importErrorsCount}
- </StateBadge>
- ) : (
- <StatsCard
- colorScheme="failed"
- count={importErrorsCount}
- icon={<LuPlug />}
- isLoading={isLoading}
- isRTL={isRTL}
- label={translate("plugins.importError", { count: importErrorsCount
})}
- onClick={onOpen}
- />
- )}
+ {importErrorsCount > 0 &&
+ (iconOnly ? (
+ <StateBadge
+ as={Button}
+ colorPalette="failed"
+ height={7}
+ onClick={onOpen}
+ title={translate("plugins.importError", { count: importErrorsCount
})}
+ >
+ <LuPlug size={16} />
+ {importErrorsCount}
+ </StateBadge>
+ ) : (
+ <StatsCard
+ colorScheme="failed"
+ count={importErrorsCount}
+ icon={<LuPlug />}
+ isLoading={isLoading}
+ isRTL={isRTL}
+ label={translate("plugins.importError", { count: importErrorsCount
})}
+ onClick={onOpen}
+ />
+ ))}
Review Comment:
When the API call fails and `importErrorsCount` is `0`, the component now
renders only `ErrorAlert` and suppresses the `StateBadge`/`StatsCard`. This can
(a) remove the primary visual affordance from the stats area (potential layout
shift/empty slot) and (b) remove the only click target that opens the modal
(`onOpen`). Consider rendering the badge/card even when count is 0 if `error`
is present (e.g., disabled/readonly), or rendering a fixed-size placeholder so
the layout remains stable while still showing the error.
##########
airflow-core/src/airflow/ui/src/pages/Dashboard/Stats/DAGImportErrors.tsx:
##########
@@ -37,38 +37,46 @@ export const DAGImportErrors = ({ iconOnly = false }: {
readonly iconOnly?: bool
const importErrorsCount = data?.total_entries ?? 0;
if (isLoading) {
- return <Skeleton height="9" width="225px" />;
+ // Skeleton dimensions match the rendered component sizes to prevent
layout shift.
+ // iconOnly: 28px × 60px matches StateBadge (height={7} = 28px in Chakra
spacing scale).
+ // full: 42px × 175px matches the rendered StatsCard dimensions.
+ return iconOnly ? (
+ <Skeleton height="28px" width="60px" />
+ ) : (
+ <Skeleton height="42px" width="175px" />
+ );
}
- if (importErrorsCount === 0) {
+ if (importErrorsCount === 0 && !error) {
return undefined;
}
return (
<Box alignItems="center" display="flex">
<ErrorAlert error={error} />
- {iconOnly ? (
- <StateBadge
- as={Button}
- colorPalette="failed"
- height={7}
- onClick={onOpen}
- title={translate("importErrors.dagImportError", { count:
importErrorsCount })}
- >
- <LuFileWarning size={8} />
- {importErrorsCount}
- </StateBadge>
- ) : (
- <StatsCard
- colorScheme="failed"
- count={importErrorsCount}
- icon={<LuFileWarning />}
- isLoading={isLoading}
- isRTL={isRTL}
- label={translate("importErrors.dagImportError", { count:
importErrorsCount })}
- onClick={onOpen}
- />
- )}
+ {importErrorsCount > 0 &&
+ (iconOnly ? (
+ <StateBadge
+ as={Button}
+ colorPalette="failed"
+ height={7}
+ onClick={onOpen}
+ title={translate("importErrors.dagImportError", { count:
importErrorsCount })}
+ >
+ <LuFileWarning size={16} />
+ {importErrorsCount}
+ </StateBadge>
+ ) : (
+ <StatsCard
+ colorScheme="failed"
+ count={importErrorsCount}
+ icon={<LuFileWarning />}
+ isLoading={isLoading}
+ isRTL={isRTL}
+ label={translate("importErrors.dagImportError", { count:
importErrorsCount })}
+ onClick={onOpen}
+ />
+ ))}
Review Comment:
Same as `PluginImportErrors`: on API error with a default `total_entries` of
`0`, this suppresses the `StateBadge`/`StatsCard`, leaving only `ErrorAlert`
and removing the modal open trigger. Consider keeping a consistent-sized
badge/card (possibly disabled) when `error` is present so the dashboard layout
stays stable and users still have a clear interaction point.
--
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]
