bbovenzi commented on code in PR #76083:
URL: https://github.com/apache/airflow/pull/76083#discussion_r4515088337
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/useGridScrollRestore.ts:
##########
@@ -16,72 +16,210 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { useLayoutEffect } from "react";
+import type { RefObject } from "react";
+import { useLayoutEffect, useRef } from "react";
import type { Virtualizer } from "@tanstack/react-virtual";
import { ROW_HEIGHT } from "./constants";
import type { GridTask } from "./utils";
-// Last Grid scrollTop per Dag. Module scope so it outlives the Grid
remounting.
-const gridScrollTops = new Map<string, number>();
+// Each axis is absent until it has actually been scrollable: a Dag whose runs
have not arrived
+// reports 0 for an axis that cannot move, and storing that 0 would read back
as a real position.
+type ScrollPosition = {
+ left?: number;
+ top?: number;
+};
+
+// Last Grid scroll offsets per Dag. Module scope so they outlive the Grid
remounting.
+const gridScrollPositions = new Map<string, ScrollPosition>();
type Params = {
dagId: string;
flatNodes: Array<GridTask>;
// Height of the sticky header above the rows (also the virtualizer's
scrollPaddingStart).
headerPad: number;
+ isRtl: boolean;
rowVirtualizer: Virtualizer<HTMLDivElement, Element>;
+ runIds: Array<string>;
selectedGroupId?: string;
+ selectedRunId?: string;
selectedTaskId?: string;
+ // Gantt view shares one scroller with the Grid, where the horizontal axis
is the Gantt's timeline.
+ sharesScroller: boolean;
+ taskNameColumnRef: RefObject<HTMLDivElement | null>;
+};
+
+// Horizontal counterpart to the virtualizer's scrollToIndex, with the same
meaning of `align`.
+//
+// Run columns scroll beneath the sticky task name column, so a column tucked
behind it is as hidden
+// as one past the far edge. The visible scrollport is clientWidth wide
(unlike the bounding rect, it
+// excludes the vertical scrollbar), minus that column: on the left in LTR, on
the right in RTL, where
+// the scrollbar also moves to the left. Returns whether the run's column was
found.
+const scrollRunIntoView = ({
+ align,
+ isRtl,
+ scrollEl,
+ selectedRunId,
+ taskNameColumn,
+}: {
+ align: "center" | "nearest";
+ isRtl: boolean;
+ scrollEl: HTMLElement;
+ selectedRunId: string | undefined;
+ taskNameColumn: HTMLDivElement | null;
+}) => {
+ if (selectedRunId === undefined) {
+ return false;
+ }
+
+ // Both the duration bar and the task instance column carry the run id;
either one gives the
+ // column's x position, so take whichever the query finds first.
+ const column =
scrollEl.querySelector(`[data-run-id="${CSS.escape(selectedRunId)}"]`);
+
+ if (!column) {
+ return false;
+ }
+
+ const columnRect = column.getBoundingClientRect();
+ const viewRect = scrollEl.getBoundingClientRect();
+ const nameColumnWidth = taskNameColumn?.getBoundingClientRect().width ?? 0;
+ const visibleStart = isRtl ? viewRect.right - scrollEl.clientWidth :
viewRect.left + nameColumnWidth;
+ const visibleEnd = isRtl ? viewRect.right - nameColumnWidth : viewRect.left
+ scrollEl.clientWidth;
Review Comment:
Good catch — fixed in 24177efe293. The reveal now pulls the visible end in
by `PINNED_RUN_BUTTONS_WIDTH_PX`, a new constant that the scrollbar spacer also
reads, so the two can't drift apart; in RTL the reserve moves to the left edge
along with the buttons. Covered by a parametrized test that fails without the
change.
_🤖 Addressed by [Claude Code](https://claude.com/claude-code)_
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
