bbovenzi commented on code in PR #76083:
URL: https://github.com/apache/airflow/pull/76083#discussion_r4515139927
##########
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:
Agreed on both, and I checked #69693 — it added the spacer for exactly the
overlay-scrollbar case, so the reveal had the same gap. Done in 6f451f05351:
the reserve is now the full spacer width always (`SCROLLBAR_SPACER_WIDTH_PX`,
widened to `PINNED_RUN_BUTTONS_WIDTH_PX` with the pager), not just when the
buttons are shown, and `Grid` derives the spacer and the reveal from one value
so they stay in step. The test now covers both widths — pager in LTR,
spacer-only in RTL — and both fail without the subtraction.
One thing I did not verify: I have not measured the collapse button's ~8px
overlap myself, so that part rests on your measurement. 16px covers it if it is
8.
_🤖 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]
