bbovenzi commented on code in PR #51146:
URL: https://github.com/apache/airflow/pull/51146#discussion_r2127090289
##########
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/TaskLogHeader.tsx:
##########
@@ -178,20 +200,57 @@ export const TaskLogHeader = ({
</Select.Content>
</Select.Root>
) : undefined}
- <HStack>
+ <HStack gap={1}>
<Tooltip closeDelay={100} content="Press w to toggle wrap"
openDelay={100}>
<Button
+ _hover={{ bg: "gray.800" }}
aria-label={wrap ? "Unwrap" : "Wrap"}
- bg="bg.panel"
+ bg="black"
+ color="white"
+ m={0}
onClick={toggleWrap}
+ px={4}
+ py={2}
variant="outline"
>
{wrap ? "Unwrap" : "Wrap"}
</Button>
</Tooltip>
+ {toggleExpanded ? (
+ <Tooltip
+ closeDelay={100}
+ content={expanded ? "Press e to collapse" : "Press e to expand"}
+ openDelay={100}
+ >
+ <Button
+ _hover={{ bg: "gray.800" }}
+ aria-label={expanded ? "Collapse All" : "Expand"}
+ bg="black"
+ color="white"
+ m={0}
+ onClick={toggleExpanded}
+ px={4}
+ py={2}
+ size="md"
+ variant="outline"
+ >
+ {expanded ? "Collapse All" : "Expand "}
Review Comment:
I think its also ok to reuse the expand all / collapse all icons we use for
the grid view in `ToggleGroups.tsx`
##########
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/TaskLogHeader.tsx:
##########
@@ -111,6 +115,24 @@ export const TaskLogHeader = ({
[searchParams, setSearchParams],
);
+ useEffect(() => {
+ if (!toggleExpanded) {
+ return undefined;
+ }
+ const handler = (event: KeyboardEvent) => {
+ if (event.key === "e" || event.key === "E") {
+ event.preventDefault();
+ toggleExpanded();
+ }
+ };
+
+ globalThis.addEventListener("keydown", handler);
+
+ return () => {
+ globalThis.removeEventListener("keydown", handler);
+ };
+ }, [toggleExpanded]);
+
Review Comment:
You can just check how we call `useHotkeys()` in other places
##########
airflow-core/src/airflow/ui/src/pages/TaskInstance/Logs/TaskLogHeader.tsx:
##########
@@ -178,20 +200,57 @@ export const TaskLogHeader = ({
</Select.Content>
</Select.Root>
) : undefined}
- <HStack>
+ <HStack gap={1}>
<Tooltip closeDelay={100} content="Press w to toggle wrap"
openDelay={100}>
<Button
+ _hover={{ bg: "gray.800" }}
aria-label={wrap ? "Unwrap" : "Wrap"}
- bg="bg.panel"
+ bg="black"
+ color="white"
+ m={0}
onClick={toggleWrap}
+ px={4}
+ py={2}
variant="outline"
>
{wrap ? "Unwrap" : "Wrap"}
</Button>
</Tooltip>
+ {toggleExpanded ? (
+ <Tooltip
+ closeDelay={100}
+ content={expanded ? "Press e to collapse" : "Press e to expand"}
+ openDelay={100}
+ >
+ <Button
+ _hover={{ bg: "gray.800" }}
+ aria-label={expanded ? "Collapse All" : "Expand"}
+ bg="black"
+ color="white"
+ m={0}
+ onClick={toggleExpanded}
+ px={4}
+ py={2}
+ size="md"
+ variant="outline"
+ >
+ {expanded ? "Collapse All" : "Expand "}
+ </Button>
+ </Tooltip>
+ ) : undefined}
{!isFullscreen && (
<Tooltip closeDelay={100} content="Press f for fullscreen"
openDelay={100}>
- <IconButton aria-label="Full screen" bg="bg.panel"
onClick={toggleFullscreen} variant="outline">
+ <IconButton
+ _hover={{ bg: "gray.800" }}
Review Comment:
Let's use the semantic colors here.
```suggestion
_hover={{ bg: "bg.emphasized" }}
```
--
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]
