diff --git a/airflow/www/static/js/dag/Main.tsx b/airflow/www/static/js/dag/Main.tsx index 7665d27fe35fb..248dc3b0a275f 100644 --- a/airflow/www/static/js/dag/Main.tsx +++ b/airflow/www/static/js/dag/Main.tsx @@ -190,9 +190,15 @@ const Main = () => { height={`${mainHeight}px`} overflow="hidden" position="relative" - minHeight="750px" + display="flex" + flexDirection="column" > - + { - + {isLoading || isEmpty(groups) ? ( ) : ( diff --git a/airflow/www/static/js/dag/details/EventLog.tsx b/airflow/www/static/js/dag/details/EventLog.tsx index 93f7d4a127e96..4b54c5ef0b6c0 100644 --- a/airflow/www/static/js/dag/details/EventLog.tsx +++ b/airflow/www/static/js/dag/details/EventLog.tsx @@ -19,7 +19,7 @@ /* global moment */ -import React, { useMemo, useRef, useState } from "react"; +import React, { useMemo, useState } from "react"; import { Box, Flex, @@ -43,7 +43,7 @@ import { } from "chakra-react-select"; import { useEventLogs } from "src/api"; -import { getMetaValue, useContentHeight } from "src/utils"; +import { getMetaValue } from "src/utils"; import type { DagRun } from "src/types"; import LinkButton from "src/components/LinkButton"; import type { EventLog as EventLogType } from "src/types/api-generated"; @@ -74,8 +74,6 @@ const dagId = getMetaValue("dag_id") || undefined; const columnHelper = createColumnHelper(); const EventLog = ({ taskId, run, showMapped }: Props) => { - const logRef = useRef(null); - const contentHeight = useContentHeight(logRef); const { tableURLState, setTableURLState } = useTableURLState({ sorting: [{ id: "when", desc: true }], }); @@ -214,12 +212,7 @@ const EventLog = ({ taskId, run, showMapped }: Props) => { }); return ( - + { const { data: { dagRuns, groups }, } = useGridData(); - const detailsRef = useRef(null); - const contentHeight = useContentHeight(detailsRef); const { data: dagDetailsData, isLoading: isLoadingDagDetails } = useDagDetails(); @@ -158,12 +155,7 @@ const Dag = () => { ); return ( - + {durations.length > 0 && ( diff --git a/airflow/www/static/js/dag/details/dagCode/CodeBlock.tsx b/airflow/www/static/js/dag/details/dagCode/CodeBlock.tsx index fd97c6984772c..55c52e31eaa6b 100644 --- a/airflow/www/static/js/dag/details/dagCode/CodeBlock.tsx +++ b/airflow/www/static/js/dag/details/dagCode/CodeBlock.tsx @@ -37,7 +37,8 @@ export default function CodeBlock({ code }: Props) { return ( { - const dagCodeRef = useRef(null); - const contentHeight = useContentHeight(dagCodeRef); const { data: dagData, isLoading: isLoadingDag, error: dagError } = useDag(); const { data: codeSource = "", @@ -41,9 +38,15 @@ const DagCode = () => { const error = codeError || dagError; return ( - + {dagData?.lastParsedTime && ( - + Parsed at: )} diff --git a/airflow/www/static/js/dag/details/dagRun/index.tsx b/airflow/www/static/js/dag/details/dagRun/index.tsx index 9a3b7628ff528..3c895ec93b649 100644 --- a/airflow/www/static/js/dag/details/dagRun/index.tsx +++ b/airflow/www/static/js/dag/details/dagRun/index.tsx @@ -16,11 +16,11 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useRef } from "react"; +import React from "react"; import { Box } from "@chakra-ui/react"; import { useGridData } from "src/api"; -import { getMetaValue, useContentHeight } from "src/utils"; +import { getMetaValue } from "src/utils"; import type { DagRun as DagRunType } from "src/types"; import NotesAccordion from "src/dag/details/NotesAccordion"; @@ -37,8 +37,6 @@ const DagRun = ({ runId }: Props) => { const { data: { dagRuns }, } = useGridData(); - const detailsRef = useRef(null); - const contentHeight = useContentHeight(detailsRef); const run = dagRuns.find((dr) => dr.runId === runId); @@ -46,12 +44,7 @@ const DagRun = ({ runId }: Props) => { const { runType, note } = run; return ( - + + {!runId && ( diff --git a/airflow/www/static/js/dag/details/graph/index.tsx b/airflow/www/static/js/dag/details/graph/index.tsx index 2d39dd07f36da..77422177dcd5a 100644 --- a/airflow/www/static/js/dag/details/graph/index.tsx +++ b/airflow/www/static/js/dag/details/graph/index.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { useRef, useState, useEffect, useMemo } from "react"; +import React, { useState, useEffect, useMemo } from "react"; import { Box, useTheme, Select, Text, Switch, Flex } from "@chakra-ui/react"; import ReactFlow, { ReactFlowProvider, @@ -39,7 +39,7 @@ import { useUpstreamDatasetEvents, } from "src/api"; import useSelection from "src/dag/useSelection"; -import { getMetaValue, getTask, useContentHeight } from "src/utils"; +import { getMetaValue, getTask } from "src/utils"; import { useGraphLayout } from "src/utils/graph"; import Edge from "src/components/Graph/Edge"; import type { DepNode, WebserverEdge } from "src/types"; @@ -132,7 +132,6 @@ const getUpstreamDatasets = ( }; const Graph = ({ openGroupIds, onToggleGroups, hoveredTaskState }: Props) => { - const graphRef = useRef(null); const { data } = useGraphData(); const [arrange, setArrange] = useState(data?.arrange || "LR"); const [hasRendered, setHasRendered] = useState(false); @@ -244,7 +243,6 @@ const Graph = ({ openGroupIds, onToggleGroups, hoveredTaskState }: Props) => { const { colors } = useTheme(); const { getZoom, fitView } = useReactFlow(); const latestDagRunId = dagRuns[dagRuns.length - 1]?.runId; - const contentHeight = useContentHeight(graphRef); useOnViewportChange({ onEnd: (viewport: Viewport) => { @@ -310,68 +308,61 @@ const Graph = ({ openGroupIds, onToggleGroups, hoveredTaskState }: Props) => { }); return ( - - {!!contentHeight && ( - - - - {!!datasetsCollection?.datasets?.length && ( - - - Show datasets: - - setShowDatasets(!showDatasets)} - /> - - )} - Layout: - - - - - - nodeStrokeColor(props, colors)} - nodeColor={nodeColor} - zoomable - pannable - /> - - )} + + + + + {!!datasetsCollection?.datasets?.length && ( + + + Show datasets: + + setShowDatasets(!showDatasets)} + /> + + )} + Layout: + + + + + + nodeStrokeColor(props, colors)} + nodeColor={nodeColor} + zoomable + pannable + /> + ); }; diff --git a/airflow/www/static/js/dag/details/index.tsx b/airflow/www/static/js/dag/details/index.tsx index f4cd3c502e624..90eeddf2cecc5 100644 --- a/airflow/www/static/js/dag/details/index.tsx +++ b/airflow/www/static/js/dag/details/index.tsx @@ -156,6 +156,16 @@ const indexToTab = ( export const TAB_PARAM = "tab"; +// Each panel fills the tab area and lays its content out as a column, so a single child can claim +// the leftover space and scroll on its own rather than pushing the page past the viewport. +// `overflowY` is a safety net for content that does not opt into that sizing. +const panelProps = { + height: "100%", + display: "flex", + flexDirection: "column", + overflowY: "auto", +} as const; + const Details = ({ openGroupIds, onToggleGroups, @@ -260,8 +270,13 @@ const Details = ({ : group?.instances.find((ti) => ti.runId === runId); return ( - - + +
- + - + @@ -426,8 +444,8 @@ const Details = ({ )} - - + + {isDag && } {isDagRun && } {!!runId && !!taskId && ( @@ -445,14 +463,14 @@ const Details = ({ )} {showTaskDetails && } - + - + - + - + {isDag && ( - + )} {isDag && ( - + )} {isMappedTaskSummary && ( - + )} {isTaskInstance && ( - + )} {isTaskInstance && isK8sExecutor && ( - + )} diff --git a/airflow/www/static/js/dag/details/taskInstance/BackToTaskSummary.tsx b/airflow/www/static/js/dag/details/taskInstance/BackToTaskSummary.tsx index df65cefe9dcf7..b4c365dd9970b 100644 --- a/airflow/www/static/js/dag/details/taskInstance/BackToTaskSummary.tsx +++ b/airflow/www/static/js/dag/details/taskInstance/BackToTaskSummary.tsx @@ -29,7 +29,7 @@ const BackToTaskSummary = ({ isMapIndexDefined, onClick }: Props) => { if (!isMapIndexDefined) return null; return ( - + diff --git a/airflow/www/static/js/dag/details/taskInstance/Logs/LogBlock.tsx b/airflow/www/static/js/dag/details/taskInstance/Logs/LogBlock.tsx index 4b3e62f095f42..d03ebef71cf4b 100644 --- a/airflow/www/static/js/dag/details/taskInstance/Logs/LogBlock.tsx +++ b/airflow/www/static/js/dag/details/taskInstance/Logs/LogBlock.tsx @@ -19,7 +19,6 @@ import React, { useRef, useEffect, useState } from "react"; import { Code } from "@chakra-ui/react"; -import { useContentHeight } from "src/utils"; interface Props { parsedLogs: string; @@ -37,7 +36,6 @@ const LogBlock = ({ const [autoScroll, setAutoScroll] = useState(true); const logBoxRef = useRef(null); - const contentHeight = useContentHeight(logBoxRef); const scrollToBottom = () => { if (logBoxRef.current) { @@ -47,13 +45,13 @@ const LogBlock = ({ useEffect(() => { // Always scroll to bottom when wrap or tryNumber change - if (contentHeight) scrollToBottom(); - }, [wrap, tryNumber, contentHeight]); + scrollToBottom(); + }, [wrap, tryNumber]); useEffect(() => { // When logs change, only scroll if autoScroll is enabled - if (autoScroll && contentHeight) scrollToBottom(); - }, [parsedLogs, autoScroll, contentHeight]); + if (autoScroll) scrollToBottom(); + }, [parsedLogs, autoScroll]); const onScroll = (e: React.UIEvent) => { if (e.currentTarget) { @@ -103,7 +101,8 @@ const LogBlock = ({ ref={logBoxRef} onScroll={onScroll} onClick={onClick} - maxHeight={`${contentHeight}px`} + flex={1} + minHeight={0} overflowY="auto" p={3} display="block" diff --git a/airflow/www/static/js/dag/details/taskInstance/Logs/index.tsx b/airflow/www/static/js/dag/details/taskInstance/Logs/index.tsx index 0e5d9bbfbc48c..73d13ec068582 100644 --- a/airflow/www/static/js/dag/details/taskInstance/Logs/index.tsx +++ b/airflow/www/static/js/dag/details/taskInstance/Logs/index.tsx @@ -155,7 +155,7 @@ const Logs = ({ return ( <> {showExternalLogRedirect && externalLogName && ( - + View Logs in {externalLogName} (by attempts): {Array.from({ length: finalTryNumber || 1 }, (_, i) => i + 1).map( @@ -173,7 +173,7 @@ const Logs = ({ )} - + {!!taskInstance && ( {warning} @@ -263,6 +264,7 @@ const Logs = ({ alignItems="center" p={2} mb={2} + flexShrink={0} > diff --git a/airflow/www/static/js/dag/details/taskInstance/MappedInstances.tsx b/airflow/www/static/js/dag/details/taskInstance/MappedInstances.tsx index af70b70762494..4df30ae20add9 100644 --- a/airflow/www/static/js/dag/details/taskInstance/MappedInstances.tsx +++ b/airflow/www/static/js/dag/details/taskInstance/MappedInstances.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { useState, useMemo, useRef } from "react"; +import React, { useState, useMemo } from "react"; import { Flex, Box } from "@chakra-ui/react"; import { snakeCase } from "lodash"; import type { Row, SortingRule } from "react-table"; @@ -27,7 +27,6 @@ import { useMappedInstances } from "src/api"; import { StatusWithNotes } from "src/dag/StatusBox"; import { Table, CellProps } from "src/components/Table"; import Time from "src/components/Time"; -import { useContentHeight } from "src/utils"; interface Props { dagId: string; @@ -37,8 +36,6 @@ interface Props { } const MappedInstances = ({ dagId, runId, taskId, onRowClicked }: Props) => { - const mappedTasksRef = useRef(null); - const contentHeight = useContentHeight(mappedTasksRef); const limit = 25; const [offset, setOffset] = useState(0); const [sortBy, setSortBy] = useState[]>([]); @@ -125,7 +122,7 @@ const MappedInstances = ({ dagId, runId, taskId, onRowClicked }: Props) => { ); return ( - +
{ const { data: renderedK8s } = useRenderedK8s(runId, taskId, mapIndex); - const k8sRef = useRef(null); - const contentHeight = useContentHeight(k8sRef); - if (!isK8sExecutor || !runId || !taskId) return null; return ( - + 
{YAML.stringify(renderedK8s)}
 ); diff --git a/airflow/www/static/js/dag/details/taskInstance/Xcom/index.tsx b/airflow/www/static/js/dag/details/taskInstance/Xcom/index.tsx index 0b51293edab13..a745837100614 100644 --- a/airflow/www/static/js/dag/details/taskInstance/Xcom/index.tsx +++ b/airflow/www/static/js/dag/details/taskInstance/Xcom/index.tsx @@ -17,7 +17,7 @@ * under the License. */ -import React, { useRef } from "react"; +import React from "react"; import { Table, Text, @@ -31,7 +31,6 @@ import { import type { Dag, DagRun, TaskInstance } from "src/types"; import { useTaskXcomCollection } from "src/api"; -import { useContentHeight } from "src/utils"; import ErrorAlert from "src/components/ErrorAlert"; import XcomEntry from "./XcomEntry"; @@ -51,9 +50,6 @@ const XcomCollection = ({ mapIndex, tryNumber, }: Props) => { - const taskXcomRef = useRef(null); - const contentHeight = useContentHeight(taskXcomRef); - const { data: xcomCollection, isLoading, @@ -67,12 +63,7 @@ const XcomCollection = ({ }); return ( - + {isLoading && } {xcomCollection && diff --git a/airflow/www/static/js/dag/details/taskInstance/index.tsx b/airflow/www/static/js/dag/details/taskInstance/index.tsx index 7ef972ee8082c..d7e5c4fc83dd1 100644 --- a/airflow/www/static/js/dag/details/taskInstance/index.tsx +++ b/airflow/www/static/js/dag/details/taskInstance/index.tsx @@ -17,11 +17,11 @@ * under the License. */ -import React, { useRef } from "react"; +import React from "react"; import { Box } from "@chakra-ui/react"; import { useGridData, useTaskInstance } from "src/api"; -import { getMetaValue, getTask, useContentHeight } from "src/utils"; +import { getMetaValue, getTask } from "src/utils"; import type { DagRun, TaskInstance as GridTaskInstance } from "src/types"; import NotesAccordion from "src/dag/details/NotesAccordion"; @@ -42,8 +42,6 @@ interface Props { } const TaskInstance = ({ taskId, runId, mapIndex }: Props) => { - const taskInstanceRef = useRef(null); - const contentHeight = useContentHeight(taskInstanceRef); const isMapIndexDefined = !(mapIndex === undefined); const { data: { dagRuns, groups }, @@ -77,12 +75,7 @@ const TaskInstance = ({ taskId, runId, mapIndex }: Props) => { const gridInstance = group?.instances.find((ti) => ti.runId === runId); return ( - + {!isGroup && run?.executionDate && ( + ({ + current: { + getBoundingClientRect: () => ({ top: viewportTop }), + }, + }) as unknown as React.RefObject; + +const scrollTo = (scrollY: number) => + Object.defineProperty(window, "scrollY", { value: scrollY, writable: true }); + +describe("Test useContentHeight", () => { + beforeEach(() => { + window.innerHeight = viewportHeight; + document.body.style.paddingBottom = `${footerReserve}px`; + scrollTo(0); + }); + + test("Fills the viewport below the element, leaving the footer reserve", () => { + const { result } = renderHook(() => useContentHeight(refAt(180))); + + expect(result.current).toBe(viewportHeight - footerReserve - 180); + }); + + test("Ignores the page scroll position", () => { + // Same element, but the page is scrolled down 200px so it sits higher in the viewport. + scrollTo(200); + const { result } = renderHook(() => useContentHeight(refAt(180 - 200))); + + expect(result.current).toBe(viewportHeight - footerReserve - 180); + }); + + test("Never returns a negative height", () => { + const { result } = renderHook(() => + useContentHeight(refAt(viewportHeight)), + ); + + expect(result.current).toBe(0); + }); +}); diff --git a/airflow/www/static/js/utils/useContentHeight.ts b/airflow/www/static/js/utils/useContentHeight.ts index fbfcb8b20ef96..82d51bb5c1a30 100644 --- a/airflow/www/static/js/utils/useContentHeight.ts +++ b/airflow/www/static/js/utils/useContentHeight.ts @@ -19,14 +19,11 @@ import React, { useLayoutEffect, useState } from "react"; -// Vertical space available to a scroll container, measured down to the bottom of the nearest -// min-height-constrained ancestor — the grid's Main box, which has a min-height floor. Anchoring to -// that box (rather than the viewport) lets the content grow to fill the floored area even when the -// box is taller than the viewport and the page scrolls, instead of shrinking to the fold and -// leaving a blank strip below. When there is no such ancestor (the Main box measuring itself), it -// falls back to the viewport minus the body's bottom padding (the Flask footer reserve). Either way -// this avoids the `height: 100%` cascade, which over-claims because / have sibling -// chrome (tab headers) stacked above the panel — that cascade is what truncated the details tabs. +// Vertical space between the top of an element and the bottom of the viewport, less the body's +// bottom padding (the Flask footer reserve). The element's position is taken in document +// coordinates so the result does not change while the page is scrolled — anchoring to the viewport +// instead would feed the element's own height back into the measurement and grow it on every +// observer tick. const useContentHeight = (contentRef: React.RefObject) => { const [height, setHeight] = useState(0); @@ -34,19 +31,11 @@ const useContentHeight = (contentRef: React.RefObject) => { const update = () => { const element = contentRef.current; if (!element) return; - let container = element.parentElement; - while ( - container && - !(parseFloat(getComputedStyle(container).minHeight) > 0) - ) { - container = container.parentElement; - } - const bottom = container - ? container.getBoundingClientRect().bottom - : window.innerHeight - - (parseFloat(getComputedStyle(document.body).paddingBottom) || 0); + const documentTop = element.getBoundingClientRect().top + window.scrollY; + const footerReserve = + parseFloat(getComputedStyle(document.body).paddingBottom) || 0; const available = Math.max( - bottom - element.getBoundingClientRect().top, + window.innerHeight - footerReserve - documentTop, 0, ); // Guard against re-setting the same value (avoids ResizeObserver feedback loops).