bbovenzi commented on code in PR #76083:
URL: https://github.com/apache/airflow/pull/76083#discussion_r4515411063


##########
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:
   Followed up on the 8px, and your number is right. Taking the token values 
from the running app (`--chakra-spacing-1` = 4px, `--chakra-spacing-6` = 24px 
at a 16px root):
   
   - the handle `Box` is `w={1}` = 4px
   - the collapse `IconButton` is `size="2xs"` → `minW: 6` = 24px, and Chakra's 
`IconButton` sets `px: 0`, so that is its full width
   - `insetInlineStart="50%"` + `translate(-50%, -50%)` centres it on the 
handle, so it overhangs 10px on each side
   - the main panel's `Flex` has `borderWidth="2px"`, so 2px of that sits on 
the border and **8px** lands on the Grid's content
   
   So 16px clears it with 8px to spare. I could not drive the Grid itself here 
— the measurement is from the live stylesheet plus the component definitions, 
not from the rendered button.
   
   _🤖 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]

Reply via email to