Refactor useScheduleJob and Pipeline Steps rendering (#612)

web/src/hooks/useScheduleJob.ts

@@ -0,0 +1,87 @@
+/*
+ * Copyright 2023 Harness, Inc.
+ *
+ * Licensed 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 { useCallback, useEffect, useRef } from 'react'
+
+interface UseScheduleJobProps<T> {
+  handler: (items: T[]) => void
+  initialData?: T[]
+  isStreaming?: boolean
+  maxProcessingBlockSize?: number
+}
+
+export function useScheduleJob<T>({
+  handler,
+  initialData = [],
+  maxProcessingBlockSize = DEFAULT_PROCESSING_BLOCK_SIZE,
+  isStreaming = false
+}: UseScheduleJobProps<T>) {
+  const progressRef = useRef(false)
+  const rAFRef = useRef(0)
+  const data = useRef(initialData)
+  const scheduleJob = useCallback(() => {
+    if (progressRef.current || !data.current.length) {
+      return
+    }
+
+    cancelAnimationFrame(rAFRef.current)
+    progressRef.current = true
+
+    rAFRef.current = requestAnimationFrame(() => {
+      try {
+        // Process aggressively when data set is large, then slow down to one
+        // item at a time when it is small to improve (scrolling) experience
+        const size = isStreaming
+          ? data.current.length > maxProcessingBlockSize
+            ? maxProcessingBlockSize
+            : 1
+          : maxProcessingBlockSize
+
+        // Cut a block to handler to process
+        handler(data.current.splice(0, size))
+
+        // When handler is done, turn off in-progress flag
+        progressRef.current = false
+
+        // Process the next block, if there's still data
+        if (data.current.length) {
+          scheduleJob()
+        }
+      } catch (error) {
+        console.error('An error has occurred', error) // eslint-disable-line no-console
+      }
+    })
+  }, [handler, maxProcessingBlockSize, isStreaming])
+
+  useEffect(() => {
+    return () => cancelAnimationFrame(rAFRef.current)
+  }, [])
+
+  return useCallback(
+    function sendDataToScheduler(item: T | T[]) {
+      if (Array.isArray(item)) {
+        data.current.push(...item)
+      } else {
+        data.current.push(item)
+      }
+
+      scheduleJob()
+    },
+    [scheduleJob]
+  )
+}
+
+const DEFAULT_PROCESSING_BLOCK_SIZE = 60

web/src/hooks/useScheduleRendering.ts

@@ -1,60 +0,0 @@
-/*
- * Copyright 2023 Harness, Inc.
- *
- * Licensed 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 { useCallback, useRef } from 'react'
-
-interface UseSchedueRenderingProps<T> {
-  renderer: (items: T[]) => void
-  size?: number
-  initialData?: T[]
-}
-
-export function useScheduleRendering<T>({ renderer, size = 50, initialData = [] }: UseSchedueRenderingProps<T>) {
-  const progressRef = useRef(false)
-  const rafRef = useRef(0)
-  const data = useRef<T[]>(initialData)
-  const startRendering = useCallback(() => {
-    if (progressRef.current || !data.current.length) {
-      return
-    }
-
-    cancelAnimationFrame(rafRef.current)
-    progressRef.current = true
-
-    rafRef.current = requestAnimationFrame(() => {
-      try {
-        renderer(data.current.splice(0, size))
-        progressRef.current = false
-
-        if (data.current.length) {
-          startRendering()
-        }
-      } catch (error) {
-        console.error('An error has occurred', error) // eslint-disable-line no-console
-      }
-    })
-  }, [renderer, size])
-
-  const sendDataToRender = useCallback(
-    (item: T) => {
-      data.current.push(item)
-      startRendering()
-    },
-    [startRendering]
-  )
-
-  return sendDataToRender
-}

web/src/layouts/menu/NavMenuItem.module.scss

@@ -81,7 +81,8 @@
     color: var(--grey-700);
   }
 
-  &.selected .customIcon {
+  &.selected .customIcon,
+  &.highlighted .customIcon {
     color: var(--primary-8);
   }
 

web/src/pages/PullRequest/Checks/CheckPipelineSteps.tsx

@@ -28,7 +28,7 @@ import { ButtonRoleProps, getErrorMessage, timeDistance } from 'utils/Utils'
 import type { GitInfoProps } from 'utils/GitUtils'
 import type { LivelogLine, TypesStage, TypesStep } from 'services/code'
 import { ExecutionState, ExecutionStatus } from 'components/ExecutionStatus/ExecutionStatus'
-import { useScheduleRendering } from 'hooks/useScheduleRendering'
+import { useScheduleJob } from 'hooks/useScheduleJob'
 import { useShowRequestError } from 'hooks/useShowRequestError'
 import css from './Checks.module.scss'
 
@@ -105,23 +105,40 @@ const CheckPipelineStep: React.FC<CheckPipelineStepsProps & { step: TypesStep }>
       }
     }
   }, [])
-  const streamLogsRenderer = useCallback((_logs: string[]) => {
-    const logContainer = containerRef.current as HTMLDivElement
-    const fragment = new DocumentFragment()
+  const sendStreamLogToRenderer = useScheduleJob({
+    handler: useCallback((blocks: string[]) => {
+      const logContainer = containerRef.current as HTMLDivElement
 
-    _logs.forEach(_log => fragment.appendChild(createLogLineElement(_log)))
+      if (logContainer) {
+        const fragment = new DocumentFragment()
 
-    const scrollParent = logContainer?.closest(`.${css.content}`) as HTMLDivElement
-    const autoScroll = scrollParent && scrollParent.scrollTop === scrollParent.scrollHeight - scrollParent.offsetHeight
+        blocks.forEach(block => fragment.appendChild(createLogLineElement(block)))
 
-    logContainer.appendChild(fragment)
+        const scrollParent = logContainer.closest(`.${css.content}`) as HTMLDivElement
+        const autoScroll =
+          scrollParent && scrollParent.scrollTop === scrollParent.scrollHeight - scrollParent.offsetHeight
 
-    if (autoScroll) {
-      scrollParent.scrollTop = scrollParent.scrollHeight
-    }
-  }, [])
+        logContainer?.appendChild(fragment)
 
-  const sendStreamingDataToRender = useScheduleRendering({ renderer: streamLogsRenderer })
+        if (autoScroll) {
+          scrollParent.scrollTop = scrollParent.scrollHeight
+        }
+      }
+    }, []),
+    isStreaming: true
+  })
+  const sendCompleteLogsToRenderer = useScheduleJob({
+    handler: useCallback((blocks: string[]) => {
+      const logContainer = containerRef.current as HTMLDivElement
+
+      if (logContainer) {
+        const fragment = new DocumentFragment()
+        blocks.forEach(block => fragment.appendChild(createLogLineElement(block)))
+        logContainer.appendChild(fragment)
+      }
+    }, []),
+    maxProcessingBlockSize: 100
+  })
 
   useEffect(() => {
     if (expanded && isRunning) {
@@ -134,7 +151,7 @@ const CheckPipelineStep: React.FC<CheckPipelineStepsProps & { step: TypesStep }>
       eventSourceRef.current = new EventSource(`${path}/stream`)
       eventSourceRef.current.onmessage = event => {
         try {
-          sendStreamingDataToRender((JSON.parse(event.data) as LivelogLine).out || '')
+          sendStreamLogToRenderer((JSON.parse(event.data) as LivelogLine).out || '')
         } catch (exception) {
           showError(getErrorMessage(exception))
           closeEventStream()
@@ -151,13 +168,17 @@ const CheckPipelineStep: React.FC<CheckPipelineStepsProps & { step: TypesStep }>
     }
 
     return closeEventStream
-  }, [expanded, isRunning, showError, path, step.status, closeEventStream, sendStreamingDataToRender])
+  }, [expanded, isRunning, showError, path, step.status, closeEventStream, sendStreamLogToRenderer])
 
   useEffect(() => {
-    if (!lazy && !error && (!isStreamingDone || !isRunning) && expanded) {
-      refetch()
+    if (!lazy && !error && !isRunning && !isStreamingDone && expanded) {
+      if (!logs) {
+        refetch()
+      } else {
+        sendCompleteLogsToRenderer(logs.map(({ out = '' }) => out))
+      }
     }
-  }, [lazy, error, refetch, isStreamingDone, expanded, isRunning])
+  }, [lazy, error, logs, refetch, isStreamingDone, expanded, isRunning, sendCompleteLogsToRenderer])
 
   useEffect(() => {
     if (autoCollapse && expanded && step.status === ExecutionState.SUCCESS) {
@@ -168,12 +189,9 @@ const CheckPipelineStep: React.FC<CheckPipelineStepsProps & { step: TypesStep }>
 
   useEffect(() => {
     if (!isRunning && logs?.length) {
-      logs.forEach(_log => {
-        const element = createLogLineElement(_log.out)
-        containerRef.current?.appendChild(element)
-      })
+      sendCompleteLogsToRenderer(logs.map(({ out = '' }) => out))
     }
-  }, [isRunning, logs])
+  }, [isRunning, logs, sendCompleteLogsToRenderer])
 
   useShowRequestError(error, 0)
 
@@ -184,6 +202,9 @@ const CheckPipelineStep: React.FC<CheckPipelineStepsProps & { step: TypesStep }>
         className={cx(css.stepHeader, { [css.expanded]: expanded, [css.selected]: expanded })}
         {...ButtonRoleProps}
         onClick={() => {
+          if (expanded && isStreamingDone) {
+            setIsStreamingDone(false)
+          }
           setExpanded(!expanded)
         }}>
         <NavArrowRight color={Utils.getRealCSSColor(Color.GREY_500)} className={cx(css.noShrink, css.chevron)} />

web/src/pages/PullRequest/Checks/Checks.module.scss

@@ -284,6 +284,11 @@
       cursor: text;
       margin: 0;
       padding: 0;
+
+      &:empty {
+        display: inline-block;
+        min-height: 20px;
+      }
     }
   }
 }

web/src/pages/PullRequest/PullRequest.module.scss

@@ -17,7 +17,8 @@
 .main {
   min-height: var(--page-height);
 
-  > div[class*='PageHeader'] {
+  > div[class*='PageHeader'],
+  [data-testid='page-header'] {
     border-bottom: none !important;
   }
 }