Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/components/app-sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from "@/components/ui/sidebar"
import type { RouteObject } from "@/app/reducers/uiReducer"
import UserDropdown from "@/components/custom/system/UserDropdown"
import ServerStatus from "@/components/custom/system/ServerStatus"

export interface AppSidebarProps extends React.ComponentProps<typeof Sidebar> {
data: {
Expand Down Expand Up @@ -59,6 +60,7 @@ export function AppSidebar({
</SidebarContent>
<SidebarRail className={"after:!border-border"} />
<SidebarFooter>
<ServerStatus />
<UserDropdown />
</SidebarFooter>
</Sidebar>
Expand Down
101 changes: 70 additions & 31 deletions src/components/custom/DrawerJobEvents.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,22 @@
import SheetActionDialog from "@/components/sheet-action-dialog"
import type { jobsTableData } from "@/features/jobsTable/interfaces"
import type { ReactNode } from "react"
import { ReactNode, useEffect } from "react"
import { useCallback } from "react"
import { useMemo, useRef } from "react"
import { useState } from "react"
import { JobEventTypes } from "@/models/jobs"
import { useJobEvents } from "@/hooks/useJobEvents"
import { EventItem } from "@/components/custom/general/EventItem"
import { ScrollArea } from "@/components/ui/scroll-area"
import { Separator } from "@/components/ui/separator"
import { ManagedSimpleDropdown } from "@/components/custom/ManagedSimpleDropdown"
import { Spinner } from "@/components/ui/spinner"
import { ButtonWithTooltip } from "@/components/custom/general/ButtonWithTooltip"
import { Switch } from "@/components/ui/switch"
import { cn } from "@/lib/utils"
import { ListCheck } from "lucide-react"
import { useVirtualizer } from "@tanstack/react-virtual"
import LoadingOverlay from "@/components/custom/LoadingOverlay"
import { debounce } from "@/utils/generalUtils"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Unused import: debounce.

The debounce function is imported but never used in this file. This appears to be leftover from development.

🧹 Proposed fix
 import LoadingOverlay from "@/components/custom/LoadingOverlay"
-import { debounce } from "@/utils/generalUtils"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { debounce } from "@/utils/generalUtils"
import LoadingOverlay from "@/components/custom/LoadingOverlay"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/custom/DrawerJobEvents.tsx` at line 19, Remove the unused
import "debounce" from the top of the DrawerJobEvents component file; locate the
import line that reads import { debounce } from "@/utils/generalUtils" and
delete it (or remove debounce from the named imports if combined), ensuring the
DrawerJobEvents component and any other imports remain unchanged.


export interface DrawerNewComponentProps {
JobDetails: jobsTableData
Expand All @@ -36,6 +39,7 @@ export default function DrawerJobEvents({
JobEventTypes.INFO,
])
const [unreadOnly, setUnreadOnly] = useState(false)
const parentRef = useRef<HTMLDivElement>(null)

const { events, latestEvents, loading, setEventsToHandled } = useJobEvents({
jobId: JobDetails.id,
Expand Down Expand Up @@ -63,6 +67,30 @@ export default function DrawerJobEvents({
[filteredEventTypes],
)

const allEvents = useMemo(() => {
return [...latestEvents, ...events]
}, [events, latestEvents, latestEvents.length, events.length])
Comment on lines +70 to +72
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Redundant dependency array entries.

The .length properties are redundant since events and latestEvents references change when the arrays are updated.

🧹 Simplified dependencies
   const allEvents = useMemo(() => {
     return [...latestEvents, ...events]
-  }, [events, latestEvents, latestEvents.length, events.length])
+  }, [events, latestEvents])
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const allEvents = useMemo(() => {
return [...latestEvents, ...events]
}, [events, latestEvents, latestEvents.length, events.length])
const allEvents = useMemo(() => {
return [...latestEvents, ...events]
}, [events, latestEvents])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/custom/DrawerJobEvents.tsx` around lines 70 - 72, The useMemo
for allEvents includes redundant dependencies (latestEvents.length and
events.length); update the dependency array to only include latestEvents and
events so it becomes useMemo(() => [...latestEvents, ...events], [latestEvents,
events]) — modify the dependency list around the allEvents declaration to remove
the .length entries while keeping the useMemo logic unchanged.


const rowVirtualizer = useVirtualizer({
count: allEvents.length,
getScrollElement: () => parentRef.current,
estimateSize: () => 80,
overscan: 40,
gap: 8,
lanes: 1,
enabled: allEvents.length > 0,
initialOffset: 0,
getItemKey: index => String(allEvents[index].id),
})
Comment thread
moda20 marked this conversation as resolved.

const refreshVirtualizer = useCallback(() => {
setTimeout(() => {
// TODO find another solution for this timeout issue :
// the allEvent array although is always updated, doesn't trigger the virtualizer getVirtualItems() function to return filled item array
rowVirtualizer.measure()
}, 200)
}, [rowVirtualizer])
Comment on lines +86 to +92
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Acknowledged workaround with setTimeout.

The PR notes mention this timeout is needed because the virtualizer doesn't refresh correctly when the event list updates. This is likely related to the key={allEvents.length} forcing remounts and the timing of when the virtualizer measures.

Consider addressing the root cause by:

  1. Removing the key={allEvents.length} from line 149
  2. Using useEffect to call rowVirtualizer.measure() when allEvents changes

This would be more React-idiomatic than the current onOpenChange + setTimeout approach.

Would you like me to help draft an alternative implementation using useEffect to handle virtualizer refresh more reliably?

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/custom/DrawerJobEvents.tsx` around lines 86 - 92, The
workaround using setTimeout in refreshVirtualizer and the remount via
key={allEvents.length} causes unreliable virtualizer updates; remove the
key={allEvents.length} and replace the timeout-based refresh by adding a
useEffect that watches allEvents (or its length/identity) and calls
rowVirtualizer.measure() when it changes (while keeping refreshVirtualizer for
manual triggers if needed); ensure you reference rowVirtualizer.measure and the
allEvents state inside the effect and clean up any stale closures by including
rowVirtualizer in the effect dependency array.


return (
<SheetActionDialog
side={"right"}
Expand All @@ -71,6 +99,8 @@ export default function DrawerJobEvents({
trigger={trigger}
modal={true}
contentClassName="w-[600px] sm:w-[800px] sm:max-w-[800px]"
onOpenChange={refreshVirtualizer}
innerContainerClassName={"gap-2"}
>
<div className="mt-2 flex gap-2 w-full">
<ManagedSimpleDropdown
Expand Down Expand Up @@ -106,36 +136,45 @@ export default function DrawerJobEvents({
</ButtonWithTooltip>
</div>
</div>
<ScrollArea>
<div className={"flex flex-col gap-2 py-4"}>
{latestEvents.length > 0 &&
latestEvents.map(ev => (
<EventItem
key={ev.id}
timestamp={ev.created_at}
message={ev.event_message}
type={ev.type}
onHandle={setEventsToRead(ev.id)}
handled={ev.handled}
handleTime={ev.handled_on}
job={JobDetails}
/>
))}
{latestEvents.length > 0 && <Separator />}
{events.map(ev => (
<EventItem
key={ev.id}
timestamp={ev.created_at}
message={ev.event_message}
type={ev.type}
onHandle={setEventsToRead(ev.id)}
handled={ev.handled}
handleTime={ev.handled_on}
job={JobDetails}
/>
))}
<LoadingOverlay isLoading={loading}>
<div
ref={parentRef}
className="overflow-auto h-[calc(100vh-10rem)] w-full"
>
<div
className="relative w-full py-4"
style={{
height: `${rowVirtualizer.getTotalSize()}px`,
}}
key={allEvents.length}
>
{rowVirtualizer.getVirtualItems().map(virtualRow => {
const ev = allEvents[virtualRow.index]
return (
<div
className="absolute w-full"
style={{
transform: `translateY(${virtualRow.start}px)`,
}}
key={virtualRow.key}
data-index={virtualRow.index}
ref={rowVirtualizer.measureElement}
>
<EventItem
timestamp={ev.created_at}
message={ev.event_message}
type={ev.type}
onHandle={setEventsToRead(ev.id)}
handled={ev.handled}
handleTime={ev.handled_on}
job={JobDetails}
/>
</div>
)
})}
</div>
</div>
</ScrollArea>
</LoadingOverlay>
</SheetActionDialog>
)
}
22 changes: 19 additions & 3 deletions src/components/custom/DrawerJobFiles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import moment from "moment"
import { humanFileSize } from "@/utils/numberUtils"
import { ScrollArea } from "@/components/ui/scroll-area"
import { ButtonGroup } from "@/components/ui/buttonGroup"
import ConfirmationDialogAction from "@/components/confirmationDialogAction"
import ConfirmationDialogAction, {
ConfirmationDialogActionType,
} from "@/components/confirmationDialogAction"
import { toast } from "@/hooks/use-toast"
import type { CacheFile, jobLog, OutputFile } from "@/models/cacheFiles"
import DrawerFilePreview from "@/components/custom/DrawerFilePreview"
Expand Down Expand Up @@ -130,6 +132,16 @@ export default function DrawerJobFiles({
[JobDetails, cacheFilesListRef.current],
)

const handleConfirmAction = useCallback(
(inputFunction: any) => {
return (action: ConfirmationDialogActionType) => {
if (action === ConfirmationDialogActionType.CANCEL) return
return inputFunction(action)
}
},
[deleteFile, downloadFile],
)

useEffect(() => {
getFirstFileList()
}, [])
Expand Down Expand Up @@ -297,7 +309,9 @@ export default function DrawerJobFiles({
<ConfirmationDialogAction
title={"Delete cache file"}
description={`This will delete the ${item.file_name} cache file permanently`}
takeAction={() => deleteFile(item, "cache", index)}
takeAction={handleConfirmAction(() =>
deleteFile(item, "cache", index),
)}
confirmText={"Delete cache file"}
confirmVariant={"destructive"}
>
Expand Down Expand Up @@ -397,7 +411,9 @@ export default function DrawerJobFiles({
<ConfirmationDialogAction
title={"Delete output file"}
description={`This will delete the ${item.file_name} output file permanently`}
takeAction={() => deleteFile(item, "output", index)}
takeAction={handleConfirmAction(() =>
deleteFile(item, "output", index),
)}
confirmText={"Delete output file"}
confirmVariant={"destructive"}
>
Expand Down
Loading