From a55ad3dc26c8873ad0fe042b22c31f0b4d5c9f46 Mon Sep 17 00:00:00 2001 From: Brent Bovenzi Date: Wed, 10 Jun 2026 15:50:07 -0600 Subject: [PATCH 1/7] Update note UX --- .../src/components/EditableMarkdownArea.tsx | 49 ++++- .../airflow/ui/src/components/HeaderCard.tsx | 8 +- .../ui/src/components/NoteAccordion.test.tsx | 170 ++++++++++++++++++ .../ui/src/components/NoteAccordion.tsx | 163 +++++++++++++++++ .../airflow/ui/src/pages/DagRuns/DagRuns.tsx | 2 + .../ui/src/pages/DagRuns/RunNoteButton.tsx | 62 +++++++ .../src/airflow/ui/src/pages/Run/Header.tsx | 31 ++-- .../ui/src/pages/TaskInstance/Header.tsx | 35 ++-- .../TaskInstances/TaskInstanceNoteButton.tsx | 66 +++++++ .../src/pages/TaskInstances/TaskInstances.tsx | 2 + 10 files changed, 531 insertions(+), 57 deletions(-) create mode 100644 airflow-core/src/airflow/ui/src/components/NoteAccordion.test.tsx create mode 100644 airflow-core/src/airflow/ui/src/components/NoteAccordion.tsx create mode 100644 airflow-core/src/airflow/ui/src/pages/DagRuns/RunNoteButton.tsx create mode 100644 airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstanceNoteButton.tsx diff --git a/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx b/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx index 3b467734a09eb..56174ed000ebe 100644 --- a/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx +++ b/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx @@ -18,23 +18,33 @@ */ import { Box, VStack, Editable, Text } from "@chakra-ui/react"; import type { ChangeEvent } from "react"; -import { useState, useRef } from "react"; +import { useState, useRef, useEffect, useCallback } from "react"; +import type { Components } from "react-markdown"; import ReactMarkdown from "./ReactMarkdown"; const EditableMarkdownArea = ({ + autoSize = false, + components, mdContent, onBlur, + onFocus, + padding = 4, placeholder, setMdContent, }: { + readonly autoSize?: boolean; + readonly components?: Partial; readonly mdContent?: string | null; readonly onBlur?: () => void; + readonly onFocus?: () => void; + readonly padding?: number; readonly placeholder?: string | null; readonly setMdContent: (value: string) => void; }) => { const [currentValue, setCurrentValue] = useState(mdContent ?? ""); const prevMdContentRef = useRef(mdContent); + const textareaRef = useRef(null); // Sync local state with prop changes if (mdContent !== prevMdContentRef.current) { @@ -42,10 +52,24 @@ const EditableMarkdownArea = ({ prevMdContentRef.current = mdContent; } + const resizeTextarea = useCallback(() => { + const el = textareaRef.current; + + if (autoSize && el !== null) { + el.style.height = "auto"; + el.style.height = `${el.scrollHeight}px`; + } + }, [autoSize]); + + // Resize whenever value changes + useEffect(() => { + resizeTextarea(); + }, [currentValue, resizeTextarea]); + return ( - + ) => { const { value } = event.target; @@ -60,21 +84,30 @@ const EditableMarkdownArea = ({ alignItems="flex-start" as={VStack} gap="0" - height="100%" - overflowY="auto" + height={autoSize ? undefined : "100%"} + overflowY={autoSize ? undefined : "auto"} width="100%" > {Boolean(currentValue) ? ( - {currentValue} + {currentValue} ) : ( {placeholder} )} { + resizeTextarea(); + onFocus?.(); + }} + overflow={autoSize ? "hidden" : "auto"} placeholder={placeholder ?? ""} + ref={textareaRef} resize="none" /> diff --git a/airflow-core/src/airflow/ui/src/components/HeaderCard.tsx b/airflow-core/src/airflow/ui/src/components/HeaderCard.tsx index 7908076bc4d9e..bb4ad874d168b 100644 --- a/airflow-core/src/airflow/ui/src/components/HeaderCard.tsx +++ b/airflow-core/src/airflow/ui/src/components/HeaderCard.tsx @@ -39,13 +39,7 @@ export const HeaderCard = ({ actions, icon, state, stats, subTitle, title }: Pro const { t: translate } = useTranslation(); return ( - + diff --git a/airflow-core/src/airflow/ui/src/components/NoteAccordion.test.tsx b/airflow-core/src/airflow/ui/src/components/NoteAccordion.test.tsx new file mode 100644 index 0000000000000..134d9ece79f40 --- /dev/null +++ b/airflow-core/src/airflow/ui/src/components/NoteAccordion.test.tsx @@ -0,0 +1,170 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import "@testing-library/jest-dom"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; + +import { Wrapper } from "src/utils/Wrapper"; + +import NoteAccordion from "./NoteAccordion"; + +const defaultProps = { + note: "", + onSave: vi.fn(), + setNote: vi.fn(), +}; + +const renderNote = (props: Partial = {}) => + render(, { wrapper: Wrapper }); + +describe("NoteAccordion", () => { + describe("empty state", () => { + it("shows the editable placeholder when there is no note", () => { + renderNote(); + // i18n returns the key in tests; the placeholder textarea is present + expect(screen.getByTestId("markdown-input")).toBeInTheDocument(); + }); + + it("does not show an expand/collapse button when there is no note", () => { + renderNote(); + expect(screen.queryByRole("button", { name: /expand/iu })).toBeNull(); + expect(screen.queryByRole("button", { name: /collapse/iu })).toBeNull(); + }); + + it("shows the editable area directly (no extra click needed)", () => { + renderNote(); + // The textarea (edit mode) is reachable without expanding first + expect(screen.getByTestId("markdown-input")).toBeInTheDocument(); + }); + }); + + describe("single-line note", () => { + const note = "A short single line note"; + + it("renders the note content", () => { + renderNote({ note }); + // The text appears in both the rendered

and the hidden textarea — target the paragraph + expect(screen.getByText(note, { selector: "p" })).toBeInTheDocument(); + }); + + it("does not show an expand/collapse button for a single-line note", () => { + renderNote({ note }); + expect(screen.queryByRole("button", { name: /expand/iu })).toBeNull(); + expect(screen.queryByRole("button", { name: /collapse/iu })).toBeNull(); + }); + + it("shows the editable area directly (single click to edit)", () => { + renderNote({ note }); + expect(screen.getByTestId("markdown-input")).toBeInTheDocument(); + }); + }); + + describe("multiline note", () => { + const note = "First line\nSecond line"; + + it("renders the first line as a collapsed preview", () => { + renderNote({ note }); + expect(screen.getByText("First line")).toBeInTheDocument(); + }); + + it("shows an expand button for a multiline note", () => { + renderNote({ note }); + expect(screen.getByRole("button", { name: /expand/iu })).toBeInTheDocument(); + }); + + it("does not show the edit area before expanding", () => { + renderNote({ note }); + // The grid hides the content — markdown-input is mounted but hidden via grid 0fr + // The collapsed preview text is visible + expect(screen.getByText("First line")).toBeInTheDocument(); + }); + + it("shows the collapse button after expanding", () => { + renderNote({ note }); + fireEvent.click(screen.getByRole("button", { name: /expand/iu })); + expect(screen.getByRole("button", { name: /collapse/iu })).toBeInTheDocument(); + }); + + it("hides the collapsed preview after expanding", () => { + renderNote({ note }); + fireEvent.click(screen.getByRole("button", { name: /expand/iu })); + // The first-line preview box is removed from the DOM when expanded + expect(screen.queryByText("First line")).toBeNull(); + }); + + it("collapses back when the collapse button is clicked", () => { + renderNote({ note }); + fireEvent.click(screen.getByRole("button", { name: /expand/iu })); + fireEvent.click(screen.getByRole("button", { name: /collapse/iu })); + expect(screen.getByText("First line")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /expand/iu })).toBeInTheDocument(); + }); + }); + + describe("long single-line note", () => { + const note = "A".repeat(81); // over the 80-char threshold + + it("shows an expand button for a long single-line note", () => { + renderNote({ note }); + expect(screen.getByRole("button", { name: /expand/iu })).toBeInTheDocument(); + }); + }); + + describe("editing", () => { + it("hides the expand/collapse button while the textarea is focused", () => { + const note = "First line\nSecond line"; + + renderNote({ note }); + fireEvent.click(screen.getByRole("button", { name: /expand/iu })); + expect(screen.getByRole("button", { name: /collapse/iu })).toBeInTheDocument(); + + fireEvent.focus(screen.getByTestId("markdown-input")); + expect(screen.queryByRole("button", { name: /collapse/iu })).toBeNull(); + }); + + it("calls onSave when the textarea loses focus", () => { + const onSave = vi.fn(); + + renderNote({ note: "some note", onSave }); + fireEvent.focus(screen.getByTestId("markdown-input")); + fireEvent.blur(screen.getByTestId("markdown-input")); + expect(onSave).toHaveBeenCalledTimes(1); + }); + + it("calls setNote when the textarea value changes", () => { + const setNote = vi.fn(); + + renderNote({ setNote }); + fireEvent.change(screen.getByTestId("markdown-input"), { target: { value: "new content" } }); + expect(setNote).toHaveBeenCalledWith("new content"); + }); + }); + + describe("note with only whitespace or trailing newlines", () => { + it("treats a whitespace-only note as empty", () => { + renderNote({ note: " \n " }); + expect(screen.queryByRole("button", { name: /expand/iu })).toBeNull(); + }); + + it("does not show expand for a note with a single meaningful line and trailing newline", () => { + renderNote({ note: "Single line\n" }); + expect(screen.queryByRole("button", { name: /expand/iu })).toBeNull(); + }); + }); +}); diff --git a/airflow-core/src/airflow/ui/src/components/NoteAccordion.tsx b/airflow-core/src/airflow/ui/src/components/NoteAccordion.tsx new file mode 100644 index 0000000000000..e4df6ea97d356 --- /dev/null +++ b/airflow-core/src/airflow/ui/src/components/NoteAccordion.tsx @@ -0,0 +1,163 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Box, Heading, HStack } from "@chakra-ui/react"; +import type { PropsWithChildren } from "react"; +import { useState } from "react"; +import { useTranslation } from "react-i18next"; +import { MdExpandMore } from "react-icons/md"; +import { PiNoteBold, PiNoteBlankBold } from "react-icons/pi"; + +import ReactMarkdown from "src/components/ReactMarkdown"; +import { IconButton } from "src/components/ui"; + +import EditableMarkdownArea from "./EditableMarkdownArea"; + +type Props = { + readonly note: string | null; + readonly onSave: () => void; + readonly setNote: (value: string) => void; +}; + +// Compact heading overrides — same visual weight as body text, just bold +const compactHeadingComponents = { + h1: ({ children }: PropsWithChildren) => ( + + {children} + + ), + h2: ({ children }: PropsWithChildren) => ( + + {children} + + ), + h3: ({ children }: PropsWithChildren) => ( + + {children} + + ), +}; + +const NoteAccordion = ({ note, onSave, setNote }: Props) => { + const { t: translate } = useTranslation(); + const [isExpanded, setIsExpanded] = useState(false); + const [isEditing, setIsEditing] = useState(false); + + const hasNote = note !== null && note.trim().length > 0; + const firstLine = hasNote ? (note.split("\n")[0] ?? "") : ""; + + // Content can be expanded if it has more than one non-empty line, or the first line is long + const meaningfulLines = hasNote + ? note + .trim() + .split("\n") + .filter((line) => line.trim().length > 0) + : []; + const canExpand = hasNote && (meaningfulLines.length > 1 || firstLine.length > 80); + + // While editing: always show full content, suppress expand/collapse chrome + // After blur: canExpand and isExpanded drive visibility normally + const showContent = isEditing || !hasNote || !canExpand || isExpanded; + const showCollapsedPreview = !isEditing && hasNote && canExpand && !isExpanded; + const showChevron = !isEditing && canExpand; + + return ( + + {/* Icon pinned to top-left */} + + {hasNote ? : } + + + + {/* Collapsed preview — only for multiline notes that haven't been expanded */} + {showCollapsedPreview ? ( + setIsExpanded(true)} + overflow="hidden" + style={{ + display: "-webkit-box", + overflow: "hidden", + WebkitBoxOrient: "vertical", + WebkitLineClamp: 1, + whiteSpace: "normal", + }} + > + {firstLine} + + ) : undefined} + + {/* + * EditableMarkdownArea stays mounted at the same tree position always. + * Visibility is driven by the grid trick so React never unmounts it — + * this preserves focus when hasNote flips from false→true on first keystroke. + * Single-line notes always have showContent=true so they're directly clickable. + */} + + + { + setIsEditing(false); + onSave(); + }} + onFocus={() => { + setIsEditing(true); + setIsExpanded(true); + }} + padding={0} + placeholder={translate("note.placeholder")} + setMdContent={setNote} + /> + + + + + {/* Chevron only after save, when content spans more than one line */} + {showChevron ? ( + setIsExpanded((prev) => !prev)} + size="xs" + variant="ghost" + > + + + + + ) : undefined} + + ); +}; + +export default NoteAccordion; diff --git a/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.tsx b/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.tsx index 4443ec5305649..971354ebe55b5 100644 --- a/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.tsx +++ b/airflow-core/src/airflow/ui/src/pages/DagRuns/DagRuns.tsx @@ -54,6 +54,7 @@ import BulkDeleteDagRunsButton from "./BulkDeleteDagRunsButton"; import BulkMarkDagRunsAsButton from "./BulkMarkDagRunsAsButton"; import { DagRunsFilters } from "./DagRunsFilters"; import DeleteRunButton from "./DeleteRunButton"; +import RunNoteButton from "./RunNoteButton"; // Matches the identifier the bulk Dag Run endpoint echoes back in its ``success`` / // ``errors`` lists, so the bulk response can deselect rows directly. @@ -203,6 +204,7 @@ const runColumns = ({ dagId, translate }: ColumnProps): Array ( + diff --git a/airflow-core/src/airflow/ui/src/pages/DagRuns/RunNoteButton.tsx b/airflow-core/src/airflow/ui/src/pages/DagRuns/RunNoteButton.tsx new file mode 100644 index 0000000000000..2fc7797bb5f3e --- /dev/null +++ b/airflow-core/src/airflow/ui/src/pages/DagRuns/RunNoteButton.tsx @@ -0,0 +1,62 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useState } from "react"; +import { useTranslation } from "react-i18next"; + +import type { DAGRunResponse } from "openapi/requests/types.gen"; +import EditableMarkdownButton from "src/components/EditableMarkdownButton"; +import { usePatchDagRun } from "src/queries/usePatchDagRun"; + +const RunNoteButton = ({ dagRun }: { readonly dagRun: DAGRunResponse }) => { + const { t: translate } = useTranslation(); + const [note, setNote] = useState(dagRun.note); + + const { isPending, mutate } = usePatchDagRun({ + dagId: dagRun.dag_id, + dagRunId: dagRun.dag_run_id, + }); + + const onConfirm = () => { + if (note !== dagRun.note) { + mutate({ + dagId: dagRun.dag_id, + dagRunId: dagRun.dag_run_id, + requestBody: { note }, + }); + } + }; + + const onOpen = () => { + setNote(dagRun.note ?? ""); + }; + + return ( + + ); +}; + +export default RunNoteButton; diff --git a/airflow-core/src/airflow/ui/src/pages/Run/Header.tsx b/airflow-core/src/airflow/ui/src/pages/Run/Header.tsx index 18c0e92742642..dd9487947b36a 100644 --- a/airflow-core/src/airflow/ui/src/pages/Run/Header.tsx +++ b/airflow-core/src/airflow/ui/src/pages/Run/Header.tsx @@ -25,10 +25,10 @@ import { useDeadlinesServiceGetDagDeadlineAlerts } from "openapi/queries"; import type { DAGRunResponse } from "openapi/requests/types.gen"; import { ClearRunButton } from "src/components/Clear"; import { DagVersion } from "src/components/DagVersion"; -import EditableMarkdownButton from "src/components/EditableMarkdownButton"; import { HeaderCard } from "src/components/HeaderCard"; import { LimitedItemsList } from "src/components/LimitedItemsList"; import { MarkRunAsButton } from "src/components/MarkAs"; +import NoteAccordion from "src/components/NoteAccordion"; import { RunTypeIcon } from "src/components/RunTypeIcon"; import Time from "src/components/Time"; import { RouterLink } from "src/components/ui"; @@ -49,39 +49,29 @@ export const Header = ({ dagRun }: { readonly dagRun: DAGRunResponse }) => { const { data: alertData } = useDeadlinesServiceGetDagDeadlineAlerts({ dagId }); const hasDeadlineAlerts = (alertData?.total_entries ?? 0) > 0; - const { isPending, mutate } = usePatchDagRun({ + const { mutate } = usePatchDagRun({ dagId, dagRunId, }); const onConfirm = () => { if (note !== dagRun.note) { - mutate({ - dagId, - dagRunId, - requestBody: { note }, - }); + mutate( + { + dagId, + dagRunId, + requestBody: { note }, + }, + { onError: () => setNote(dagRun.note ?? null) }, + ); } }; - const onOpen = () => { - setNote(dagRun.note ?? ""); - }; - return ( - @@ -154,6 +144,7 @@ export const Header = ({ dagRun }: { readonly dagRun: DAGRunResponse }) => { ]} title={dagRun.dag_run_id} /> + ); }; diff --git a/airflow-core/src/airflow/ui/src/pages/TaskInstance/Header.tsx b/airflow-core/src/airflow/ui/src/pages/TaskInstance/Header.tsx index d053c68534c10..4385ef4ec762c 100644 --- a/airflow-core/src/airflow/ui/src/pages/TaskInstance/Header.tsx +++ b/airflow-core/src/airflow/ui/src/pages/TaskInstance/Header.tsx @@ -25,9 +25,9 @@ import type { TaskInstanceResponse } from "openapi/requests/types.gen"; import { ClearTaskInstanceButton } from "src/components/Clear"; import ClearTaskInstanceDialog from "src/components/Clear/TaskInstance/ClearTaskInstanceDialog"; import { DagVersion } from "src/components/DagVersion"; -import EditableMarkdownButton from "src/components/EditableMarkdownButton"; import { HeaderCard } from "src/components/HeaderCard"; import { MarkTaskInstanceAsButton } from "src/components/MarkAs"; +import NoteAccordion from "src/components/NoteAccordion"; import Time from "src/components/Time"; import { usePatchTaskInstance } from "src/queries/usePatchTaskInstance"; import { getDuration, renderDuration } from "src/utils"; @@ -68,7 +68,7 @@ export const Header = ({ taskInstance }: { readonly taskInstance: TaskInstanceRe }, ]; - const { isPending, mutate } = usePatchTaskInstance({ + const { mutate } = usePatchTaskInstance({ dagId, dagRunId, mapIndex, @@ -77,20 +77,19 @@ export const Header = ({ taskInstance }: { readonly taskInstance: TaskInstanceRe const onConfirm = () => { if (note !== taskInstance.note) { - mutate({ - dagId, - dagRunId, - mapIndex, - requestBody: { note }, - taskId, - }); + mutate( + { + dagId, + dagRunId, + mapIndex, + requestBody: { note }, + taskId, + }, + { onError: () => setNote(taskInstance.note ?? null) }, + ); } }; - const onOpen = () => { - setNote(taskInstance.note ?? ""); - }; - // Stable dialog state at header/page level const [clearOpen, setClearOpen] = useState(false); @@ -99,15 +98,6 @@ export const Header = ({ taskInstance }: { readonly taskInstance: TaskInstanceRe - setClearOpen(true)} @@ -121,6 +111,7 @@ export const Header = ({ taskInstance }: { readonly taskInstance: TaskInstanceRe stats={stats} title={`${taskInstance.task_display_name}${taskInstance.map_index > -1 ? ` [${taskInstance.rendered_map_index ?? taskInstance.map_index}]` : ""}`} /> + setClearOpen(false)} open={clearOpen} diff --git a/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstanceNoteButton.tsx b/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstanceNoteButton.tsx new file mode 100644 index 0000000000000..2c2b506faec21 --- /dev/null +++ b/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstanceNoteButton.tsx @@ -0,0 +1,66 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useState } from "react"; +import { useTranslation } from "react-i18next"; + +import type { TaskInstanceResponse } from "openapi/requests/types.gen"; +import EditableMarkdownButton from "src/components/EditableMarkdownButton"; +import { usePatchTaskInstance } from "src/queries/usePatchTaskInstance"; + +const TaskInstanceNoteButton = ({ taskInstance }: { readonly taskInstance: TaskInstanceResponse }) => { + const { t: translate } = useTranslation(); + const [note, setNote] = useState(taskInstance.note); + + const { isPending, mutate } = usePatchTaskInstance({ + dagId: taskInstance.dag_id, + dagRunId: taskInstance.dag_run_id, + mapIndex: taskInstance.map_index, + taskId: taskInstance.task_id, + }); + + const onConfirm = () => { + if (note !== taskInstance.note) { + mutate({ + dagId: taskInstance.dag_id, + dagRunId: taskInstance.dag_run_id, + mapIndex: taskInstance.map_index, + requestBody: { note }, + taskId: taskInstance.task_id, + }); + } + }; + + const onOpen = () => { + setNote(taskInstance.note ?? ""); + }; + + return ( + + ); +}; + +export default TaskInstanceNoteButton; diff --git a/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstances.tsx b/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstances.tsx index ff28b5eb7b53b..3249b87662150 100644 --- a/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstances.tsx +++ b/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstances.tsx @@ -51,6 +51,7 @@ import BulkClearTaskInstancesButton from "./BulkClearTaskInstancesButton"; import BulkDeleteTaskInstancesButton from "./BulkDeleteTaskInstancesButton"; import BulkMarkTaskInstancesAsButton from "./BulkMarkTaskInstancesAsButton"; import DeleteTaskInstanceButton from "./DeleteTaskInstanceButton"; +import TaskInstanceNoteButton from "./TaskInstanceNoteButton"; import { TaskInstancesFilter } from "./TaskInstancesFilter"; type TaskInstanceRow = { row: { original: TaskInstanceResponse } }; @@ -235,6 +236,7 @@ const taskInstanceColumns = ({ accessorKey: "actions", cell: ({ row }) => ( + From db19329b49d610edb403323dfa3f644c650f1426 Mon Sep 17 00:00:00 2001 From: Brent Bovenzi Date: Wed, 10 Jun 2026 16:03:24 -0600 Subject: [PATCH 2/7] Remove useEffects --- .../ui/src/components/EditableMarkdownArea.tsx | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx b/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx index 56174ed000ebe..2305a45abe95c 100644 --- a/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx +++ b/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx @@ -18,7 +18,7 @@ */ import { Box, VStack, Editable, Text } from "@chakra-ui/react"; import type { ChangeEvent } from "react"; -import { useState, useRef, useEffect, useCallback } from "react"; +import { useState, useRef } from "react"; import type { Components } from "react-markdown"; import ReactMarkdown from "./ReactMarkdown"; @@ -46,25 +46,20 @@ const EditableMarkdownArea = ({ const prevMdContentRef = useRef(mdContent); const textareaRef = useRef(null); - // Sync local state with prop changes + // Sync local state with prop changes (e.g. revert-on-error from parent) if (mdContent !== prevMdContentRef.current) { setCurrentValue(mdContent ?? ""); prevMdContentRef.current = mdContent; } - const resizeTextarea = useCallback(() => { + const resizeTextarea = () => { const el = textareaRef.current; if (autoSize && el !== null) { el.style.height = "auto"; el.style.height = `${el.scrollHeight}px`; } - }, [autoSize]); - - // Resize whenever value changes - useEffect(() => { - resizeTextarea(); - }, [currentValue, resizeTextarea]); + }; return ( @@ -76,6 +71,7 @@ const EditableMarkdownArea = ({ setCurrentValue(value); setMdContent(value); + resizeTextarea(); }} value={currentValue} > @@ -102,6 +98,7 @@ const EditableMarkdownArea = ({ data-testid="markdown-input" height={autoSize ? undefined : "100%"} onFocus={() => { + // Resize on focus so the textarea fits existing content when first opened resizeTextarea(); onFocus?.(); }} From f6c43d7427295f8401c06af1fc2e5347a1e64396 Mon Sep 17 00:00:00 2001 From: Brent Bovenzi Date: Wed, 10 Jun 2026 16:35:22 -0600 Subject: [PATCH 3/7] code cleanup --- .../ui/src/pages/DagRuns/RunNoteButton.tsx | 28 ++------- .../src/airflow/ui/src/pages/Run/Header.tsx | 25 +------- .../ui/src/pages/TaskInstance/Header.tsx | 34 +---------- .../TaskInstances/TaskInstanceNoteButton.tsx | 32 ++-------- .../airflow/ui/src/queries/useDagRunNote.ts | 56 +++++++++++++++++ .../ui/src/queries/useTaskInstanceNote.ts | 60 +++++++++++++++++++ 6 files changed, 130 insertions(+), 105 deletions(-) create mode 100644 airflow-core/src/airflow/ui/src/queries/useDagRunNote.ts create mode 100644 airflow-core/src/airflow/ui/src/queries/useTaskInstanceNote.ts diff --git a/airflow-core/src/airflow/ui/src/pages/DagRuns/RunNoteButton.tsx b/airflow-core/src/airflow/ui/src/pages/DagRuns/RunNoteButton.tsx index 2fc7797bb5f3e..f8d5c732866ad 100644 --- a/airflow-core/src/airflow/ui/src/pages/DagRuns/RunNoteButton.tsx +++ b/airflow-core/src/airflow/ui/src/pages/DagRuns/RunNoteButton.tsx @@ -16,42 +16,22 @@ * specific language governing permissions and limitations * under the License. */ -import { useState } from "react"; import { useTranslation } from "react-i18next"; import type { DAGRunResponse } from "openapi/requests/types.gen"; import EditableMarkdownButton from "src/components/EditableMarkdownButton"; -import { usePatchDagRun } from "src/queries/usePatchDagRun"; +import { useDagRunNote } from "src/queries/useDagRunNote"; const RunNoteButton = ({ dagRun }: { readonly dagRun: DAGRunResponse }) => { const { t: translate } = useTranslation(); - const [note, setNote] = useState(dagRun.note); - - const { isPending, mutate } = usePatchDagRun({ - dagId: dagRun.dag_id, - dagRunId: dagRun.dag_run_id, - }); - - const onConfirm = () => { - if (note !== dagRun.note) { - mutate({ - dagId: dagRun.dag_id, - dagRunId: dagRun.dag_run_id, - requestBody: { note }, - }); - } - }; - - const onOpen = () => { - setNote(dagRun.note ?? ""); - }; + const { isPending, note, onOpen, onSave, setNote } = useDagRunNote(dagRun); return ( { const { t: translate } = useTranslation(); - const [note, setNote] = useState(dagRun.note); + const { note, onSave, setNote } = useDagRunNote(dagRun); const dagId = dagRun.dag_id; const dagRunId = dagRun.dag_run_id; @@ -49,24 +48,6 @@ export const Header = ({ dagRun }: { readonly dagRun: DAGRunResponse }) => { const { data: alertData } = useDeadlinesServiceGetDagDeadlineAlerts({ dagId }); const hasDeadlineAlerts = (alertData?.total_entries ?? 0) > 0; - const { mutate } = usePatchDagRun({ - dagId, - dagRunId, - }); - - const onConfirm = () => { - if (note !== dagRun.note) { - mutate( - { - dagId, - dagRunId, - requestBody: { note }, - }, - { onError: () => setNote(dagRun.note ?? null) }, - ); - } - }; - return ( { ]} title={dagRun.dag_run_id} /> - + ); }; diff --git a/airflow-core/src/airflow/ui/src/pages/TaskInstance/Header.tsx b/airflow-core/src/airflow/ui/src/pages/TaskInstance/Header.tsx index 4385ef4ec762c..62a27ab077225 100644 --- a/airflow-core/src/airflow/ui/src/pages/TaskInstance/Header.tsx +++ b/airflow-core/src/airflow/ui/src/pages/TaskInstance/Header.tsx @@ -29,18 +29,12 @@ import { HeaderCard } from "src/components/HeaderCard"; import { MarkTaskInstanceAsButton } from "src/components/MarkAs"; import NoteAccordion from "src/components/NoteAccordion"; import Time from "src/components/Time"; -import { usePatchTaskInstance } from "src/queries/usePatchTaskInstance"; +import { useTaskInstanceNote } from "src/queries/useTaskInstanceNote"; import { getDuration, renderDuration } from "src/utils"; export const Header = ({ taskInstance }: { readonly taskInstance: TaskInstanceResponse }) => { const { t: translate } = useTranslation(); - - const [note, setNote] = useState(taskInstance.note); - - const dagId = taskInstance.dag_id; - const dagRunId = taskInstance.dag_run_id; - const taskId = taskInstance.task_id; - const mapIndex = taskInstance.map_index; + const { note, onSave, setNote } = useTaskInstanceNote(taskInstance); const stats = [ { label: translate("task.operator"), value: taskInstance.operator_name }, @@ -68,28 +62,6 @@ export const Header = ({ taskInstance }: { readonly taskInstance: TaskInstanceRe }, ]; - const { mutate } = usePatchTaskInstance({ - dagId, - dagRunId, - mapIndex, - taskId, - }); - - const onConfirm = () => { - if (note !== taskInstance.note) { - mutate( - { - dagId, - dagRunId, - mapIndex, - requestBody: { note }, - taskId, - }, - { onError: () => setNote(taskInstance.note ?? null) }, - ); - } - }; - // Stable dialog state at header/page level const [clearOpen, setClearOpen] = useState(false); @@ -111,7 +83,7 @@ export const Header = ({ taskInstance }: { readonly taskInstance: TaskInstanceRe stats={stats} title={`${taskInstance.task_display_name}${taskInstance.map_index > -1 ? ` [${taskInstance.rendered_map_index ?? taskInstance.map_index}]` : ""}`} /> - + setClearOpen(false)} open={clearOpen} diff --git a/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstanceNoteButton.tsx b/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstanceNoteButton.tsx index 2c2b506faec21..58d1faa79e91b 100644 --- a/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstanceNoteButton.tsx +++ b/airflow-core/src/airflow/ui/src/pages/TaskInstances/TaskInstanceNoteButton.tsx @@ -16,46 +16,22 @@ * specific language governing permissions and limitations * under the License. */ -import { useState } from "react"; import { useTranslation } from "react-i18next"; import type { TaskInstanceResponse } from "openapi/requests/types.gen"; import EditableMarkdownButton from "src/components/EditableMarkdownButton"; -import { usePatchTaskInstance } from "src/queries/usePatchTaskInstance"; +import { useTaskInstanceNote } from "src/queries/useTaskInstanceNote"; const TaskInstanceNoteButton = ({ taskInstance }: { readonly taskInstance: TaskInstanceResponse }) => { const { t: translate } = useTranslation(); - const [note, setNote] = useState(taskInstance.note); - - const { isPending, mutate } = usePatchTaskInstance({ - dagId: taskInstance.dag_id, - dagRunId: taskInstance.dag_run_id, - mapIndex: taskInstance.map_index, - taskId: taskInstance.task_id, - }); - - const onConfirm = () => { - if (note !== taskInstance.note) { - mutate({ - dagId: taskInstance.dag_id, - dagRunId: taskInstance.dag_run_id, - mapIndex: taskInstance.map_index, - requestBody: { note }, - taskId: taskInstance.task_id, - }); - } - }; - - const onOpen = () => { - setNote(taskInstance.note ?? ""); - }; + const { isPending, note, onOpen, onSave, setNote } = useTaskInstanceNote(taskInstance); return ( { + const [note, setNote] = useState(dagRun.note); + + const { isPending, mutate } = usePatchDagRun({ + dagId: dagRun.dag_id, + dagRunId: dagRun.dag_run_id, + }); + + const onSave = () => { + if (note !== dagRun.note) { + mutate( + { + dagId: dagRun.dag_id, + dagRunId: dagRun.dag_run_id, + requestBody: { note }, + }, + { onError: () => setNote(dagRun.note ?? null) }, + ); + } + }; + + // Reset local state to server value each time an edit surface is opened, + // so stale edits from a previous session don't linger. + const onOpen = () => setNote(dagRun.note ?? ""); + + return { isPending, note, onOpen, onSave, setNote }; +}; diff --git a/airflow-core/src/airflow/ui/src/queries/useTaskInstanceNote.ts b/airflow-core/src/airflow/ui/src/queries/useTaskInstanceNote.ts new file mode 100644 index 0000000000000..0c7a3d08808f1 --- /dev/null +++ b/airflow-core/src/airflow/ui/src/queries/useTaskInstanceNote.ts @@ -0,0 +1,60 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useState } from "react"; + +import type { TaskInstanceResponse } from "openapi/requests/types.gen"; + +import { usePatchTaskInstance } from "./usePatchTaskInstance"; + +/** + * Shared note-editing state and save logic for a task instance. + * Used by both the detail-page NoteAccordion and the list-page TaskInstanceNoteButton + * so mutation + cache invalidation stays in one place. + */ +export const useTaskInstanceNote = (taskInstance: TaskInstanceResponse) => { + const [note, setNote] = useState(taskInstance.note); + + const { isPending, mutate } = usePatchTaskInstance({ + dagId: taskInstance.dag_id, + dagRunId: taskInstance.dag_run_id, + mapIndex: taskInstance.map_index, + taskId: taskInstance.task_id, + }); + + const onSave = () => { + if (note !== taskInstance.note) { + mutate( + { + dagId: taskInstance.dag_id, + dagRunId: taskInstance.dag_run_id, + mapIndex: taskInstance.map_index, + requestBody: { note }, + taskId: taskInstance.task_id, + }, + { onError: () => setNote(taskInstance.note ?? null) }, + ); + } + }; + + // Reset local state to server value each time an edit surface is opened, + // so stale edits from a previous session don't linger. + const onOpen = () => setNote(taskInstance.note ?? ""); + + return { isPending, note, onOpen, onSave, setNote }; +}; From b35e607943004bd41cd7924f59e6469520c5fb47 Mon Sep 17 00:00:00 2001 From: Brent Bovenzi Date: Thu, 11 Jun 2026 09:43:18 -0600 Subject: [PATCH 4/7] Fix markdown --- .../ui/src/components/NoteAccordion.tsx | 25 ++----------------- .../ui/src/components/ReactMarkdown.tsx | 11 ++++++-- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/airflow-core/src/airflow/ui/src/components/NoteAccordion.tsx b/airflow-core/src/airflow/ui/src/components/NoteAccordion.tsx index e4df6ea97d356..3fcc0eeccdb45 100644 --- a/airflow-core/src/airflow/ui/src/components/NoteAccordion.tsx +++ b/airflow-core/src/airflow/ui/src/components/NoteAccordion.tsx @@ -16,8 +16,7 @@ * specific language governing permissions and limitations * under the License. */ -import { Box, Heading, HStack } from "@chakra-ui/react"; -import type { PropsWithChildren } from "react"; +import { Box, HStack } from "@chakra-ui/react"; import { useState } from "react"; import { useTranslation } from "react-i18next"; import { MdExpandMore } from "react-icons/md"; @@ -34,25 +33,6 @@ type Props = { readonly setNote: (value: string) => void; }; -// Compact heading overrides — same visual weight as body text, just bold -const compactHeadingComponents = { - h1: ({ children }: PropsWithChildren) => ( - - {children} - - ), - h2: ({ children }: PropsWithChildren) => ( - - {children} - - ), - h3: ({ children }: PropsWithChildren) => ( - - {children} - - ), -}; - const NoteAccordion = ({ note, onSave, setNote }: Props) => { const { t: translate } = useTranslation(); const [isExpanded, setIsExpanded] = useState(false); @@ -98,7 +78,7 @@ const NoteAccordion = ({ note, onSave, setNote }: Props) => { whiteSpace: "normal", }} > - {firstLine} + {firstLine} ) : undefined} @@ -118,7 +98,6 @@ const NoteAccordion = ({ note, onSave, setNote }: Props) => { { setIsEditing(false); diff --git a/airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx b/airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx index 57a585a2b91c4..020e14172a08e 100644 --- a/airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx +++ b/airflow-core/src/airflow/ui/src/components/ReactMarkdown.tsx @@ -146,7 +146,7 @@ const createCodeComponent = ); }; -const ReactMarkdown = (props: Options) => { +const ReactMarkdown = ({ components: componentOverrides, ...restProps }: Options) => { const { colorMode } = useColorMode(); const style = colorMode === "dark" ? oneDark : oneLight; @@ -180,7 +180,14 @@ const ReactMarkdown = (props: Options) => { ul: UlComponent, }; - return ; + return ( + + ); }; export default ReactMarkdown; From 2ef5afc2e7ccc2c1bc9a7713c032c216630ab0fc Mon Sep 17 00:00:00 2001 From: Brent Bovenzi Date: Tue, 16 Jun 2026 00:41:20 -0600 Subject: [PATCH 5/7] Revert back to modal, with preview --- .../ui/public/i18n/locales/en/common.json | 5 +- .../src/components/EditableMarkdownArea.tsx | 115 ------------ .../src/components/EditableMarkdownButton.tsx | 60 ++----- .../ui/src/components/MarkdownModal.test.tsx | 131 ++++++++++++++ .../ui/src/components/MarkdownModal.tsx | 160 +++++++++++++++++ .../ui/src/components/NoteAccordion.test.tsx | 170 ------------------ .../ui/src/components/NoteAccordion.tsx | 142 --------------- .../airflow/ui/src/components/NoteIcon.tsx | 25 +++ .../ui/src/components/NotePreview.test.tsx | 92 ++++++++++ .../airflow/ui/src/components/NotePreview.tsx | 101 +++++++++++ .../ui/src/components/ReactMarkdown.tsx | 23 +-- .../ui/src/components/ui/ResizableWrapper.tsx | 10 +- .../src/airflow/ui/src/pages/Run/Header.tsx | 13 +- .../ui/src/pages/TaskInstance/Header.tsx | 13 +- .../airflow/ui/src/queries/useDagRunNote.ts | 33 +--- .../airflow/ui/src/queries/useNoteEditor.ts | 48 +++++ .../ui/src/queries/useTaskInstanceNote.ts | 28 ++- 17 files changed, 629 insertions(+), 540 deletions(-) delete mode 100644 airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx create mode 100644 airflow-core/src/airflow/ui/src/components/MarkdownModal.test.tsx create mode 100644 airflow-core/src/airflow/ui/src/components/MarkdownModal.tsx delete mode 100644 airflow-core/src/airflow/ui/src/components/NoteAccordion.test.tsx delete mode 100644 airflow-core/src/airflow/ui/src/components/NoteAccordion.tsx create mode 100644 airflow-core/src/airflow/ui/src/components/NoteIcon.tsx create mode 100644 airflow-core/src/airflow/ui/src/components/NotePreview.test.tsx create mode 100644 airflow-core/src/airflow/ui/src/components/NotePreview.tsx create mode 100644 airflow-core/src/airflow/ui/src/queries/useNoteEditor.ts diff --git a/airflow-core/src/airflow/ui/public/i18n/locales/en/common.json b/airflow-core/src/airflow/ui/public/i18n/locales/en/common.json index 32f9988a23963..3aaa5115bb2f5 100644 --- a/airflow-core/src/airflow/ui/public/i18n/locales/en/common.json +++ b/airflow-core/src/airflow/ui/public/i18n/locales/en/common.json @@ -179,9 +179,12 @@ "note": { "add": "Add a note", "dagRun": "Dag Run Note", + "edit": "Edit note", "label": "Note", "placeholder": "Add a note...", - "taskInstance": "Task Instance Note" + "preview": "Preview", + "taskInstance": "Task Instance Note", + "write": "Write" }, "overallStatus": "Overall Status", "partitionedDagRun_one": "Partitioned Dag Run", diff --git a/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx b/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx deleted file mode 100644 index 2305a45abe95c..0000000000000 --- a/airflow-core/src/airflow/ui/src/components/EditableMarkdownArea.tsx +++ /dev/null @@ -1,115 +0,0 @@ -/*! - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -import { Box, VStack, Editable, Text } from "@chakra-ui/react"; -import type { ChangeEvent } from "react"; -import { useState, useRef } from "react"; -import type { Components } from "react-markdown"; - -import ReactMarkdown from "./ReactMarkdown"; - -const EditableMarkdownArea = ({ - autoSize = false, - components, - mdContent, - onBlur, - onFocus, - padding = 4, - placeholder, - setMdContent, -}: { - readonly autoSize?: boolean; - readonly components?: Partial; - readonly mdContent?: string | null; - readonly onBlur?: () => void; - readonly onFocus?: () => void; - readonly padding?: number; - readonly placeholder?: string | null; - readonly setMdContent: (value: string) => void; -}) => { - const [currentValue, setCurrentValue] = useState(mdContent ?? ""); - const prevMdContentRef = useRef(mdContent); - const textareaRef = useRef(null); - - // Sync local state with prop changes (e.g. revert-on-error from parent) - if (mdContent !== prevMdContentRef.current) { - setCurrentValue(mdContent ?? ""); - prevMdContentRef.current = mdContent; - } - - const resizeTextarea = () => { - const el = textareaRef.current; - - if (autoSize && el !== null) { - el.style.height = "auto"; - el.style.height = `${el.scrollHeight}px`; - } - }; - - return ( - - ) => { - const { value } = event.target; - - setCurrentValue(value); - setMdContent(value); - resizeTextarea(); - }} - value={currentValue} - > - - {Boolean(currentValue) ? ( - {currentValue} - ) : ( - {placeholder} - )} - - { - // Resize on focus so the textarea fits existing content when first opened - resizeTextarea(); - onFocus?.(); - }} - overflow={autoSize ? "hidden" : "auto"} - placeholder={placeholder ?? ""} - ref={textareaRef} - resize="none" - /> - - - ); -}; - -export default EditableMarkdownArea; diff --git a/airflow-core/src/airflow/ui/src/components/EditableMarkdownButton.tsx b/airflow-core/src/airflow/ui/src/components/EditableMarkdownButton.tsx index 7d2f5e61c8e15..e75f4daa2757c 100644 --- a/airflow-core/src/airflow/ui/src/components/EditableMarkdownButton.tsx +++ b/airflow-core/src/airflow/ui/src/components/EditableMarkdownButton.tsx @@ -16,15 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import { Box, Button, Flex, Heading, VStack } from "@chakra-ui/react"; import { useState } from "react"; import { useTranslation } from "react-i18next"; -import { PiNoteBold, PiNoteBlankBold } from "react-icons/pi"; -import { IconButton, Dialog } from "src/components/ui"; -import { ResizableWrapper, MARKDOWN_DIALOG_STORAGE_KEY } from "src/components/ui/ResizableWrapper"; +import { IconButton } from "src/components/ui"; -import EditableMarkdownArea from "./EditableMarkdownArea"; +import MarkdownModal from "./MarkdownModal"; +import NoteIcon from "./NoteIcon"; const EditableMarkdownButton = ({ header, @@ -47,7 +45,6 @@ const EditableMarkdownButton = ({ const [isOpen, setIsOpen] = useState(false); const hasContent = Boolean(mdContent?.trim()); - const noteIcon = hasContent ? : ; const label = hasContent ? translate("note.label") : translate("note.add"); const handleOpen = () => { @@ -60,47 +57,18 @@ const EditableMarkdownButton = ({ return ( <> - {noteIcon} + - setIsOpen(false)} - open={isOpen} - size="md" - unmountOnExit={true} - > - - - - {header} - - - - - - - - - - - - - - - + setIsOpen(false)} + onConfirm={onConfirm} + placeholder={placeholder} + setMdContent={setMdContent} + /> ); }; diff --git a/airflow-core/src/airflow/ui/src/components/MarkdownModal.test.tsx b/airflow-core/src/airflow/ui/src/components/MarkdownModal.test.tsx new file mode 100644 index 0000000000000..383b0c7d9c173 --- /dev/null +++ b/airflow-core/src/airflow/ui/src/components/MarkdownModal.test.tsx @@ -0,0 +1,131 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import "@testing-library/jest-dom"; +import { fireEvent, render, screen } from "@testing-library/react"; +import { useState } from "react"; +import { describe, expect, it, vi } from "vitest"; + +import { Wrapper } from "src/utils/Wrapper"; + +import MarkdownModal, { MAX_NOTE_LENGTH } from "./MarkdownModal"; + +const defaultProps = { + header: "Note", + isOpen: true, + isPending: false, + mdContent: "An existing note", + onClose: vi.fn(), + onConfirm: vi.fn(), + placeholder: "Add a note...", + setMdContent: vi.fn(), +}; + +const renderModal = (props: Partial = {}) => + render(, { wrapper: Wrapper }); + +describe("MarkdownModal", () => { + it("shows rendered markdown (read-only) with an edit toggle for an existing note", () => { + renderModal(); + expect(screen.getByText("An existing note", { selector: "p" })).toBeInTheDocument(); + expect(screen.queryByTestId("markdown-input")).toBeNull(); + expect(screen.getByTestId("edit-markdown")).toBeInTheDocument(); + }); + + it("reveals the textarea when the edit toggle is clicked", () => { + renderModal(); + fireEvent.click(screen.getByTestId("edit-markdown")); + expect(screen.getByTestId("markdown-input")).toBeInTheDocument(); + }); + + it("opens straight into editing when there is no content", () => { + renderModal({ mdContent: "" }); + expect(screen.getByTestId("markdown-input")).toBeInTheDocument(); + }); + + it("calls setMdContent as the textarea value changes", () => { + const setMdContent = vi.fn(); + + renderModal({ mdContent: "", setMdContent }); + fireEvent.change(screen.getByTestId("markdown-input"), { target: { value: "new content" } }); + expect(setMdContent).toHaveBeenCalledWith("new content"); + }); + + describe("character limit", () => { + it("caps the textarea at the maximum length", () => { + renderModal({ mdContent: "" }); + expect(screen.getByTestId("markdown-input")).toHaveAttribute("maxlength", String(MAX_NOTE_LENGTH)); + }); + + it("shows the live character count and keeps saving enabled under the limit", () => { + renderModal({ mdContent: "hello" }); + fireEvent.click(screen.getByTestId("edit-markdown")); + expect(screen.getByText(`5/${MAX_NOTE_LENGTH}`)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /confirm/iu })).toBeEnabled(); + }); + + it("keeps saving enabled at exactly the limit", () => { + renderModal({ mdContent: "x".repeat(MAX_NOTE_LENGTH) }); + fireEvent.click(screen.getByTestId("edit-markdown")); + expect(screen.getByText(`${MAX_NOTE_LENGTH}/${MAX_NOTE_LENGTH}`)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /confirm/iu })).toBeEnabled(); + }); + + it("shows the count and disables saving when over the limit", () => { + renderModal({ mdContent: "x".repeat(MAX_NOTE_LENGTH + 1) }); + fireEvent.click(screen.getByTestId("edit-markdown")); + expect(screen.getByText(`${MAX_NOTE_LENGTH + 1}/${MAX_NOTE_LENGTH}`)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: /confirm/iu })).toBeDisabled(); + }); + }); + + it("toggles between the textarea and a rendered preview while editing", () => { + // The modal is controlled, so drive mdContent from a stateful wrapper. + const ControlledModal = () => { + const [value, setValue] = useState(""); + + return ; + }; + + render(, { wrapper: Wrapper }); + + // Starts in the editor + fireEvent.change(screen.getByTestId("markdown-input"), { target: { value: "**bold**" } }); + fireEvent.click(screen.getByTestId("preview-toggle")); + + // Preview hides the textarea and renders the markdown + expect(screen.queryByTestId("markdown-input")).toBeNull(); + expect(screen.getByText("bold", { selector: "strong" })).toBeInTheDocument(); + + // Toggling back returns to the editor + fireEvent.click(screen.getByTestId("preview-toggle")); + expect(screen.getByTestId("markdown-input")).toBeInTheDocument(); + }); + + it("confirms and closes when save is clicked", () => { + const onClose = vi.fn(); + const onConfirm = vi.fn(); + + renderModal({ mdContent: "valid note", onClose, onConfirm }); + fireEvent.click(screen.getByTestId("edit-markdown")); + fireEvent.click(screen.getByRole("button", { name: /confirm/iu })); + + expect(onConfirm).toHaveBeenCalledTimes(1); + expect(onClose).toHaveBeenCalledTimes(1); + }); +}); diff --git a/airflow-core/src/airflow/ui/src/components/MarkdownModal.tsx b/airflow-core/src/airflow/ui/src/components/MarkdownModal.tsx new file mode 100644 index 0000000000000..641dfc1d918ba --- /dev/null +++ b/airflow-core/src/airflow/ui/src/components/MarkdownModal.tsx @@ -0,0 +1,160 @@ +/*! + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { Box, Button, Flex, Heading, HStack, Text, Textarea, VStack } from "@chakra-ui/react"; +import type { ChangeEvent } from "react"; +import { useState } from "react"; +import { useTranslation } from "react-i18next"; +import { FiEdit, FiEye } from "react-icons/fi"; + +import NoteIcon from "src/components/NoteIcon"; +import ReactMarkdown from "src/components/ReactMarkdown"; +import { Dialog } from "src/components/ui"; +import { ResizableWrapper, MARKDOWN_DIALOG_STORAGE_KEY } from "src/components/ui/ResizableWrapper"; + +export const MAX_NOTE_LENGTH = 1000; + +const MarkdownModal = ({ + header, + isOpen, + isPending, + mdContent, + onClose, + onConfirm, + placeholder, + setMdContent, +}: { + readonly header: string; + readonly isOpen: boolean; + readonly isPending: boolean; + readonly mdContent?: string | null; + readonly onClose: () => void; + readonly onConfirm: () => void; + readonly placeholder: string; + readonly setMdContent: (value: string) => void; +}) => { + const { t: translate } = useTranslation("common"); + + const hasContent = Boolean(mdContent?.trim()); + // Open straight into editing when there's nothing to read; otherwise show the + // rendered note (links clickable) and let the edit button reveal the textarea. + const [isEditing, setIsEditing] = useState(!hasContent); + // While editing, toggle between the textarea and a rendered preview. + const [showPreview, setShowPreview] = useState(false); + + const length = mdContent?.length ?? 0; + // Existing notes may already exceed the limit (created before validation or + // via the API); block saving until trimmed rather than silently truncating. + const isOverLimit = length > MAX_NOTE_LENGTH; + + // Textarea only while editing and not previewing; otherwise show the rendered markdown. + const showEditor = isEditing && !showPreview; + const renderedContent = hasContent ? ( + {mdContent ?? ""} + ) : ( + {placeholder} + ); + + return ( + + + + + {header} + + + + + {showEditor ? ( +