This is an automated email from the ASF dual-hosted git repository.
bbovenzi pushed a commit to branch v3-4-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v3-4-test by this push:
new fa6b47b160d [v3-4-test] Clean up Dag Version Tab UX (#75244) (#75318)
fa6b47b160d is described below
commit fa6b47b160d3ebc34bea37603b78c8efd8c992a8
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Oct 22 18:26:59 2026 -0400
[v3-4-test] Clean up Dag Version Tab UX (#75244) (#75318)
* Wrap long values in Dag version diff table so After stays visible
The Versions tab compares two Dag versions in a table whose path,
before and after cells show serialized values of up to 120 characters.
These are mostly JSON or long paths with no spaces, so the browser
could not wrap them. The table grew far wider than its container
(2146px in a 1100px window), and since DataTable has no horizontal
scroll, the rightmost After column was clipped out of view. Narrower
windows made it worse.
Let those cells wrap with overflow-wrap: anywhere and
white-space: pre-wrap. overflow-wrap: break-word is not enough: it
does not lower the cell's min-content width, so the table would still
overflow. The trade-off is that short paths can break mid-word in
narrow windows.
* UI: Read a Dag version diff's values as JSON, not as a cut string
A change to a Dag version carries the serialized value on each side, and
a whole task added or removed carries its entire payload. Flattened into
a cell and cut at 120 characters, those values said little more than
that something had changed; as a JSON preview they name the keys that
differ and open in place when the rest matters.
The cells were also set as code, whose surface sits on the table's own
striped rows at nearly the same shade, and a path was free to break
after any character, so a column squeezed by its neighbours read
vertically, one glyph per line. A side the change does not have now
leaves its cell empty, since a dash reads as a value the version holds.
Carrying a preview on each side of a comparison asks two things of the
rendered JSON field that its other callers never did. It sized itself
for one payload per row, where one long entry — a dotted class path, a
URL — could widen its column past the pane whatever bound it was given,
since a badge sets its value on one line. And its copy button, shown
outright, competes with the values for attention on every row it
appears in; waiting for the pointer, or for focus so a keyboard can
still reach it, keeps a list to its content. Both now hold wherever a
JSON field is rendered.
What the comparison says about itself was spread over three stacked
rows above the table, and the warning that a bound had cut it short sat
nowhere near the field that sets that bound — the one control that can
lift it. The summary now heads the table's own header row, beside the
columns menu, and the bound carries its own notice.
The tab's components were sharing two files, which left the table, its
cells and the page's query in the same place as whichever one was being
read.
---------
(cherry picked from commit 5c021ce54423c022541fdcf72da2537adbf1a1bd)
Co-authored-by: Brent Bovenzi <[email protected]>
Co-authored-by: jedcunningham
<[email protected]>
Co-authored-by: Claude <[email protected]>
---
.../ui/src/components/JsonPreviewBadges.tsx | 42 ++++++--
.../ui/src/components/RenderedJsonField.test.tsx | 19 ++++
.../ui/src/components/RenderedJsonField.tsx | 36 +++++--
.../pages/Dag/Versions/DiffUnavailableAlert.tsx | 44 ++++++++
.../ui/src/pages/Dag/Versions/DiffValue.tsx | 60 +++++++++++
.../pages/Dag/Versions/MaxChangesField.test.tsx | 71 ++++++++++++
.../ui/src/pages/Dag/Versions/MaxChangesField.tsx | 119 +++++++++++++++++++++
.../airflow/ui/src/pages/Dag/Versions/MonoText.tsx | 27 +++++
.../airflow/ui/src/pages/Dag/Versions/PathText.tsx | 49 +++++++++
.../pages/Dag/Versions/SelectedVersionsDiff.tsx | 81 ++++++++++++++
.../ui/src/pages/Dag/Versions/VersionDiff.test.tsx | 71 +++++++++---
.../ui/src/pages/Dag/Versions/VersionDiff.tsx | 116 ++++++--------------
.../airflow/ui/src/pages/Dag/Versions/Versions.tsx | 98 ++---------------
13 files changed, 629 insertions(+), 204 deletions(-)
diff --git a/airflow-core/src/airflow/ui/src/components/JsonPreviewBadges.tsx
b/airflow-core/src/airflow/ui/src/components/JsonPreviewBadges.tsx
index eab98a331a9..d0ec743570c 100644
--- a/airflow-core/src/airflow/ui/src/components/JsonPreviewBadges.tsx
+++ b/airflow-core/src/airflow/ui/src/components/JsonPreviewBadges.tsx
@@ -24,6 +24,9 @@ import { Tooltip } from "src/system-components";
import type { JsonPreviewEntry } from "src/utils/jsonPreview";
const MAX_VALUE_CHARS = 40;
+// Below this a shortened value says nothing, so a narrow cap spends its width
on fewer badges
+// rather than on unreadable ones.
+const MIN_VALUE_CHARS = 12;
// Widest advance of one glyph in the 12px badge font, measured across real
payloads. Estimating
// high costs a little column width; estimating low wraps the preview onto a
second line.
const CHAR_WIDTH = 7.3;
@@ -36,10 +39,12 @@ const MAX_WIDTH = 380;
type Props = {
readonly entries: Array<JsonPreviewEntry>;
+ /** Lower cap for a table that carries more than one of these side by side.
*/
+ readonly maxWidth?: number;
readonly onExpand: () => void;
};
-export const JsonPreviewBadges = ({ entries, onExpand }: Props) => {
+export const JsonPreviewBadges = ({ entries, maxWidth = MAX_WIDTH, onExpand }:
Props) => {
const { t: translate } = useTranslation(["components", "common"]);
const previews = entries.map((entry) => {
@@ -47,7 +52,12 @@ export const JsonPreviewBadges = ({ entries, onExpand }:
Props) => {
entry.itemCount === undefined
? entry.value.replaceAll(/\s+/gu, " ")
: translate("jsonPreview.items", { count: entry.itemCount });
- const text = full.length > MAX_VALUE_CHARS ? `${full.slice(0,
MAX_VALUE_CHARS)}…` : full;
+ // A badge sets its value on one line, so one long entry — a dotted class
path, a URL — would
+ // carry the whole preview past the cap on its own, whatever the loop
below decides to show.
+ const labelChars = entry.label === undefined ? 0 : entry.label.length + 1;
+ const roomChars = Math.floor((maxWidth - BADGE_CHROME) / CHAR_WIDTH) -
labelChars;
+ const valueChars = Math.max(MIN_VALUE_CHARS, Math.min(MAX_VALUE_CHARS,
roomChars));
+ const text = full.length > valueChars ? `${full.slice(0, valueChars)}…` :
full;
// A summarised or shortened value only hints at what it stands for, so
the editor still has
// something to add even when every entry is on screen.
@@ -67,7 +77,7 @@ export const JsonPreviewBadges = ({ entries, onExpand }:
Props) => {
if (
visibleEntries.length > 0 &&
- usedWidth + badgeWidth + (needsButton ? MORE_BUTTON_WIDTH : 0) >
MAX_WIDTH
+ usedWidth + badgeWidth + (needsButton ? MORE_BUTTON_WIDTH : 0) > maxWidth
) {
break;
}
@@ -79,19 +89,39 @@ export const JsonPreviewBadges = ({ entries, onExpand }:
Props) => {
const hiddenCount = previews.length - visibleEntries.length;
// Nothing left to reveal means nothing to expand.
const canExpand = hiddenCount > 0 || visibleEntries.some((preview) =>
preview.isElided);
- const previewWidth = Math.min(usedWidth + (canExpand ? MORE_BUTTON_WIDTH :
0), MAX_WIDTH);
+ const previewWidth = Math.min(usedWidth + (canExpand ? MORE_BUTTON_WIDTH :
0), maxWidth);
+ // The expand button shares the line, so a badge that filled the cap on its
own would still carry
+ // the preview — and the column around it — past that cap.
+ const badgeMaxWidth = maxWidth - (canExpand ? MORE_BUTTON_WIDTH : 0);
return (
<HStack data-testid="json-preview-badges" gap={1}
minW={`${previewWidth}px`}>
{visibleEntries.map(({ full, id, isComplex, label, text }) => (
<Tooltip content={full} disabled={text === full} key={id} portalled>
- <Badge colorPalette="gray" gap={1} size="sm" variant="surface"
whiteSpace="nowrap">
+ {/* The character budget above only estimates how wide a badge will
be, and a font that
+ runs wider than the estimate would carry the badge — and the
column holding it — past
+ the cap. maxW is what actually holds the line. */}
+ <Badge
+ colorPalette="gray"
+ gap={1}
+ maxW={`${badgeMaxWidth}px`}
+ overflow="hidden"
+ size="sm"
+ variant="surface"
+ whiteSpace="nowrap"
+ >
{label === undefined ? undefined : (
<Text as="span" color="fg.muted">
{label}:
</Text>
)}
- <Text as="span" color={isComplex ? "fg.muted" : "fg"}
fontFamily="mono">
+ <Text
+ as="span"
+ color={isComplex ? "fg.muted" : "fg"}
+ fontFamily="mono"
+ overflow="hidden"
+ textOverflow="ellipsis"
+ >
{text}
</Text>
</Badge>
diff --git
a/airflow-core/src/airflow/ui/src/components/RenderedJsonField.test.tsx
b/airflow-core/src/airflow/ui/src/components/RenderedJsonField.test.tsx
index 4b1557f6f61..228719ab254 100644
--- a/airflow-core/src/airflow/ui/src/components/RenderedJsonField.test.tsx
+++ b/airflow-core/src/airflow/ui/src/components/RenderedJsonField.test.tsx
@@ -101,6 +101,25 @@ describe("RenderedJsonField", () => {
expect(container).toBeEmptyDOMElement();
});
+ it.each([
+ ["a collapsed preview", true],
+ ["the open editor", false],
+ ])("keeps the copy button out of the way in %s until hover or focus",
(_label, collapsed) => {
+ // One copy button per row reads as chrome. It stays mounted rather than
conditionally
+ // rendered, so it keeps its place in the tab order for a keyboard.
+ render(<RenderedJsonField collapsed={collapsed} content={{ score: 0.99 }}
/>, { wrapper: Wrapper });
+
+ expect(screen.getByTestId("json-clipboard-reveal")).toBeInTheDocument();
+ });
+
+ it("renders no copy button at all when the caller turns the clipboard off",
() => {
+ render(<RenderedJsonField collapsed content={{ score: 0.99 }}
enableClipboard={false} />, {
+ wrapper: Wrapper,
+ });
+
+
expect(screen.queryByTestId("json-clipboard-reveal")).not.toBeInTheDocument();
+ });
+
it("renders the editor when the caller never collapses it", () => {
render(<RenderedJsonField content={{ score: 0.99 }} />, { wrapper: Wrapper
});
diff --git a/airflow-core/src/airflow/ui/src/components/RenderedJsonField.tsx
b/airflow-core/src/airflow/ui/src/components/RenderedJsonField.tsx
index 87f07110dd9..e9d66013300 100644
--- a/airflow-core/src/airflow/ui/src/components/RenderedJsonField.tsx
+++ b/airflow-core/src/airflow/ui/src/components/RenderedJsonField.tsx
@@ -48,11 +48,20 @@ type Props = {
readonly enableClipboard?: boolean;
/** Replaces expanding in place — for a preview whose full value lives on
another page. */
readonly onExpand?: () => void;
+ /** Narrows the collapsed preview, for a table that carries more than one of
these per row. */
+ readonly previewMaxWidth?: number;
} & Omit<FlexProps, "content">;
type EditorInstance = Parameters<OnMount>[0];
-const RenderedJsonField = ({ collapsed, content, enableClipboard = true,
onExpand, ...rest }: Props) => {
+const RenderedJsonField = ({
+ collapsed,
+ content,
+ enableClipboard = true,
+ onExpand,
+ previewMaxWidth,
+ ...rest
+}: Props) => {
const contentFormatted = JSON.stringify(content, undefined, 2);
const { t: translate } = useTranslation("common");
const { beforeMount, theme } = useMonacoTheme();
@@ -113,23 +122,36 @@ const RenderedJsonField = ({ collapsed, content,
enableClipboard = true, onExpan
return undefined;
}
+ // A copy button on every row reads as chrome, so it waits for the pointer.
It stays mounted
+ // rather than conditionally rendered, to keep its place in the tab order
for a keyboard.
const clipboardButton = enableClipboard ? (
- <ClipboardRoot value={contentFormatted}>
- <ClipboardIconButton h={7} minW={7} />
- </ClipboardRoot>
+ <Box
+ _focusWithin={{ opacity: 1 }}
+ _groupHover={{ opacity: 1 }}
+ data-testid="json-clipboard-reveal"
+ opacity={0}
+ >
+ <ClipboardRoot value={contentFormatted}>
+ <ClipboardIconButton h={7} minW={7} />
+ </ClipboardRoot>
+ </Box>
) : undefined;
if (showBadges) {
return (
- <Flex alignItems="center" flex={1} gap={1} minW={`${MIN_WIDTH}px`}
{...rest}>
- <JsonPreviewBadges entries={previewEntries} onExpand={onExpand ?? (()
=> setIsExpanded(true))} />
+ <Flex alignItems="center" className="group" flex={1} gap={1}
minW={`${MIN_WIDTH}px`} {...rest}>
+ <JsonPreviewBadges
+ entries={previewEntries}
+ maxWidth={previewMaxWidth}
+ onExpand={onExpand ?? (() => setIsExpanded(true))}
+ />
{clipboardButton}
</Flex>
);
}
return (
- <Flex flex={1} gap={2} minW={`${MIN_WIDTH}px`} {...rest}>
+ <Flex className="group" flex={1} gap={2} minW={`${MIN_WIDTH}px`} {...rest}>
<Box flex={1} minW={`${editorWidth}px`} ref={containerRef}>
<Editor
beforeMount={beforeMount}
diff --git
a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/DiffUnavailableAlert.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/DiffUnavailableAlert.tsx
new file mode 100644
index 00000000000..4985f0e19f4
--- /dev/null
+++
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/DiffUnavailableAlert.tsx
@@ -0,0 +1,44 @@
+/*!
+ * 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 { Alert, Code } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+
+type DiffUnavailableAlertProps = {
+ readonly reason?: string | null;
+};
+
+export const DiffUnavailableAlert = ({ reason }: DiffUnavailableAlertProps) =>
{
+ const { t: translate } = useTranslation("dag");
+
+ return (
+ <Alert.Root status="info" title={translate("versions.unavailable.title")}>
+ <Alert.Description>
+ {reason === null || reason === undefined ? (
+ translate("versions.unavailable.withoutReason")
+ ) : (
+ <>
+ {translate("versions.unavailable.description")}{" "}
+ {/* A machine token, so it is shown as code rather than folded
into a translated sentence. */}
+ <Code fontSize="sm">{reason}</Code>
+ </>
+ )}
+ </Alert.Description>
+ </Alert.Root>
+ );
+};
diff --git a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/DiffValue.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/DiffValue.tsx
new file mode 100644
index 00000000000..0114e2a6b16
--- /dev/null
+++ b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/DiffValue.tsx
@@ -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 RenderedJsonField from "src/components/RenderedJsonField";
+
+import { MonoText } from "./MonoText";
+
+// A scalar can still be long — a templated command, a doc string — and
nothing below it is worth
+// the vertical space of a whole cell.
+const MAX_VALUE_LENGTH = 120;
+
+// This table is the one place with two JSON previews per row, and the
component's own cap leaves
+// them demanding more width than the pane has. The rest of a value is one
expand away.
+const PREVIEW_MAX_WIDTH = 220;
+
+type DiffValueProps = {
+ readonly value: unknown;
+};
+
+export const DiffValue = ({ value }: DiffValueProps) => {
+ // A side the change does not have is left blank: a dash would read as a
value the version holds.
+ if (value === undefined) {
+ return undefined;
+ }
+
+ // A whole task added or removed carries its entire serialized payload. The
JSON field names its
+ // top-level keys in badges and opens the editor in place, where a flattened
string could only be
+ // cut short. An empty object or array has no entries to preview, so it
falls through to the text
+ // below and shows as `{}` or `[]`.
+ if (typeof value === "object" && value !== null && Object.keys(value).length
> 0) {
+ return <RenderedJsonField collapsed content={value}
previewMaxWidth={PREVIEW_MAX_WIDTH} />;
+ }
+
+ // A stored null is a value; only an absent side gets the blank cell above.
An empty string is
+ // quoted so it reads as a value rather than as a blank cell.
+ const text = typeof value === "string" ? value || '""' :
JSON.stringify(value);
+
+ // Serialized values have no spaces to break at, so without this a single
cell widens the table
+ // past the pane and the last ("after") column is clipped.
+ return (
+ <MonoText overflowWrap="anywhere">
+ {text.length > MAX_VALUE_LENGTH ? `${text.slice(0, MAX_VALUE_LENGTH)}…`
: text}
+ </MonoText>
+ );
+};
diff --git
a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/MaxChangesField.test.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/MaxChangesField.test.tsx
new file mode 100644
index 00000000000..b932d7f64de
--- /dev/null
+++
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/MaxChangesField.test.tsx
@@ -0,0 +1,71 @@
+/*!
+ * 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/vitest";
+import { act, fireEvent, render, screen } from "@testing-library/react";
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+import { Wrapper } from "src/utils/Wrapper";
+
+import { MaxChangesField } from "./MaxChangesField";
+
+const revealTruncationNotice = async () => {
+ const warning = screen.getByTestId("max-changes-truncated");
+
+ await act(async () => {
+ fireEvent.focus(warning);
+ fireEvent.pointerEnter(warning);
+ await vi.advanceTimersByTimeAsync(500);
+ });
+};
+
+// The test harness passes translation keys through untranslated, so
assertions on chrome use the
+// key and assertions on content use what the caller passed.
+describe("MaxChangesField", () => {
+ afterEach(() => {
+ vi.useRealTimers();
+ });
+
+ it("says nothing about a limit while the comparison fits inside it", () => {
+ render(<MaxChangesField maxChanges={10} onCommit={vi.fn()}
truncated={false} />, { wrapper: Wrapper });
+
+
expect(screen.queryByTestId("max-changes-truncated")).not.toBeInTheDocument();
+ });
+
+ it("keeps the truncation notice on the field, behind the warning", async ()
=> {
+ // A banner over the table repeated itself on every comparison and sat
nowhere near the bound
+ // that caused it; the field is the one control that can lift it.
+ vi.useFakeTimers();
+ render(<MaxChangesField maxChanges={10} onCommit={vi.fn()} truncated />, {
wrapper: Wrapper });
+
+
expect(screen.queryByText("versions.limitReached.description")).not.toBeInTheDocument();
+
+ await revealTruncationNotice();
+
+
expect(screen.getByRole("tooltip")).toHaveTextContent("versions.limitReached.description");
+ });
+
+ it("names the endpoint's own limit when no bound was set", async () => {
+ vi.useFakeTimers();
+ render(<MaxChangesField onCommit={vi.fn()} truncated />, { wrapper:
Wrapper });
+
+ await revealTruncationNotice();
+
+
expect(screen.getByRole("tooltip")).toHaveTextContent("versions.limitReached.descriptionDefault");
+ });
+});
diff --git
a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/MaxChangesField.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/MaxChangesField.tsx
new file mode 100644
index 00000000000..d646758fb8e
--- /dev/null
+++ b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/MaxChangesField.tsx
@@ -0,0 +1,119 @@
+/*!
+ * 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 { Stack, Text, VStack } from "@chakra-ui/react";
+import { useTranslation } from "react-i18next";
+import { FiAlertTriangle } from "react-icons/fi";
+import { useDebouncedCallback } from "use-debounce";
+
+import { NumberInputField, NumberInputRoot, Tooltip } from
"src/system-components";
+
+// Mirrors MAX_ALLOWED_CHANGES on the diff endpoint, which is the real guard:
this only spares
+// the caller a rejected request, so a stale value costs a 422 the error alert
explains.
+export const MAX_CHANGES_LIMIT = 5000;
+// Mirrors DEFAULT_MAX_CHANGES on the diff endpoint, so the truncation notice
can name the limit
+// that cut the comparison short when the caller left the field empty.
+const DEFAULT_MAX_CHANGES = 500;
+// Every distinct bound is its own request, and each one re-reads and compares
two whole serialized
+// Dags, so a keystroke is too cheap a trigger.
+const MAX_CHANGES_DEBOUNCE_MS = 400;
+
+type MaxChangesFieldProps = {
+ /** The bound in force; undefined means the endpoint's own default applied.
*/
+ readonly maxChanges?: number;
+ readonly onCommit: (value: string) => void;
+ /** Whether the comparison in view was cut short by that bound. */
+ readonly truncated: boolean;
+};
+
+export const MaxChangesField = ({ maxChanges, onCommit, truncated }:
MaxChangesFieldProps) => {
+ const { t: translate } = useTranslation("dag");
+ // The field keeps what was typed while the debounced write settles; the URL
only ever holds a
+ // bound the endpoint accepts.
+ const [draft, setDraft] = useState(maxChanges === undefined ? "" :
`${maxChanges}`);
+ const write = useDebouncedCallback(onCommit, MAX_CHANGES_DEBOUNCE_MS);
+
+ // Explicit because the number input otherwise flags an empty field as out
of range (an empty
+ // value parses to NaN), which drew an error border around the field's
normal, unset state.
+ const truncationTitle = translate("versions.limitReached.title", {
+ limit: maxChanges ?? DEFAULT_MAX_CHANGES,
+ });
+
+ const draftNumber = Number(draft);
+ const invalid =
+ draft !== "" && (!Number.isInteger(draftNumber) || draftNumber < 1 ||
draftNumber > MAX_CHANGES_LIMIT);
+
+ return (
+ <VStack alignItems="flex-start" gap={1}>
+ <Stack alignItems="center" direction="row" gap={1}>
+ <Text fontSize="xs">{translate("versions.maxChanges")}</Text>
+ {/* The bound that cut the comparison short is the one control that
can lift it, so the
+ notice sits on the field rather than as a banner over the table it
describes. */}
+ {truncated ? (
+ <Tooltip
+ content={
+ <Stack gap={1}>
+ <Text fontSize="sm" fontWeight="bold">
+ {truncationTitle}
+ </Text>
+ <Text fontSize="sm">
+ {translate(
+ maxChanges === undefined
+ ? "versions.limitReached.descriptionDefault"
+ : "versions.limitReached.description",
+ )}
+ </Text>
+ </Stack>
+ }
+ contentProps={{ maxW: "320px" }}
+ portalled
+ >
+ <Text
+ aria-label={truncationTitle}
+ as="button"
+ color="warning.fg"
+ data-testid="max-changes-truncated"
+ display="flex"
+ >
+ <FiAlertTriangle />
+ </Text>
+ </Tooltip>
+ ) : undefined}
+ </Stack>
+ <NumberInputRoot
+ invalid={invalid}
+ max={MAX_CHANGES_LIMIT}
+ min={1}
+ onValueChange={({ value }) => {
+ setDraft(value);
+ write(value);
+ }}
+ size="sm"
+ value={draft}
+ w={28}
+ >
+ <NumberInputField
+ aria-label={translate("versions.maxChanges")}
+ placeholder={translate("versions.maxChangesDefault")}
+ />
+ </NumberInputRoot>
+ </VStack>
+ );
+};
diff --git a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/MonoText.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/MonoText.tsx
new file mode 100644
index 00000000000..188e04bb1a3
--- /dev/null
+++ b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/MonoText.tsx
@@ -0,0 +1,27 @@
+/*!
+ * 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 { Text, type TextProps } from "@chakra-ui/react";
+
+// Paths and serialized values are machine text, so they are set in mono.
Chakra's Code would add
+// its own surface on top of the table's striped rows, which leaves the two
nearly the same shade.
+export const MonoText = ({ children, ...rest }: TextProps) => (
+ <Text fontFamily="mono" fontSize="sm" whiteSpace="pre-wrap" {...rest}>
+ {children}
+ </Text>
+);
diff --git a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/PathText.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/PathText.tsx
new file mode 100644
index 00000000000..08a046fb819
--- /dev/null
+++ b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/PathText.tsx
@@ -0,0 +1,49 @@
+/*!
+ * 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 { Fragment } from "react";
+
+import { MonoText } from "./MonoText";
+
+type PathTextProps = {
+ readonly path: string;
+};
+
+// A path has no spaces, and `overflow-wrap: anywhere` would let it break
after any character,
+// which drops the column's min-content width to one glyph — the other columns
then squeeze it
+// until the path reads vertically. Offering a break after each separator
instead keeps the column
+// at least as wide as the longest segment and breaks where the path already
divides.
+export const PathText = ({ path }: PathTextProps) => {
+ let walked = "";
+
+ return (
+ <MonoText overflowWrap="break-word">
+ {path.split("/").map((segment, index) => {
+ walked += index === 0 ? segment : `/${segment}`;
+
+ return (
+ <Fragment key={walked === "" ? "/" : walked}>
+ {index === 0 ? undefined : "/"}
+ <wbr />
+ {segment}
+ </Fragment>
+ );
+ })}
+ </MonoText>
+ );
+};
diff --git
a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/SelectedVersionsDiff.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/SelectedVersionsDiff.tsx
new file mode 100644
index 00000000000..f968ef507ee
--- /dev/null
+++
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/SelectedVersionsDiff.tsx
@@ -0,0 +1,81 @@
+/*!
+ * 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 { useEffect } from "react";
+
+import { useDagVersionServiceGetDagVersionDiff } from "openapi/queries";
+
+import { ProgressBar } from "src/system-components";
+
+import { ErrorAlert } from "src/components/ErrorAlert";
+
+import { VersionDiff } from "./VersionDiff";
+
+type SelectedVersionsDiffProps = {
+ readonly baseVersionNumber: number;
+ readonly dagId: string;
+ readonly maxChanges: number | undefined;
+ /** Reports whether the bound cut this comparison short, for the field that
sets it. */
+ readonly onTruncatedChange: (truncated: boolean) => void;
+ readonly targetVersionNumber: number;
+};
+
+/** Split out so the query only exists once both versions are chosen, rather
than being disabled. */
+export const SelectedVersionsDiff = ({
+ baseVersionNumber,
+ dagId,
+ maxChanges,
+ onTruncatedChange,
+ targetVersionNumber,
+}: SelectedVersionsDiffProps) => {
+ const { data, error, isLoading } = useDagVersionServiceGetDagVersionDiff(
+ {
+ baseVersionNumber,
+ dagId,
+ maxChanges,
+ targetVersionNumber,
+ },
+ undefined,
+ // Changing the bound is a new query key; without this the table would
blank out while it loads.
+ { placeholderData: (previous) => previous },
+ );
+
+ const truncated = data?.truncated ?? false;
+
+ // Cleared on the way out as well, so a bound that cut one comparison short
does not go on
+ // warning about the next pair of versions.
+ useEffect(() => {
+ onTruncatedChange(truncated);
+
+ return () => onTruncatedChange(false);
+ }, [onTruncatedChange, truncated]);
+
+ return (
+ <>
+ <ErrorAlert error={error} />
+ <ProgressBar size="xs" visibility={isLoading ? "visible" : "hidden"} />
+ {data === undefined ? undefined : (
+ <VersionDiff
+ baseVersionNumber={baseVersionNumber}
+ diff={data}
+ targetVersionNumber={targetVersionNumber}
+ />
+ )}
+ </>
+ );
+};
diff --git
a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.test.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.test.tsx
index 87c28cffb23..18bedaded99 100644
--- a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.test.tsx
+++ b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.test.tsx
@@ -79,10 +79,10 @@ const authorized: DagVersionDiffResponse = {
values_status: "available",
};
-const renderDiff = (diff: DagVersionDiffResponse, maxChanges?: number) =>
+const renderDiff = (diff: DagVersionDiffResponse) =>
render(
<Wrapper>
- <VersionDiff baseVersionNumber={1} diff={diff} maxChanges={maxChanges}
targetVersionNumber={2} />
+ <VersionDiff baseVersionNumber={1} diff={diff} targetVersionNumber={2} />
</Wrapper>,
);
@@ -152,18 +152,12 @@ describe("VersionDiff", () => {
expect(screen.getByText("bob")).toBeInTheDocument();
});
- it("says a truncated count is a lower bound and points at the limit that was
set", () => {
- renderDiff({ ...redacted, truncated: true }, 10);
-
- expect(screen.getByText("versions.summaryAtLeast")).toBeInTheDocument();
-
expect(screen.getByText("versions.limitReached.title")).toBeInTheDocument();
-
expect(screen.getByText("versions.limitReached.description")).toBeInTheDocument();
- });
-
- it("names the default limit when no bound was set", () => {
+ it("says a truncated count is a lower bound", () => {
renderDiff({ ...redacted, truncated: true });
-
expect(screen.getByText("versions.limitReached.descriptionDefault")).toBeInTheDocument();
+ expect(screen.getByText("versions.summaryAtLeast")).toBeInTheDocument();
+ // What the bound was, and how to lift it, belongs on the field that sets
it.
+
expect(screen.queryByText("versions.limitReached.title")).not.toBeInTheDocument();
});
it("counts changes once, in the summary, not again above the table", () => {
@@ -180,7 +174,7 @@ describe("VersionDiff", () => {
expect(screen.queryByText("versions.limitReached.title")).not.toBeInTheDocument();
});
- it("marks the side an added change does not have", () => {
+ it("leaves the side an added change does not have blank", () => {
renderDiff({
...authorized,
changes: [
@@ -197,10 +191,14 @@ describe("VersionDiff", () => {
],
});
- expect(screen.getByText("—")).toBeInTheDocument();
+ const cells = within(screen.getAllByRole("row")[1] as
HTMLElement).getAllByRole("cell");
+
+ // A dash would read as a value the base version holds, rather than as a
side that is absent.
+ expect(cells[5]).toBeEmptyDOMElement();
+ expect(screen.queryByText("—")).not.toBeInTheDocument();
});
- it("truncates a value too large to read in a table cell", () => {
+ it("previews a structured value as JSON rather than flattening it into the
cell", () => {
const wholeTask = { __type: "operator", __var: { task_id: "extract",
template_fields: "x".repeat(400) } };
renderDiff({
@@ -217,7 +215,48 @@ describe("VersionDiff", () => {
],
});
- const text = screen.getByText(/^\{"__type"/u).textContent;
+ expect(screen.getByTestId("json-preview-badges")).toBeInTheDocument();
+ expect(screen.getByText("__type:")).toBeInTheDocument();
+ // The serialized payload itself stays in the editor the preview opens.
+ expect(screen.queryByText(/^\{"__type"/u)).not.toBeInTheDocument();
+ });
+
+ it("shows an empty structure as a value, since it has no entries to
preview", () => {
+ renderDiff({
+ ...authorized,
+ changes: [
+ {
+ after_value: [],
+ before_value: {},
+ category: "task",
+ impact: "execution",
+ occurrence_count: 1,
+ operation: "changed",
+ path: "/dag/tasks/extract/outlets",
+ },
+ ],
+ });
+
+ expect(screen.getByText("{}")).toBeInTheDocument();
+ expect(screen.getByText("[]")).toBeInTheDocument();
+ });
+
+ it("truncates a scalar too long to read in a table cell", () => {
+ renderDiff({
+ ...authorized,
+ changes: [
+ {
+ after_value: `s3://bucket/${"x".repeat(400)}`,
+ category: "task",
+ impact: "execution",
+ occurrence_count: 1,
+ operation: "changed",
+ path: "/dag/tasks/extract/bash_command",
+ },
+ ],
+ });
+
+ const text = screen.getByText(/^s3:\/\/bucket\//u).textContent;
expect(text.endsWith("…")).toBe(true);
expect(text.length).toBeLessThan(200);
diff --git a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.tsx
index f388e91c0c1..da395557d79 100644
--- a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.tsx
+++ b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/VersionDiff.tsx
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { Alert, Badge, Box, Code, Flex, Heading, Text } from
"@chakra-ui/react";
+import { Badge, Box, Heading, Text } from "@chakra-ui/react";
import type { TFunction } from "i18next";
import { useTranslation } from "react-i18next";
import { LuInfo } from "react-icons/lu";
@@ -28,18 +28,16 @@ import { Tooltip } from "src/system-components";
import { DataTable } from "src/components/DataTable";
import type { MetaColumn } from "src/components/DataTable/types";
+import { DiffUnavailableAlert } from "./DiffUnavailableAlert";
+import { DiffValue } from "./DiffValue";
+import { PathText } from "./PathText";
+
type VersionDiffProps = {
readonly baseVersionNumber: number;
readonly diff: DagVersionDiffResponse;
- /** The bound the request asked for; undefined means the endpoint's default
applied. */
- readonly maxChanges?: number;
readonly targetVersionNumber: number;
};
-// Mirrors DEFAULT_MAX_CHANGES on the diff endpoint, so the truncation notice
can name the limit
-// that cut the comparison short when the caller left the field empty.
-const DEFAULT_MAX_CHANGES = 500;
-
const IMPACT_COLORS: Record<string, string> = {
authorization: "orange",
execution: "red",
@@ -48,22 +46,6 @@ const IMPACT_COLORS: Record<string, string> = {
unknown: "gray",
};
-// A whole task added or removed carries its entire serialized payload, which
would otherwise
-// unfold into one unreadable table cell.
-const MAX_VALUE_LENGTH = 120;
-
-const renderValue = (value: unknown) => {
- if (value === undefined) {
- return "—";
- }
-
- // A stored null is a value; only an absent side gets the dash above. An
empty string is quoted
- // so it reads as a value rather than as a blank cell.
- const text = typeof value === "string" ? value || '""' :
JSON.stringify(value);
-
- return text.length > MAX_VALUE_LENGTH ? `${text.slice(0,
MAX_VALUE_LENGTH)}…` : text;
-};
-
type ChangeColumns = Array<MetaColumn<DagVersionDiffChangeResponse>>;
const buildValueColumns = (translate: TFunction<"dag">): ChangeColumns => [
@@ -71,13 +53,13 @@ const buildValueColumns = (translate: TFunction<"dag">):
ChangeColumns => [
accessorKey: "before_value",
// Read off the record rather than the accessor value, so a key the server
omitted stays
// distinguishable from one holding a stored null.
- cell: ({ row }) => <Code
fontSize="sm">{renderValue(row.original.before_value)}</Code>,
+ cell: ({ row }) => <DiffValue value={row.original.before_value} />,
enableSorting: false,
header: translate("versions.columns.before"),
},
{
accessorKey: "after_value",
- cell: ({ row }) => <Code
fontSize="sm">{renderValue(row.original.after_value)}</Code>,
+ cell: ({ row }) => <DiffValue value={row.original.after_value} />,
enableSorting: false,
header: translate("versions.columns.after"),
},
@@ -86,7 +68,7 @@ const buildValueColumns = (translate: TFunction<"dag">):
ChangeColumns => [
const buildColumns = (translate: TFunction<"dag">, valuesShown: boolean):
ChangeColumns => [
{
accessorKey: "path",
- cell: ({ row }) => <Code fontSize="sm">{row.original.path}</Code>,
+ cell: ({ row }) => <PathText path={row.original.path} />,
enableSorting: false,
header: translate("versions.columns.path"),
},
@@ -126,37 +108,22 @@ const buildColumns = (translate: TFunction<"dag">,
valuesShown: boolean): Change
...(valuesShown ? buildValueColumns(translate) : []),
];
-export const VersionDiff = ({
- baseVersionNumber,
- diff,
- maxChanges,
- targetVersionNumber,
-}: VersionDiffProps) => {
+export const VersionDiff = ({ baseVersionNumber, diff, targetVersionNumber }:
VersionDiffProps) => {
const { t: translate } = useTranslation("dag");
const valuesShown = diff.values_status === "available";
const schemaChanged = diff.serializer_versions.base !==
diff.serializer_versions.target;
if (diff.mode === "unavailable") {
- return (
- <Alert.Root status="info"
title={translate("versions.unavailable.title")}>
- <Alert.Description>
- {diff.unavailable_reason === null || diff.unavailable_reason ===
undefined ? (
- translate("versions.unavailable.withoutReason")
- ) : (
- <>
- {translate("versions.unavailable.description")}{" "}
- {/* A machine token, so it is shown as code rather than folded
into a translated sentence. */}
- <Code fontSize="sm">{diff.unavailable_reason}</Code>
- </>
- )}
- </Alert.Description>
- </Alert.Root>
- );
+ return <DiffUnavailableAlert reason={diff.unavailable_reason} />;
}
return (
- <Box>
- <Flex alignItems="center" gap={3} justifyContent="space-between" mb={2}>
+ <DataTable
+ columns={buildColumns(translate, valuesShown)}
+ data={diff.changes}
+ // The summary heads the table's own header row, so the comparison, what
it leaves out and
+ // the control over its columns read as one line rather than three
stacked ones.
+ headingExtra={
<Box>
<Heading size="md">
{translate("versions.heading", { base: baseVersionNumber, target:
targetVersionNumber })}
@@ -175,54 +142,31 @@ export const VersionDiff = ({
: undefined}
</Text>
</Box>
- <Flex alignItems="center" gap={2}>
+ }
+ // The summary above already counts the changes. A second "N Records"
count only invites a
+ // comparison that does not hold: a redacted record stands for every
change sharing its path.
+ hideRowCountHeading
+ modelName="dag:versions.record"
+ noRowsMessage={translate("versions.noChanges")}
+ presentationActions={
+ <>
{/* The hover says what grants values, since the viewer cannot grant
it here: the server
- decides from the caller's access to Dag code. */}
+ decides from the caller's access to Dag code. */}
{valuesShown ? undefined : (
<Tooltip content={translate("versions.codeAccess")} portalled>
<Badge
colorPalette="gray">{translate("versions.valuesHidden")}</Badge>
</Tooltip>
)}
{/* Above the table rather than below it: a long diff would
otherwise push the caveat
- off-screen exactly when it matters most. */}
+ off-screen exactly when it matters most. */}
<Tooltip content={translate("versions.observedStateBoundary")}
portalled>
<Box aria-label={translate("versions.aboutThisComparison")}
as="button" color="fg.muted" p={1}>
<LuInfo />
</Box>
</Tooltip>
- </Flex>
- </Flex>
-
- {diff.truncated ? (
- // The title is a child, not a `title` prop: Chakra's Alert.Root
passes that through as the
- // HTML attribute, so it only ever showed as a hover tooltip.
- <Alert.Root mb={2} status="warning">
- <Alert.Indicator />
- <Alert.Content>
- <Alert.Title>
- {translate("versions.limitReached.title", { limit: maxChanges ??
DEFAULT_MAX_CHANGES })}
- </Alert.Title>
- <Alert.Description>
- {translate(
- maxChanges === undefined
- ? "versions.limitReached.descriptionDefault"
- : "versions.limitReached.description",
- )}
- </Alert.Description>
- </Alert.Content>
- </Alert.Root>
- ) : undefined}
-
- <DataTable
- columns={buildColumns(translate, valuesShown)}
- data={diff.changes}
- // The summary above already counts the changes. A second "N Records"
count only invites a
- // comparison that does not hold: a redacted record stands for every
change sharing its path.
- hideRowCountHeading
- modelName="dag:versions.record"
- noRowsMessage={translate("versions.noChanges")}
- total={diff.changes.length}
- />
- </Box>
+ </>
+ }
+ total={diff.changes.length}
+ />
);
};
diff --git a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/Versions.tsx
b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/Versions.tsx
index f2f35151b27..7b97f443957 100644
--- a/airflow-core/src/airflow/ui/src/pages/Dag/Versions/Versions.tsx
+++ b/airflow-core/src/airflow/ui/src/pages/Dag/Versions/Versions.tsx
@@ -18,70 +18,16 @@
*/
import { useState } from "react";
-import { Box, Flex, Text, VStack } from "@chakra-ui/react";
+import { Box, Flex, Text } from "@chakra-ui/react";
import { useTranslation } from "react-i18next";
import { useParams, useSearchParams } from "react-router-dom";
-import { useDebouncedCallback } from "use-debounce";
-import { useDagVersionServiceGetDagVersionDiff } from "openapi/queries";
-
-import { NumberInputField, NumberInputRoot, ProgressBar } from
"src/system-components";
-
-import { ErrorAlert } from "src/components/ErrorAlert";
import { VersionCompareSelect } from "src/components/VersionCompareSelect";
import { SearchParamsKeys } from "src/constants/searchParams";
-import { VersionDiff } from "./VersionDiff";
-
-// Mirrors MAX_ALLOWED_CHANGES on the diff endpoint, which is the real guard:
this only spares
-// the caller a rejected request, so a stale value costs a 422 the error alert
explains.
-const MAX_CHANGES_LIMIT = 5000;
-// Every distinct bound is its own request, and each one re-reads and compares
two whole serialized
-// Dags, so a keystroke is too cheap a trigger.
-const MAX_CHANGES_DEBOUNCE_MS = 400;
-
-type SelectedVersionsProps = {
- readonly baseVersionNumber: number;
- readonly dagId: string;
- readonly maxChanges: number | undefined;
- readonly targetVersionNumber: number;
-};
-
-/** Split out so the query only exists once both versions are chosen, rather
than being disabled. */
-const SelectedVersionsDiff = ({
- baseVersionNumber,
- dagId,
- maxChanges,
- targetVersionNumber,
-}: SelectedVersionsProps) => {
- const { data, error, isLoading } = useDagVersionServiceGetDagVersionDiff(
- {
- baseVersionNumber,
- dagId,
- maxChanges,
- targetVersionNumber,
- },
- undefined,
- // Changing the bound is a new query key; without this the table would
blank out while it loads.
- { placeholderData: (previous) => previous },
- );
-
- return (
- <>
- <ErrorAlert error={error} />
- <ProgressBar size="xs" visibility={isLoading ? "visible" : "hidden"} />
- {data === undefined ? undefined : (
- <VersionDiff
- baseVersionNumber={baseVersionNumber}
- diff={data}
- maxChanges={maxChanges}
- targetVersionNumber={targetVersionNumber}
- />
- )}
- </>
- );
-};
+import { MAX_CHANGES_LIMIT, MaxChangesField } from "./MaxChangesField";
+import { SelectedVersionsDiff } from "./SelectedVersionsDiff";
const parsePositiveInt = (raw: string | null) => {
const parsed = Number(raw);
@@ -101,16 +47,8 @@ export const Versions = () => {
// request. An empty bound means the endpoint's own default applies.
const maxChanges =
requestedMaxChanges === undefined ? undefined :
Math.min(requestedMaxChanges, MAX_CHANGES_LIMIT);
- // The field keeps what was typed while the debounced write settles; the URL
only ever holds a
- // bound the endpoint accepts.
- const [maxChangesDraft, setMaxChangesDraft] = useState(maxChanges ===
undefined ? "" : `${maxChanges}`);
-
- // Explicit because the number input otherwise flags an empty field as out
of range (an empty
- // value parses to NaN), which drew an error border around the field's
normal, unset state.
- const draftNumber = Number(maxChangesDraft);
- const maxChangesInvalid =
- maxChangesDraft !== "" &&
- (!Number.isInteger(draftNumber) || draftNumber < 1 || draftNumber >
MAX_CHANGES_LIMIT);
+ // Only the comparison knows whether its bound cut it short, and only the
field can lift it.
+ const [truncated, setTruncated] = useState(false);
const updateParam = (key: string, value: string) =>
setSearchParams(
@@ -128,14 +66,14 @@ export const Versions = () => {
{ replace: true },
);
- const writeMaxChanges = useDebouncedCallback((value: string) => {
+ const writeMaxChanges = (value: string) => {
const parsed = parsePositiveInt(value);
updateParam(
SearchParamsKeys.MAX_CHANGES,
parsed === undefined ? "" : `${Math.min(parsed, MAX_CHANGES_LIMIT)}`,
);
- }, MAX_CHANGES_DEBOUNCE_MS);
+ };
return (
<Box p={2}>
@@ -154,26 +92,7 @@ export const Versions = () => {
}
selectedVersionNumber={targetVersionNumber}
/>
- <VStack alignItems="flex-start" gap={1}>
- <Text fontSize="xs">{translate("versions.maxChanges")}</Text>
- <NumberInputRoot
- invalid={maxChangesInvalid}
- max={MAX_CHANGES_LIMIT}
- min={1}
- onValueChange={({ value }) => {
- setMaxChangesDraft(value);
- writeMaxChanges(value);
- }}
- size="sm"
- value={maxChangesDraft}
- w={28}
- >
- <NumberInputField
- aria-label={translate("versions.maxChanges")}
- placeholder={translate("versions.maxChangesDefault")}
- />
- </NumberInputRoot>
- </VStack>
+ <MaxChangesField maxChanges={maxChanges} onCommit={writeMaxChanges}
truncated={truncated} />
</Flex>
{baseVersionNumber === undefined || targetVersionNumber === undefined ? (
@@ -183,6 +102,7 @@ export const Versions = () => {
baseVersionNumber={baseVersionNumber}
dagId={dagId}
maxChanges={maxChanges}
+ onTruncatedChange={setTruncated}
targetVersionNumber={targetVersionNumber}
/>
)}
