pierrejeambrun commented on code in PR #64284:
URL: https://github.com/apache/airflow/pull/64284#discussion_r3016906475
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/TaskNames.tsx:
##########
@@ -109,7 +129,12 @@ export const TaskNames = ({ nodes, onRowClick,
virtualItems }: Props) => {
{node.isGroup ? (
<Link asChild data-testid={node.id} display="block" width="100%">
<RouterLink
- onClick={onRowClick}
+ onClick={() => {
+ handleToggleGroupOnLinkClick(node.id);
+ if (onRowClick) {
+ onRowClick();
+ }
+ }}
Review Comment:
We are passing the `groupId` through data-group-id similarly to other places
in the same components.
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/TaskNames.tsx:
##########
@@ -67,6 +67,26 @@ export const TaskNames = ({ nodes, onRowClick, virtualItems
}: Props) => {
}
};
+ const handleToggleGroupOnLinkClick = (groupNodeId?: string) => {
+ if (groupNodeId === undefined || groupNodeId === "") {
+ return;
+ }
+
+ const id = groupNodeId;
+ const isViewingSameGroup = typeof groupId === "string" && groupId === id;
+ const isOpen = openGroupIds.includes(id);
+
+ if (isViewingSameGroup) {
+ toggleGroupId(id);
+
+ return;
+ }
+
+ if (!isOpen) {
+ setOpenGroupIds([...new Set([...openGroupIds, id])]);
+ }
+ };
Review Comment:
```
const onClick = (groupNodeId?: string) => {
if (groupNodeId === undefined || groupNodeId === "") {
return;
}
const id = groupNodeId;
const isViewingSameGroup = typeof groupId === "string" && groupId === id;
if (isViewingSameGroup) {
toggleGroupId(id);
}
onRowClick?.();
};
```
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/TaskNames.tsx:
##########
@@ -109,7 +129,12 @@ export const TaskNames = ({ nodes, onRowClick,
virtualItems }: Props) => {
{node.isGroup ? (
<Link asChild data-testid={node.id} display="block" width="100%">
<RouterLink
- onClick={onRowClick}
+ onClick={() => {
+ handleToggleGroupOnLinkClick(node.id);
+ if (onRowClick) {
+ onRowClick();
+ }
+ }}
Review Comment:
```suggestion
data-group-id={node.id}
onClick={onClick}
```
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/TaskNames.tsx:
##########
@@ -67,6 +67,26 @@ export const TaskNames = ({ nodes, onRowClick, virtualItems
}: Props) => {
}
};
+ const handleToggleGroupOnLinkClick = (groupNodeId?: string) => {
+ if (groupNodeId === undefined || groupNodeId === "") {
+ return;
+ }
+
+ const id = groupNodeId;
+ const isViewingSameGroup = typeof groupId === "string" && groupId === id;
+ const isOpen = openGroupIds.includes(id);
+
+ if (isViewingSameGroup) {
+ toggleGroupId(id);
+
+ return;
+ }
+
+ if (!isOpen) {
+ setOpenGroupIds([...new Set([...openGroupIds, id])]);
+ }
+ };
Review Comment:
Also you can call this onClick and call 'onRowClick` at the end for a
cleaner code.
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/TaskNames.tsx:
##########
@@ -67,6 +67,26 @@ export const TaskNames = ({ nodes, onRowClick, virtualItems
}: Props) => {
}
};
+ const handleToggleGroupOnLinkClick = (groupNodeId?: string) => {
+ if (groupNodeId === undefined || groupNodeId === "") {
+ return;
+ }
+
+ const id = groupNodeId;
+ const isViewingSameGroup = typeof groupId === "string" && groupId === id;
+ const isOpen = openGroupIds.includes(id);
+
+ if (isViewingSameGroup) {
+ toggleGroupId(id);
+
+ return;
+ }
+
+ if (!isOpen) {
+ setOpenGroupIds([...new Set([...openGroupIds, id])]);
+ }
+ };
Review Comment:
```suggestion
const onClick = (event: MouseEvent<HTMLSpanElement>) => {
const groupNodeId = event.currentTarget.dataset.groupId;
if (groupNodeId === undefined || groupNodeId === "") {
return;
}
const id = groupNodeId;
const isViewingSameGroup = typeof groupId === "string" && groupId === id;
if (isViewingSameGroup) {
toggleGroupId(id);
}
onRowClick?.();
};
```
--
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]
