bbovenzi commented on code in PR #75418:
URL: https://github.com/apache/airflow/pull/75418#discussion_r4360795718
##########
airflow-core/src/airflow/ui/src/components/FilterBar/FilterBar.test.tsx:
##########
@@ -90,6 +90,20 @@ describe("FilterBar boolean filters", () => {
expect(screen.getByTestId("needs_review-pill")).not.toHaveTextContent(":");
});
+ it("keeps the remove control out of the pill button", () => {
+ render(
+ <FilterBar
+ configs={[booleanConfig]}
+ initialValues={{ needs_review: "true" }}
+ onFiltersChange={vi.fn()}
+ />,
+ { wrapper },
+ );
+
+ // A button nested inside another button is invalid HTML and React warns
about it at runtime.
+
expect(within(screen.getByTestId("needs_review-pill")).queryAllByRole("button")).toHaveLength(0);
Review Comment:
Fixed in 3ca3a75 — now `expect(container.querySelector("button
button")).toBeNull()`. I checked it by putting `as="button"` back on the
wrapper: it fails, as does a new setup guard that fails any test logging React
markup errors.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
airflow-core/src/airflow/ui/src/components/WarningAlert.tsx:
##########
@@ -16,26 +16,30 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { Flex } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
-import { Alert } from "src/system-components";
+import { StatusMessage } from "src/components/StatusMessage";
type Props = {
readonly warning?: {
message: string;
+ warning_type?: string;
};
};
export const WarningAlert = ({ warning }: Props) => {
- if (!Boolean(warning)) {
+ const { t: translate } = useTranslation("components");
+
+ if (warning === undefined) {
return undefined;
}
+ // The type is the warning's own short form; only a synthesised warning
arrives without one.
return (
- <Alert data-testid="warning-alert" status="warning">
- <Flex align="center" whiteSpace="preserve" wordBreak="break-all">
- {warning?.message}
- </Flex>
- </Alert>
+ <StatusMessage
+ body={warning.message}
+ heading={warning.warning_type ?? translate("dagWarnings.warning", {
count: 1 })}
Review Comment:
Fixed in 3ca3a75 — types map through `components:dagWarnings.types.*` with a
`defaultValue` for unknown ones, and are translated into all 20 locales
(machine-generated, anchored on each locale's existing glossary — a locale
owner's eye would be welcome). The prop is now `DAGWarningResponse`, so the
unreachable fallback is gone.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
##########
airflow-core/src/airflow/ui/src/components/DAGWarningsModal.tsx:
##########
@@ -20,33 +20,65 @@ import { Heading, HStack, Spacer, VStack } from
"@chakra-ui/react";
import { useTranslation } from "react-i18next";
import { LuFileWarning } from "react-icons/lu";
-import type { DAGWarningResponse } from "openapi/requests/types.gen";
+import type { DAGWarningResponse, ImportErrorResponse } from
"openapi/requests/types.gen";
import { Modal } from "src/system-components";
-import { ErrorAlert } from "./ErrorAlert";
+import { getApiErrorDetail } from "src/utils";
+
+import { DagImportErrorDetails } from "./DagImportErrorDetails";
+import type { ExpandedApiError } from "./ErrorAlert";
+import { StatusMessage } from "./StatusMessage";
import { WarningAlert } from "./WarningAlert";
/** The modal lists the page error as one more item next to the warnings, so
the button count does too. */
Review Comment:
Dropped both in 3ca3a75 — `countDagWarnings` with its test, and
`common:dagWarnings` from all 21 locale files. Neither had a caller left.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
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]
