You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "RoyLee1224 (via GitHub)" <gi...@apache.org> on 2025/11/16 06:56:33 UTC
[PR] Implement task instance and runs tooltips in Gridview [airflow]
RoyLee1224 opened a new pull request, #58359:
URL: https://github.com/apache/airflow/pull/58359
## Related
part of #55900
part of #56250 (The hover tooltips disappear about a second after popping up.)
## Changes
- Refactor `HoverTooltip` and rename it to `BasicTooltip`
- Migrate from `HoverTooltip` to `BasicTooltip` in Calendar tab
- Apply `BasicTooltip` to task instance and runs tooltips in Grid view
## Why
- Chakra UI's Tooltip got some performance issues. The custom `BasicTooltip` provides similar functionality with better rendering performance
Performance issue Reference: https://github.com/apache/airflow/pull/51736#issuecomment-2981597364
- Renamed from `HoverTooltip` to `BasicTooltip` to better reflect its purpose as a performant tooltip component suitable for high-density UI(eg. Grid view, Calendar tab).
## Screenshots
### Performance (Chakra UI tooltip vs. BasicTooltip)
I've tried with both approaches, Charka UI looks laggy with large amount TIs
https://github.com/user-attachments/assets/8c0924ac-e3f9-461a-b625-4088514679ee
https://github.com/user-attachments/assets/dd343f13-af2c-4c0c-bd9f-ce21302ea460
### Before / After
**Before** (tooltip somehow disappear)
https://github.com/user-attachments/assets/ddc67ccc-6588-41b2-b315-f2ae06429ece
**After**(tooltip last correctly)
https://github.com/user-attachments/assets/542f4257-d0ed-4ff7-b693-d2b050e3fdf8
**Before**
| Before (use default title for run tooltip) | After (consistent tooltip with TI) |
| :---: | :---: |
| <img src="https://github.com/user-attachments/assets/a2142cf9-c16c-41bc-82d6-f02ece053ac5" width="350"> | <img src="https://github.com/user-attachments/assets/88d571b1-fadc-48d3-9c5e-e3447e2b3438" width="350"> |
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #58359:
URL: https://github.com/apache/airflow/pull/58359#discussion_r2560247429
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/GridTI.tsx:
##########
@@ -106,35 +109,44 @@ const Instance = ({ dagId, instance, isGroup, isMapped, onClick, runId, taskId }
py={0}
transition="background-color 0.2s"
>
- <Link
- id={`grid-${runId}-${taskId}`}
- onClick={onClick}
- replace
- to={{
- pathname: getTaskUrl(),
- search: redirectionSearch,
- }}
+ <BasicTooltip
+ content={
+ <>
+ {translate("taskId")}: {taskId}
+ <br />
+ {translate("state")}:{" "}
+ {instance.state
+ ? translate(`common:states.${instance.state}`)
+ : translate("common:states.no_status")}
+ {instance.min_start_date !== null && (
+ <>
+ <br />
+ {translate("startDate")}: <Time datetime={instance.min_start_date} />
+ </>
+ )}
+ {instance.max_end_date !== null && (
+ <>
+ <br />
+ {translate("endDate")}: <Time datetime={instance.max_end_date} />
+ </>
+ )}
+ {Boolean(duration) && (
+ <>
+ <br />
+ {translate("duration")}: {duration}
+ </>
+ )}
+ </>
+ }
Review Comment:
We should move this to its own PR. Because this will only work for task group, if we want this everywhere, we need to discuss if we want to update the backend TiSummary, or not.
In any case I think it's unrelated to this PR, and to not make the scope bigger I would split it.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "RoyLee1224 (via GitHub)" <gi...@apache.org>.
RoyLee1224 commented on code in PR #58359:
URL: https://github.com/apache/airflow/pull/58359#discussion_r2566940393
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,123 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Portal } from "@chakra-ui/react";
+import type { ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useRef, useState } from "react";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+};
+
+const offset = 8;
+const zIndex = 1500;
Review Comment:
Tried `zIndex="popover"` and it works well, thanks!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #58359:
URL: https://github.com/apache/airflow/pull/58359#discussion_r2560086040
##########
airflow-core/src/airflow/ui/src/components/DagRunInfo.tsx:
##########
@@ -42,7 +42,7 @@ const DagRunInfo = ({ endDate, logicalDate, runAfter, startDate, state }: Props)
<VStack align="left" gap={0}>
{state === undefined ? undefined : (
<Text>
- {translate("state")}: {state}
+ {translate("state")}: {translate(`common:states.${state}`)}
Review Comment:
Nice catch.
##########
airflow-core/src/airflow/ui/src/components/TaskInstanceTooltip.tsx:
##########
@@ -43,7 +43,10 @@ const TaskInstanceTooltip = ({ children, positioning, taskInstance, ...rest }: P
content={
<Box>
<Text>
- {translate("state")}: {taskInstance.state}
+ {translate("state")}:{" "}
+ {taskInstance.state
+ ? translate(`common:states.${taskInstance.state}`)
+ : translate("common:states.no_status")}
Review Comment:
Nice catch too.
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,123 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Portal } from "@chakra-ui/react";
+import type { ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useRef, useState } from "react";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+};
+
+const offset = 8;
+const zIndex = 1500;
+// Estimated tooltip height for viewport boundary detection
+const estimatedTooltipHeight = 100;
Review Comment:
This won't be true for `task groups` the height of the tooltip seems bigger than this, so it would still be able to overflow without the flip actually occurring.
We can either put a safer value there, (but I don't like that). Or actually use the `ref.current.clientHeight` to get the real value.
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/GridTI.tsx:
##########
@@ -106,35 +109,44 @@ const Instance = ({ dagId, instance, isGroup, isMapped, onClick, runId, taskId }
py={0}
transition="background-color 0.2s"
>
- <Link
- id={`grid-${runId}-${taskId}`}
- onClick={onClick}
- replace
- to={{
- pathname: getTaskUrl(),
- search: redirectionSearch,
- }}
+ <BasicTooltip
+ content={
+ <>
+ {translate("taskId")}: {taskId}
+ <br />
+ {translate("state")}:{" "}
+ {instance.state
+ ? translate(`common:states.${instance.state}`)
+ : translate("common:states.no_status")}
+ {instance.min_start_date !== null && (
+ <>
+ <br />
+ {translate("startDate")}: <Time datetime={instance.min_start_date} />
+ </>
+ )}
+ {instance.max_end_date !== null && (
+ <>
+ <br />
+ {translate("endDate")}: <Time datetime={instance.max_end_date} />
+ </>
+ )}
+ {Boolean(duration) && (
+ <>
+ <br />
+ {translate("duration")}: {duration}
+ </>
+ )}
+ </>
+ }
>
Review Comment:
Adding the duration seems outside the scope of this PR.
Do you mind removing so we limit the scope of this one. It will only make sense for 'groups' for now and we can discuss about it if we want to expand more.
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,123 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Portal } from "@chakra-ui/react";
+import type { ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useRef, useState } from "react";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+};
+
+const offset = 8;
+const zIndex = 1500;
Review Comment:
`popover` chakra semantic token:
`system.tokens.categoryMap.get("zIndex")?.get("popover")?.value`
Or something like it
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement task instance and runs tooltips in Gridview [airflow]
Posted by "aaron-wolmutt (via GitHub)" <gi...@apache.org>.
aaron-wolmutt commented on PR #58359:
URL: https://github.com/apache/airflow/pull/58359#issuecomment-3560741111
> Thanks @NilsJPWerner @waltervillarreal @aaron-wolmutt. I agree those time fields are very useful.
>
> However, the current `LightGridTaskInstanceSummary` explicitly returns None for standard tasks (it only calculates time for Groups).
>
> Modifying the API to return these timestamps for all tasks involves backend changes and potential schema updates (similar to the attempts in #55927)
>
> To avoid scope creep, I'd like to keep this PR focused on fixing the flashing bug and performance, restoring the tooltip functionality as seen in #53438.
>
> We can open a separate PR to update the backend and enable the time fields display.
You have a great point about avoiding scope creep in pull requests. I'm trying keep my PRs separated by backend/front end as well. I think the fix would fit 3.1.X then and adding those fields to the API would probably fit the 3.2 release
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #58359:
URL: https://github.com/apache/airflow/pull/58359#issuecomment-3607809550
### Backport successfully created: v3-1-test
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>v3-1-test</td>
<td><a href="https://github.com/apache/airflow/pull/59013"><img src="https://img.shields.io/badge/PR-59013-blue" alt="PR Link"></a></td>
</tr>
</table>
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "RoyLee1224 (via GitHub)" <gi...@apache.org>.
RoyLee1224 commented on code in PR #58359:
URL: https://github.com/apache/airflow/pull/58359#discussion_r2567003652
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,122 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Portal } from "@chakra-ui/react";
+import type { ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useRef, useState } from "react";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+};
+
+const offset = 8;
+// Estimated tooltip height for viewport boundary detection
+const estimatedTooltipHeight = 100;
+
+export const BasicTooltip = ({ children, content }: Props): ReactElement => {
+ const triggerRef = useRef<HTMLElement>(null);
+ const [isOpen, setIsOpen] = useState(false);
+ const [showOnTop, setShowOnTop] = useState(false);
+ const timeoutRef = useRef<NodeJS.Timeout>();
+
+ const handleMouseEnter = useCallback(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ timeoutRef.current = setTimeout(() => {
+ // Check if tooltip would overflow viewport bottom
+ if (triggerRef.current) {
+ const triggerRect = triggerRef.current.getBoundingClientRect();
+ const wouldOverflow = triggerRect.bottom + offset + estimatedTooltipHeight > globalThis.innerHeight;
+
+ setShowOnTop(wouldOverflow);
+ }
+ setIsOpen(true);
+ }, 500);
Review Comment:
Currently using 500ms delay. We can drop it to 300ms if we want a faster interaction.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "RoyLee1224 (via GitHub)" <gi...@apache.org>.
RoyLee1224 commented on code in PR #58359:
URL: https://github.com/apache/airflow/pull/58359#discussion_r2551233781
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,257 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Portal } from "@chakra-ui/react";
+import type { CSSProperties, ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useMemo, useRef, useState } from "react";
+
+export type TooltipPlacement =
+ | "bottom-end"
+ | "bottom-start"
+ | "bottom"
+ | "left"
+ | "right"
+ | "top-end"
+ | "top-start"
+ | "top";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+ readonly placement?: TooltipPlacement;
+};
+
+const calculatePosition = (
+ rect: DOMRect,
+ placement: TooltipPlacement,
+ offset: number,
+): { left: string; top: string; transform: string } => {
+ const { bottom, height, left, right, top, width } = rect;
+ const { scrollX, scrollY } = globalThis;
+
+ switch (placement) {
+ case "bottom":
+ return {
+ left: `${left + scrollX + width / 2}px`,
+ top: `${bottom + scrollY + offset}px`,
+ transform: "translateX(-50%)",
+ };
+
+ case "bottom-end":
+ return {
+ left: `${right + scrollX}px`,
+ top: `${bottom + scrollY + offset}px`,
+ transform: "translateX(-100%)",
+ };
+
+ case "bottom-start":
+ return {
+ left: `${left + scrollX}px`,
+ top: `${bottom + scrollY + offset}px`,
+ transform: "none",
+ };
+
+ case "left":
+ return {
Review Comment:
Agree, let me try to make it automatic flip to top when tooltip would overflow viewport bottom
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement task instance and runs tooltips in Gridview [airflow]
Posted by "aaron-wolmutt (via GitHub)" <gi...@apache.org>.
aaron-wolmutt commented on PR #58359:
URL: https://github.com/apache/airflow/pull/58359#issuecomment-3550034058
Is the datetime in the screenshots the task start or the task end? I think the datetime should be in yyy-mm-dd, hh:mm:ss tz like in 2.11
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement task instance and runs tooltips in Gridview [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #58359:
URL: https://github.com/apache/airflow/pull/58359#discussion_r2550088063
##########
airflow-core/src/airflow/ui/src/pages/Dag/Calendar/CalendarCell.tsx:
##########
@@ -103,5 +96,9 @@ export const CalendarCell = ({
return cellBox;
}
- return <HoverTooltip tooltip={renderTooltip(cellData, viewMode)}>{cellBox}</HoverTooltip>;
+ return (
+ <BasicTooltip content={<CalendarTooltip cellData={cellData} viewMode={viewMode} />} placement="bottom">
Review Comment:
```suggestion
<BasicTooltip content={<CalendarTooltip cellData={cellData} viewMode={viewMode} />}>
```
Bottom is the default.
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,257 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Portal } from "@chakra-ui/react";
+import type { CSSProperties, ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useMemo, useRef, useState } from "react";
+
+export type TooltipPlacement =
+ | "bottom-end"
+ | "bottom-start"
+ | "bottom"
+ | "left"
+ | "right"
+ | "top-end"
+ | "top-start"
+ | "top";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+ readonly placement?: TooltipPlacement;
+};
+
+const calculatePosition = (
+ rect: DOMRect,
+ placement: TooltipPlacement,
+ offset: number,
+): { left: string; top: string; transform: string } => {
+ const { bottom, height, left, right, top, width } = rect;
+ const { scrollX, scrollY } = globalThis;
+
+ switch (placement) {
+ case "bottom":
+ return {
+ left: `${left + scrollX + width / 2}px`,
+ top: `${bottom + scrollY + offset}px`,
+ transform: "translateX(-50%)",
+ };
+
+ case "bottom-end":
+ return {
+ left: `${right + scrollX}px`,
+ top: `${bottom + scrollY + offset}px`,
+ transform: "translateX(-100%)",
+ };
+
+ case "bottom-start":
+ return {
+ left: `${left + scrollX}px`,
+ top: `${bottom + scrollY + offset}px`,
+ transform: "none",
+ };
+
+ case "left":
+ return {
Review Comment:
It appears that we only use placement bottom for now. I wouldn't go through all that effort to handle other placement option if we don't need them at this stage.
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,257 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Portal } from "@chakra-ui/react";
+import type { CSSProperties, ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useMemo, useRef, useState } from "react";
+
+export type TooltipPlacement =
+ | "bottom-end"
+ | "bottom-start"
+ | "bottom"
+ | "left"
+ | "right"
+ | "top-end"
+ | "top-start"
+ | "top";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+ readonly placement?: TooltipPlacement;
+};
+
+const calculatePosition = (
+ rect: DOMRect,
+ placement: TooltipPlacement,
+ offset: number,
+): { left: string; top: string; transform: string } => {
+ const { bottom, height, left, right, top, width } = rect;
+ const { scrollX, scrollY } = globalThis;
+
+ switch (placement) {
+ case "bottom":
+ return {
+ left: `${left + scrollX + width / 2}px`,
+ top: `${bottom + scrollY + offset}px`,
+ transform: "translateX(-50%)",
+ };
+
+ case "bottom-end":
+ return {
+ left: `${right + scrollX}px`,
+ top: `${bottom + scrollY + offset}px`,
+ transform: "translateX(-100%)",
+ };
+
+ case "bottom-start":
+ return {
+ left: `${left + scrollX}px`,
+ top: `${bottom + scrollY + offset}px`,
+ transform: "none",
+ };
+
+ case "left":
+ return {
+ left: `${left + scrollX - offset}px`,
+ top: `${top + scrollY + height / 2}px`,
+ transform: "translate(-100%, -50%)",
+ };
+
+ case "right":
+ return {
+ left: `${right + scrollX + offset}px`,
+ top: `${top + scrollY + height / 2}px`,
+ transform: "translateY(-50%)",
+ };
+
+ case "top":
+ return {
+ left: `${left + scrollX + width / 2}px`,
+ top: `${top + scrollY - offset}px`,
+ transform: "translate(-50%, -100%)",
+ };
+
+ case "top-end":
+ return {
+ left: `${right + scrollX}px`,
+ top: `${top + scrollY - offset}px`,
+ transform: "translate(-100%, -100%)",
+ };
+
+ case "top-start":
+ return {
+ left: `${left + scrollX}px`,
+ top: `${top + scrollY - offset}px`,
+ transform: "translateY(-100%)",
+ };
+
+ default:
+ return {
+ left: `${left + scrollX + width / 2}px`,
+ top: `${top + scrollY - offset}px`,
+ transform: "translate(-50%, -100%)",
+ };
+ }
+};
+
+const getArrowStyle = (placement: TooltipPlacement): CSSProperties => {
+ const baseStyle: CSSProperties = {
+ content: '""',
+ height: 0,
+ position: "absolute",
+ width: 0,
+ };
+
+ switch (placement) {
+ case "bottom":
+ case "bottom-end":
+ case "bottom-start":
+ return {
+ ...baseStyle,
+ borderBottom: "4px solid var(--chakra-colors-bg-inverted)",
+ borderLeft: "4px solid transparent",
+ borderRight: "4px solid transparent",
+ left: placement === "bottom" ? "50%" : placement === "bottom-start" ? "12px" : undefined,
+ right: placement === "bottom-end" ? "12px" : undefined,
+ top: "-4px",
+ transform: placement === "bottom" ? "translateX(-50%)" : undefined,
+ };
+
+ case "left":
+ return {
+ ...baseStyle,
+ borderBottom: "4px solid transparent",
+ borderLeft: "4px solid var(--chakra-colors-bg-inverted)",
+ borderTop: "4px solid transparent",
+ right: "-4px",
+ top: "50%",
+ transform: "translateY(-50%)",
+ };
+
+ case "right":
+ return {
+ ...baseStyle,
+ borderBottom: "4px solid transparent",
+ borderRight: "4px solid var(--chakra-colors-bg-inverted)",
+ borderTop: "4px solid transparent",
+ left: "-4px",
+ top: "50%",
+ transform: "translateY(-50%)",
+ };
+
+ case "top":
+ case "top-end":
+ case "top-start":
+ return {
+ ...baseStyle,
+ borderLeft: "4px solid transparent",
+ borderRight: "4px solid transparent",
+ borderTop: "4px solid var(--chakra-colors-bg-inverted)",
+ bottom: "-4px",
+ left: placement === "top" ? "50%" : placement === "top-start" ? "12px" : undefined,
+ right: placement === "top-end" ? "12px" : undefined,
+ transform: placement === "top" ? "translateX(-50%)" : undefined,
+ };
+
+ default:
+ return baseStyle;
+ }
+};
+
+export const BasicTooltip = ({ children, content, placement = "bottom" }: Props): ReactElement => {
+ const triggerRef = useRef<HTMLElement>(null);
+ const [isOpen, setIsOpen] = useState(false);
+ const timeoutRef = useRef<NodeJS.Timeout>();
+
+ const offset = 8;
+ const zIndex = 1500;
+
+ const handleMouseEnter = useCallback(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ timeoutRef.current = setTimeout(() => {
+ setIsOpen(true);
+ }, 500);
+ }, []);
+
+ const handleMouseLeave = useCallback(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ timeoutRef.current = undefined;
+ }
+ setIsOpen(false);
+ }, []);
+
+ // Cleanup on unmount
+ useEffect(
+ () => () => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ },
+ [],
+ );
+
+ const tooltipStyle = useMemo(() => {
+ if (!isOpen || !triggerRef.current) {
+ return { display: "none" };
+ }
+
+ const rect = triggerRef.current.getBoundingClientRect();
+ const position = calculatePosition(rect, placement, offset);
+
+ return {
+ ...position,
+ backgroundColor: "var(--chakra-colors-bg-inverted)",
+ borderRadius: "4px",
+ boxShadow: "0 2px 8px rgba(0, 0, 0, 0.15)",
+ color: "var(--chakra-colors-fg-inverted)",
+ fontSize: "14px",
+ padding: "8px 12px",
+ pointerEvents: "none" as const,
+ position: "absolute" as const,
+ whiteSpace: "nowrap" as const,
+ zIndex,
+ };
+ }, [isOpen, placement, offset, zIndex]);
+
+ const arrowStyle = useMemo(() => getArrowStyle(placement), [placement]);
+
+ // Clone children and attach event handlers + ref
+ const trigger = cloneElement(children, {
+ onMouseEnter: handleMouseEnter,
+ onMouseLeave: handleMouseLeave,
+ ref: triggerRef,
+ });
+
+ return (
+ <>
+ {trigger}
+ {Boolean(isOpen) && (
+ <Portal>
+ <div style={tooltipStyle}>
Review Comment:
If we remove the placement and position compute, we can also simplify this.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "RoyLee1224 (via GitHub)" <gi...@apache.org>.
RoyLee1224 commented on code in PR #58359:
URL: https://github.com/apache/airflow/pull/58359#discussion_r2582298658
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/GridButton.tsx:
##########
@@ -41,39 +43,53 @@ export const GridButton = ({
state,
taskId,
...rest
-}: Props) =>
- isGroup ? (
- <Flex
- background={`${state}.solid`}
- borderRadius={2}
- height="10px"
- minW="14px"
- pb="2px"
- px="2px"
- title={`${label}\n${state}`}
- {...rest}
- >
- {children}
- </Flex>
- ) : (
- <Link
- replace
- to={{
- pathname: `/dags/${dagId}/runs/${runId}/${taskId === undefined ? "" : `tasks/${taskId}`}`,
- search: searchParams.toString(),
- }}
- >
+}: Props) => {
+ const { t: translate } = useTranslation();
+
+ const tooltipContent = (
+ <>
+ {label}
+ <br />
+ {translate("state")}:{" "}
+ {state ? translate(`common:states.${state}`) : translate("common:states.no_status")}
+ </>
+ );
+
+ return isGroup ? (
+ <BasicTooltip content={tooltipContent}>
<Flex
background={`${state}.solid`}
borderRadius={2}
height="10px"
+ minW="14px"
pb="2px"
px="2px"
- title={`${label}\n${state}`}
- width="14px"
{...rest}
>
{children}
</Flex>
- </Link>
+ </BasicTooltip>
+ ) : (
+ <BasicTooltip content={tooltipContent}>
+ <Link
+ replace
+ to={{
+ pathname: `/dags/${dagId}/runs/${runId}/${taskId === undefined ? "" : `tasks/${taskId}`}`,
Review Comment:
Oh I've implemented `buildTaskInstanceUrl` in #58881, perhaps leaving the url refactors in that PR would be better?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement task instance and runs tooltips in Gridview [airflow]
Posted by "waltervillarreal (via GitHub)" <gi...@apache.org>.
waltervillarreal commented on PR #58359:
URL: https://github.com/apache/airflow/pull/58359#issuecomment-3557945633
Thanks @RoyLee1224, I agree that it would be super useful to have the version, status, start, end, duration, and trigger rule in the tool tip.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement task instance and runs tooltips in Gridview [airflow]
Posted by "NilsJPWerner (via GitHub)" <gi...@apache.org>.
NilsJPWerner commented on PR #58359:
URL: https://github.com/apache/airflow/pull/58359#issuecomment-3543246446
Would it be worth adding duration to the tooltip?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement task instance and runs tooltips in Gridview [airflow]
Posted by "aaron-wolmutt (via GitHub)" <gi...@apache.org>.
aaron-wolmutt commented on PR #58359:
URL: https://github.com/apache/airflow/pull/58359#issuecomment-3550019672
> Would it be worth adding duration to the tooltip?
The 2.11 version has task id, status, started, ended, duration and trigger rule in the tool tip
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "RoyLee1224 (via GitHub)" <gi...@apache.org>.
RoyLee1224 commented on code in PR #58359:
URL: https://github.com/apache/airflow/pull/58359#discussion_r2583496600
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,127 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Portal } from "@chakra-ui/react";
+import type { ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useLayoutEffect, useRef, useState } from "react";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+};
+
+const offset = 8;
+
+export const BasicTooltip = ({ children, content }: Props): ReactElement => {
+ const triggerRef = useRef<HTMLElement>(null);
+ const tooltipRef = useRef<HTMLDivElement>(null);
+ const [isOpen, setIsOpen] = useState(false);
+ const [showOnTop, setShowOnTop] = useState(false);
+ const timeoutRef = useRef<NodeJS.Timeout>();
+
+ const handleMouseEnter = useCallback(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ timeoutRef.current = setTimeout(() => {
+ setIsOpen(true);
+ }, 500);
+ }, []);
+
+ const handleMouseLeave = useCallback(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ timeoutRef.current = undefined;
+ }
+ setIsOpen(false);
+ }, []);
+
+ // Calculate position based on actual tooltip height before paint
+ useLayoutEffect(() => {
+ if (isOpen && triggerRef.current && tooltipRef.current) {
+ const triggerRect = triggerRef.current.getBoundingClientRect();
+ const tooltipHeight = tooltipRef.current.clientHeight;
+ const wouldOverflow = triggerRect.bottom + offset + tooltipHeight > globalThis.innerHeight;
+
+ setShowOnTop(wouldOverflow);
+ }
+ }, [isOpen]);
+
+ // Cleanup on unmount
Review Comment:
Strictly speaking, the current code won't crash because React 18 suppresses updates on unmounted components.
But I think the cleanup prevents potential crashes if we access refs inside the timeout, and it handles the edge case where the component unmounts without triggering `onMouseLeave` (e.g., switching to Graph view).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #58359:
URL: https://github.com/apache/airflow/pull/58359#discussion_r2585880381
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,127 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Portal } from "@chakra-ui/react";
+import type { ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useLayoutEffect, useRef, useState } from "react";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+};
+
+const offset = 8;
+
+export const BasicTooltip = ({ children, content }: Props): ReactElement => {
+ const triggerRef = useRef<HTMLElement>(null);
+ const tooltipRef = useRef<HTMLDivElement>(null);
+ const [isOpen, setIsOpen] = useState(false);
+ const [showOnTop, setShowOnTop] = useState(false);
+ const timeoutRef = useRef<NodeJS.Timeout>();
+
+ const handleMouseEnter = useCallback(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ timeoutRef.current = setTimeout(() => {
+ setIsOpen(true);
+ }, 500);
+ }, []);
+
+ const handleMouseLeave = useCallback(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ timeoutRef.current = undefined;
+ }
+ setIsOpen(false);
+ }, []);
+
+ // Calculate position based on actual tooltip height before paint
+ useLayoutEffect(() => {
+ if (isOpen && triggerRef.current && tooltipRef.current) {
+ const triggerRect = triggerRef.current.getBoundingClientRect();
+ const tooltipHeight = tooltipRef.current.clientHeight;
+ const wouldOverflow = triggerRect.bottom + offset + tooltipHeight > globalThis.innerHeight;
+
+ setShowOnTop(wouldOverflow);
+ }
+ }, [isOpen]);
+
+ // Cleanup on unmount
Review Comment:
I wondered the same. We can leave it as a safety net
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #58359:
URL: https://github.com/apache/airflow/pull/58359
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "RoyLee1224 (via GitHub)" <gi...@apache.org>.
RoyLee1224 commented on PR #58359:
URL: https://github.com/apache/airflow/pull/58359#issuecomment-3565020461
## Update
- Add auto-flip for viewport overflow
- Add translation for tooltip states
https://github.com/user-attachments/assets/1898adc7-4add-4cca-8719-7ab326d65b90
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Implement task instance and runs tooltips in Gridview [airflow]
Posted by "RoyLee1224 (via GitHub)" <gi...@apache.org>.
RoyLee1224 commented on PR #58359:
URL: https://github.com/apache/airflow/pull/58359#issuecomment-3560708425
Thanks @NilsJPWerner @waltervillarreal @aaron-wolmutt. I agree those time fields are very useful.
However, the current `LightGridTaskInstanceSummary` explicitly returns None for standard tasks (it only calculates time for Groups).
Modifying the API to return these timestamps for all tasks involves backend changes and potential schema updates (similar to the attempts in #55927)
To avoid scope creep, I'd like to keep this PR focused on fixing the flashing bug and performance, restoring the tooltip functionality as seen in #53438.
We can open a separate PR to update the backend and enable the time fields display.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix task instance and runs tooltips in Grid view [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on code in PR #58359:
URL: https://github.com/apache/airflow/pull/58359#discussion_r2575844882
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/GridButton.tsx:
##########
@@ -41,39 +43,53 @@ export const GridButton = ({
state,
taskId,
...rest
-}: Props) =>
- isGroup ? (
- <Flex
- background={`${state}.solid`}
- borderRadius={2}
- height="10px"
- minW="14px"
- pb="2px"
- px="2px"
- title={`${label}\n${state}`}
- {...rest}
- >
- {children}
- </Flex>
- ) : (
- <Link
- replace
- to={{
- pathname: `/dags/${dagId}/runs/${runId}/${taskId === undefined ? "" : `tasks/${taskId}`}`,
- search: searchParams.toString(),
- }}
- >
+}: Props) => {
+ const { t: translate } = useTranslation();
+
+ const tooltipContent = (
+ <>
+ {label}
+ <br />
+ {translate("state")}:{" "}
+ {state ? translate(`common:states.${state}`) : translate("common:states.no_status")}
+ </>
+ );
+
+ return isGroup ? (
+ <BasicTooltip content={tooltipContent}>
<Flex
background={`${state}.solid`}
borderRadius={2}
height="10px"
+ minW="14px"
pb="2px"
px="2px"
- title={`${label}\n${state}`}
- width="14px"
{...rest}
>
{children}
</Flex>
- </Link>
+ </BasicTooltip>
+ ) : (
+ <BasicTooltip content={tooltipContent}>
+ <Link
+ replace
+ to={{
+ pathname: `/dags/${dagId}/runs/${runId}/${taskId === undefined ? "" : `tasks/${taskId}`}`,
Review Comment:
Small nit, We could reuse the `buildTaskInstanceUrl`.
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,122 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Portal } from "@chakra-ui/react";
+import type { ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useRef, useState } from "react";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+};
+
+const offset = 8;
+// Estimated tooltip height for viewport boundary detection
+const estimatedTooltipHeight = 100;
+
+export const BasicTooltip = ({ children, content }: Props): ReactElement => {
+ const triggerRef = useRef<HTMLElement>(null);
+ const [isOpen, setIsOpen] = useState(false);
+ const [showOnTop, setShowOnTop] = useState(false);
+ const timeoutRef = useRef<NodeJS.Timeout>();
+
+ const handleMouseEnter = useCallback(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ timeoutRef.current = setTimeout(() => {
+ // Check if tooltip would overflow viewport bottom
+ if (triggerRef.current) {
+ const triggerRect = triggerRef.current.getBoundingClientRect();
+ const wouldOverflow = triggerRect.bottom + offset + estimatedTooltipHeight > globalThis.innerHeight;
+
+ setShowOnTop(wouldOverflow);
+ }
+ setIsOpen(true);
+ }, 500);
Review Comment:
I think 500ms is better based on my local test, but open to any opinion!
##########
airflow-core/src/airflow/ui/src/components/BasicTooltip.tsx:
##########
@@ -0,0 +1,127 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { Box, Portal } from "@chakra-ui/react";
+import type { ReactElement, ReactNode } from "react";
+import { cloneElement, useCallback, useEffect, useLayoutEffect, useRef, useState } from "react";
+
+type Props = {
+ readonly children: ReactElement;
+ readonly content: ReactNode;
+};
+
+const offset = 8;
+
+export const BasicTooltip = ({ children, content }: Props): ReactElement => {
+ const triggerRef = useRef<HTMLElement>(null);
+ const tooltipRef = useRef<HTMLDivElement>(null);
+ const [isOpen, setIsOpen] = useState(false);
+ const [showOnTop, setShowOnTop] = useState(false);
+ const timeoutRef = useRef<NodeJS.Timeout>();
+
+ const handleMouseEnter = useCallback(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ timeoutRef.current = setTimeout(() => {
+ setIsOpen(true);
+ }, 500);
+ }, []);
+
+ const handleMouseLeave = useCallback(() => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ timeoutRef.current = undefined;
+ }
+ setIsOpen(false);
+ }, []);
+
+ // Calculate position based on actual tooltip height before paint
+ useLayoutEffect(() => {
+ if (isOpen && triggerRef.current && tooltipRef.current) {
+ const triggerRect = triggerRef.current.getBoundingClientRect();
+ const tooltipHeight = tooltipRef.current.clientHeight;
+ const wouldOverflow = triggerRect.bottom + offset + tooltipHeight > globalThis.innerHeight;
+
+ setShowOnTop(wouldOverflow);
+ }
+ }, [isOpen]);
+
+ // Cleanup on unmount
Review Comment:
I'm not sure whether we should need this, could you help confirm this?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org