pierrejeambrun commented on code in PR #54447:
URL: https://github.com/apache/airflow/pull/54447#discussion_r2293851940
##########
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/TaskLogContent.tsx:
##########
@@ -54,14 +54,8 @@ const ScrollToButton = ({
openDelay={100}
>
<IconButton
- _ltr={{
- left: "auto",
- right: 4,
- }}
- _rtl={{
- left: 4,
- right: "auto",
- }}
+ _ltr={{ left: "auto", right: 4 }}
+ _rtl={{ left: 4, right: "auto" }}
Review Comment:
Not needed, to change formatting there.
##########
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/TaskLogContent.tsx:
##########
@@ -94,16 +90,114 @@ export const TaskLogContent = ({ error, isLoading,
logError, parsedLogs, wrap }:
return parsedLogs.length > 1 && contentHeight > containerHeight;
}, [rowVirtualizer, parsedLogs]);
+ // Precise jump to hash index
useLayoutEffect(() => {
- if (location.hash && !isLoading) {
- rowVirtualizer.scrollToIndex(Math.min(Number(hash) + 5,
parsedLogs.length - 1));
+ if (!location.hash || isLoading || parsedLogs.length === 0) {
+ return;
+ }
+
+ const targetIndex = Math.max(0, Math.min(parsedLogs.length - 1,
Number(hash) || 0));
+ const el = parentRef.current;
+
+ // Prefer exact offset if we already have the virtual item measured
+ const total = rowVirtualizer.getTotalSize();
+ const clientH = el?.clientHeight ?? 0;
+
+ // If the item is currently virtualized, use its start; otherwise
approximate
+ const vItem = rowVirtualizer.getVirtualItems().find((virtualRow) =>
virtualRow.index === targetIndex);
+ const approxPerItem = 20; // same as estimateSize
+ const anchor = vItem?.start ?? targetIndex * approxPerItem;
+
+ const offset = Math.max(0, Math.min(total - clientH, anchor));
+
+ if (el) {
+ // Force both virtualizer and DOM to the exact offset to avoid slow
incremental scrolling
+ if (typeof rowVirtualizer.scrollToOffset === "function") {
+ try {
+ rowVirtualizer.scrollToOffset(offset);
+ } catch {
+ rowVirtualizer.scrollToIndex(targetIndex, { align: "start" });
+ }
+ } else {
+ rowVirtualizer.scrollToIndex(targetIndex, { align: "start" });
+ }
+
+ el.scrollTop = offset;
+
+ // One more frame to settle measurements for large lists
+ requestAnimationFrame(() => {
+ el.scrollTop = offset;
+ });
+ } else {
+ rowVirtualizer.scrollToIndex(targetIndex, { align: "start" });
}
}, [isLoading, rowVirtualizer, hash, parsedLogs]);
+ // Robust, instant jump-to handler
const handleScrollTo = (to: "bottom" | "top") => {
- if (parsedLogs.length > 0) {
- rowVirtualizer.scrollToIndex(to === "bottom" ? parsedLogs.length - 1 :
0);
+ if (parsedLogs.length === 0) {
+ return;
+ }
+
+ const el = rowVirtualizer.scrollElement ?? parentRef.current;
+
+ if (!el) {
+ return;
+ }
+
+ if (to === "top") {
+ // Jump instantly to top
+ if (typeof rowVirtualizer.scrollToOffset === "function") {
+ try {
+ rowVirtualizer.scrollToOffset(0);
+ } catch {
+ rowVirtualizer.scrollToIndex(0, { align: "start" });
+ }
+ } else {
+ rowVirtualizer.scrollToIndex(0, { align: "start" });
+ }
+ el.scrollTop = 0;
+ requestAnimationFrame(() => {
+ el.scrollTop = 0;
+ });
+
+ return;
}
+
+ // === bottom === (instant jump even for huge lists)
+ const total = rowVirtualizer.getTotalSize();
+ const clientH = el.clientHeight || 0;
+ const offset = Math.max(0, Math.floor(total - clientH));
+
+ // First tell the virtualizer where we want to be
+ if (typeof rowVirtualizer.scrollToOffset === "function") {
+ try {
+ rowVirtualizer.scrollToOffset(offset);
+ } catch {
+ rowVirtualizer.scrollToIndex(parsedLogs.length - 1, { align: "end" });
+ }
+ } else {
+ rowVirtualizer.scrollToIndex(parsedLogs.length - 1, { align: "end" });
+ }
+
+ // Then force the DOM to that exact pixel offset
+ el.scrollTop = offset;
+
+ // A couple of RAFs to ensure post-measurement settling for very large
lists
+ requestAnimationFrame(() => {
+ el.scrollTop = offset;
+ requestAnimationFrame(() => {
+ // final nudge + fallback to last item visibility if needed
+ el.scrollTop = offset;
+ const lastItem = el.querySelector<HTMLElement>(
+ `[data-testid="virtualized-item-${parsedLogs.length - 1}"]`,
+ );
+
+ if (lastItem) {
+ lastItem.scrollIntoView({ behavior: "auto", block: "end" });
+ }
+ });
+ });
Review Comment:
This looks overly complicated to achieve what we want (maybe hinted by an AI
agent?). Can we trim this down to the bare minimum to achieve the feature
please.
--
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]
