Render off-screen diffs in a single pre tag to boost performance (#2271)

* Tan's last PR :)
* Sync with main
* Prevent browser crash on big PR (>200 files changed)
* Fixed link to comment not working
* Add NPE prevention
* Fixed Show Diff works only once
* Fix Harness nav is not auto-collapsed when visiting PR Changes
* Add code to restore new and edited comments
* Render off-screen diffs in a single pre tag to boost performance
This commit is contained in:
Tan Nhu 2024-08-09 17:41:59 +00:00 committed by Harness
parent 68af08e69e
commit f626dc640e
10 changed files with 355 additions and 95 deletions

View File

@ -31,7 +31,7 @@ export default {
PULL_REQUEST_DIFF_RENDERING_BLOCK_SIZE: 10,
/** Detection margin for on-screen / off-screen rendering optimization. In pixels. */
IN_VIEWPORT_DETECTION_MARGIN: 5_000,
IN_VIEWPORT_DETECTION_MARGIN: 256_000,
/** Limit for the secret input in bytes */
SECRET_LIMIT_IN_BYTES: 5_242_880

View File

@ -390,7 +390,7 @@ const ChangesInternal: React.FC<ChangesProps> = ({
}
scheduleTask(() => {
if (isMounted.current && loopCount++ < 50) {
if (isMounted.current && loopCount++ < 100) {
if (
!outterBlockDOM ||
!innerBlockDOM ||
@ -548,14 +548,16 @@ const ChangesInternal: React.FC<ChangesProps> = ({
key={key}
blockName={outterBlockName(blockIndex)}
root={scrollElementRef as RefObject<Element>}
shouldRetainChildren={shouldRetainDiffChildren}>
shouldRetainChildren={shouldRetainDiffChildren}
detectionMargin={calculateDetectionMargin(diffs?.length as number)}>
{diffsBlock.map((diff, index) => {
return (
<InViewDiffBlockRenderer
key={key + index}
blockName={innerBlockName(diff.filePath)}
root={diffsContainerRef}
shouldRetainChildren={shouldRetainDiffChildren}>
shouldRetainChildren={shouldRetainDiffChildren}
detectionMargin={Config.IN_VIEWPORT_DETECTION_MARGIN}>
<DiffViewer
readOnly={readOnly || (commitRange?.length || 0) > 0} // render in readonly mode in case a commit is selected
diff={diff}
@ -618,6 +620,12 @@ const outterBlockName = (blockIndex: number) => `outter-${blockIndex}`
const innerBlockName = (filePath: string) => `inner-${filePath}`
const { scheduleTask } = createRequestIdleCallbackTaskPool()
// If there are more than 200 diffs, we decrease the detection margin to make sure browser do not crash. As a result, Cmd-F
// won't work well on diffs that got hidden/out of viewport.
// TODO: This could be more accurate to calculate based on the complexity of the diff contents (added/deleted lines)?
const calculateDetectionMargin = (diffsLength: number) =>
diffsLength >= 200 ? 5000 : Config.IN_VIEWPORT_DETECTION_MARGIN
// Workaround util to correct filePath which is not correctly produced by
// git itself when filename contains space
// @see https://stackoverflow.com/questions/77596606/why-does-git-add-trailing-tab-to-the-b-line-of-the-diff-when-the-file-nam

View File

@ -14,7 +14,7 @@
* limitations under the License.
*/
import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
import React, { createRef, useCallback, useEffect, useMemo, useRef, useState } from 'react'
import type { EditorView } from '@codemirror/view'
import { Render, Match, Truthy, Falsy, Else } from 'react-jsx-match'
import { Container, Layout, Avatar, TextInput, Text, FlexExpander, Button, useIsMounted } from '@harnessio/uicore'
@ -34,6 +34,7 @@ import { ButtonRoleProps, CodeCommentState } from 'utils/Utils'
import { useResizeObserver } from 'hooks/useResizeObserver'
import { useCustomEventListener } from 'hooks/useEventListener'
import type { SuggestionBlock } from 'components/SuggestionBlock/SuggestionBlock'
import type { CommentRestorationTrackingState, DiffViewerExchangeState } from 'components/DiffViewer/DiffViewer'
import commentActiveIconUrl from './comment.svg?url'
import commentResolvedIconUrl from './comment-resolved.svg?url'
import css from './CommentBox.module.scss'
@ -83,6 +84,7 @@ interface CommentBoxProps<T> {
resetOnSave?: boolean
hideCancel?: boolean
currentUserName: string
commentThreadId?: number
commentItems: CommentItem<T>[]
handleAction: (
action: CommentAction,
@ -99,6 +101,8 @@ interface CommentBoxProps<T> {
routingId: string
copyLinkToComment: (commentId: number, commentItem: CommentItem<T>) => void
suggestionBlock?: SuggestionBlock
memorizedState?: CommentRestorationTrackingState
commentsVisibilityAtLineNumber?: DiffViewerExchangeState['commentsVisibilityAtLineNumber']
}
const CommentBoxInternal = <T = unknown,>({
@ -109,6 +113,7 @@ const CommentBoxInternal = <T = unknown,>({
initialContent = '',
width,
fluid,
commentThreadId,
commentItems = [],
currentUserName,
handleAction,
@ -123,7 +128,9 @@ const CommentBoxInternal = <T = unknown,>({
standalone,
routingId,
copyLinkToComment,
suggestionBlock
suggestionBlock,
memorizedState,
commentsVisibilityAtLineNumber
}: CommentBoxProps<T>) => {
const { getString } = useStrings()
const [comments, setComments] = useState<CommentItem<T>[]>(commentItems)
@ -134,6 +141,13 @@ const CommentBoxInternal = <T = unknown,>({
const containerRef = useRef<HTMLDivElement>(null)
const isMounted = useIsMounted()
const clearMemorizedState = useCallback(() => {
if (memorizedState) {
delete memorizedState.showReplyPlaceHolder
delete memorizedState.uncommittedText
}
}, [memorizedState])
useResizeObserver(
containerRef,
useCallback(
@ -156,10 +170,13 @@ const CommentBoxInternal = <T = unknown,>({
const _onCancel = useCallback(() => {
setMarkdown('')
setShowReplyPlaceHolder(true)
clearMemorizedState()
if (onCancel && !comments.length) {
onCancel()
}
}, [setShowReplyPlaceHolder, onCancel, comments.length])
}, [setShowReplyPlaceHolder, onCancel, comments.length, clearMemorizedState])
const hidePlaceHolder = useCallback(() => setShowReplyPlaceHolder(false), [setShowReplyPlaceHolder])
const onQuote = useCallback((content: string) => {
const replyContent = content
@ -179,13 +196,73 @@ const CommentBoxInternal = <T = unknown,>({
})
}, [dirties, setDirty])
useEffect(
// This function restores CommentBox internal states from memorizedState
// after it got destroyed during HTML/textContent serialization/deserialization
// This approach is not optimized, we probably have to think about a shared
// store per diff or something else to make the flow nicer
function serializeNewCommentInfo() {
if (!commentThreadId || !memorizedState) return
if (commentThreadId < 0) {
if (!comments?.[0]?.id) {
if (!markdown && memorizedState.uncommittedText) {
setMarkdown(memorizedState.uncommittedText)
viewRef.current?.dispatch({
changes: {
from: 0,
to: viewRef.current.state.doc.length,
insert: memorizedState.uncommittedText
}
})
viewRef.current?.contentDOM?.blur()
} else {
memorizedState.uncommittedText = markdown
memorizedState.showReplyPlaceHolder = showReplyPlaceHolder
}
} else {
clearMemorizedState()
}
} else if (commentThreadId > 0) {
if (!showReplyPlaceHolder) {
if (markdown) {
memorizedState.uncommittedText = markdown
memorizedState.showReplyPlaceHolder = false
}
} else {
if (!markdown && memorizedState.showReplyPlaceHolder === false) {
setShowReplyPlaceHolder(false)
const { uncommittedText = '' } = memorizedState
setTimeout(() => {
setMarkdown(uncommittedText)
viewRef.current?.dispatch({
changes: {
from: 0,
to: viewRef.current.state.doc.length,
insert: uncommittedText
}
})
viewRef.current?.contentDOM?.blur()
}, 0)
}
delete memorizedState.showReplyPlaceHolder
delete memorizedState.uncommittedText
}
}
},
[markdown, commentThreadId, comments, memorizedState, clearMemorizedState, showReplyPlaceHolder]
)
return (
<Container
className={cx(css.main, { [css.fluid]: fluid }, outerClassName)}
padding={!fluid ? 'medium' : undefined}
width={width}
ref={containerRef}
data-comment-thread-id={comments?.[0]?.id || ''}>
data-comment-thread-id={comments?.[0]?.id || commentThreadId || ''}>
{outlets[CommentBoxOutletPosition.TOP]}
<Container className={cx(boxClassName, css.box)}>
<Layout.Vertical>
@ -210,6 +287,8 @@ const CommentBoxInternal = <T = unknown,>({
outlets={outlets}
copyLinkToComment={copyLinkToComment}
suggestionBlock={suggestionBlock}
memorizedState={memorizedState}
commentsVisibilityAtLineNumber={commentsVisibilityAtLineNumber}
/>
<Match expr={showReplyPlaceHolder && enableReplyPlaceHolderRef.current}>
<Truthy>
@ -250,6 +329,8 @@ const CommentBoxInternal = <T = unknown,>({
value={markdown}
onChange={setMarkdown}
onSave={async (value: string) => {
clearMemorizedState()
if (handleAction) {
const [result, updatedItem] = await handleAction(
comments.length ? CommentAction.REPLY : CommentAction.NEW,
@ -306,7 +387,13 @@ const CommentBoxInternal = <T = unknown,>({
interface CommentsThreadProps<T>
extends Pick<
CommentBoxProps<T>,
'commentItems' | 'handleAction' | 'outlets' | 'copyLinkToComment' | 'suggestionBlock'
| 'commentItems'
| 'handleAction'
| 'outlets'
| 'copyLinkToComment'
| 'suggestionBlock'
| 'memorizedState'
| 'commentsVisibilityAtLineNumber'
> {
onQuote: (content: string) => void
setDirty: (index: number, dirty: boolean) => void
@ -321,21 +408,26 @@ const CommentsThread = <T = unknown,>({
outlets = {},
repoMetadata,
copyLinkToComment,
suggestionBlock
suggestionBlock,
memorizedState,
commentsVisibilityAtLineNumber
}: CommentsThreadProps<T>) => {
const { getString } = useStrings()
const { standalone, routingId } = useAppContext()
const [editIndexes, setEditIndexes] = useState<Record<number, boolean>>({})
const resetStateAtIndex = useCallback(
(index: number) => {
(index: number, commentItem: CommentItem<T>) => {
delete editIndexes[index]
setEditIndexes({ ...editIndexes })
if (memorizedState?.uncommittedEditComments && commentItem?.id) {
memorizedState.uncommittedEditComments.delete(commentItem.id)
}
},
[editIndexes]
[editIndexes, memorizedState]
)
const isCommentThreadResolved = useMemo(() => !!get(commentItems[0], 'payload.resolved'), [commentItems])
const domRef = useRef<HTMLElement>()
const show = useRef(isCommentThreadResolved ? false : true)
const internalFlags = useRef({ initialized: false })
useEffect(
@ -353,18 +445,18 @@ const CommentsThread = <T = unknown,>({
const lineNumColDOM = annotatedRow.firstElementChild as HTMLElement
const sourceLineNumber = annotatedRow.dataset.sourceLineNumber
const button: HTMLButtonElement = lineNumColDOM?.querySelector('button') || document.createElement('button')
const showFromMemory = commentsVisibilityAtLineNumber?.get(Number(sourceLineNumber))
let show = showFromMemory !== undefined ? showFromMemory : isCommentThreadResolved ? false : true
if (!button.onclick) {
const toggleHidden = (dom: Element) => {
if (show.current) dom.setAttribute('hidden', '')
if (show) dom.setAttribute('hidden', '')
else dom.removeAttribute('hidden')
}
const toggleComments = (e: KeyboardEvent | MouseEvent) => {
let commentRow = annotatedRow.nextElementSibling as HTMLElement
while (commentRow?.dataset?.annotatedLine) {
toggleHidden(commentRow)
// Toggle opposite place-holder as well
const diffParent = commentRow.closest('.d2h-code-wrapper')?.parentElement
const oppositeDiv = diffParent?.classList.contains('right')
@ -376,11 +468,16 @@ const CommentsThread = <T = unknown,>({
oppositePlaceHolders?.forEach(dom => toggleHidden(dom))
toggleHidden(commentRow)
commentRow = commentRow.nextElementSibling as HTMLElement
}
show.current = !show.current
show = !show
if (!show.current) button.dataset.threadsCount = String(activeThreads + resolvedThreads)
if (memorizedState) {
commentsVisibilityAtLineNumber?.set(Number(sourceLineNumber), show)
}
if (!show) button.dataset.threadsCount = String(activeThreads + resolvedThreads)
else delete button.dataset.threadsCount
e.stopPropagation()
@ -388,6 +485,7 @@ const CommentsThread = <T = unknown,>({
button.classList.add(css.toggleComment)
button.title = getString('pr.toggleComments')
button.dataset.toggleComment = 'true'
button.addEventListener('keydown', e => {
if (e.key === 'Enter') toggleComments(e)
@ -404,7 +502,9 @@ const CommentsThread = <T = unknown,>({
while (commentRow?.dataset?.annotatedLine) {
if (commentRow.dataset.commentThreadStatus == CodeCommentState.RESOLVED) {
resolvedThreads++
if (!internalFlags.current.initialized) show.current = false
if (!internalFlags.current.initialized && !showFromMemory) {
show = false
}
} else activeThreads++
commentRow = commentRow.nextElementSibling as HTMLElement
@ -415,19 +515,44 @@ const CommentsThread = <T = unknown,>({
if (!internalFlags.current.initialized) {
internalFlags.current.initialized = true
if (!show.current && resolvedThreads) button.dataset.threadsCount = String(resolvedThreads)
if (!show && resolvedThreads) button.dataset.threadsCount = String(resolvedThreads)
else delete button.dataset.threadsCount
}
}
},
[isCommentThreadResolved, getString]
[isCommentThreadResolved, getString, commentsVisibilityAtLineNumber, memorizedState]
)
const viewRefs = useRef(
Object.fromEntries(
commentItems.map(commentItem => [commentItem.id, createRef() as React.MutableRefObject<EditorView | undefined>])
)
)
const contentRestoredRefs = useRef<Record<number, boolean>>({})
return (
<Render when={commentItems.length}>
<Container className={css.viewer} padding="xlarge" ref={domRef}>
{commentItems.map((commentItem, index) => {
const isLastItem = index === commentItems.length - 1
const contentFromMemorizedState = memorizedState?.uncommittedEditComments?.get(commentItem.id)
const viewRef = viewRefs.current[commentItem.id]
if (viewRef && contentFromMemorizedState !== undefined && !contentRestoredRefs.current[commentItem.id]) {
editIndexes[index] = true
contentRestoredRefs.current[commentItem.id] = true
setTimeout(() => {
if (contentFromMemorizedState !== commentItem.content) {
viewRef.current?.dispatch({
changes: {
from: 0,
to: viewRef.current.state.doc.length,
insert: contentFromMemorizedState
}
})
}
}, 0)
}
return (
<ThreadSection
@ -487,7 +612,14 @@ const CommentsThread = <T = unknown,>({
className: cx(css.optionMenuIcon, css.edit),
iconName: 'Edit',
text: getString('edit'),
onClick: () => setEditIndexes({ ...editIndexes, ...{ [index]: true } })
onClick: () => {
setEditIndexes({ ...editIndexes, ...{ [index]: true } })
if (memorizedState) {
memorizedState.uncommittedEditComments =
memorizedState.uncommittedEditComments || new Map()
memorizedState.uncommittedEditComments.set(commentItem.id, commentItem.content)
}
}
},
{
hasIcon: true,
@ -512,7 +644,7 @@ const CommentsThread = <T = unknown,>({
text: getString('delete'),
onClick: async () => {
if (await handleAction(CommentAction.DELETE, '', commentItem)) {
resetStateAtIndex(index)
resetStateAtIndex(index, commentItem)
}
}
}
@ -538,13 +670,20 @@ const CommentsThread = <T = unknown,>({
standalone={standalone}
repoMetadata={repoMetadata}
value={commentItem?.content}
viewRef={viewRefs.current[commentItem.id]}
onSave={async value => {
if (await handleAction(CommentAction.UPDATE, value, commentItem)) {
commentItem.content = value
resetStateAtIndex(index)
resetStateAtIndex(index, commentItem)
}
}}
onCancel={() => resetStateAtIndex(index)}
onChange={value => {
if (memorizedState) {
memorizedState.uncommittedEditComments = memorizedState.uncommittedEditComments || new Map()
memorizedState.uncommittedEditComments.set(commentItem.id, value)
}
}}
onCancel={() => resetStateAtIndex(index, commentItem)}
setDirty={_dirty => {
setDirty(index, _dirty)
}}
@ -555,7 +694,7 @@ const CommentsThread = <T = unknown,>({
save: getString('save'),
cancel: getString('cancel')
}}
autoFocusAndPosition
autoFocusAndPosition={contentFromMemorizedState ? false : true}
suggestionBlock={suggestionBlock}
/>
</Container>

View File

@ -1 +1 @@
<svg fill="none" viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><mask id="a" height="20" maskUnits="userSpaceOnUse" width="20" x="0" y="0"><path clip-rule="evenodd" d="m10 20c5.5228 0 10-4.4772 10-10 0-5.52285-4.4772-10-10-10-5.52285 0-10 4.47715-10 10 0 5.5228 4.47715 10 10 10z" fill="#fff" fill-rule="evenodd"/></mask><mask id="b" fill="#fff"><path clip-rule="evenodd" d="m7.33325 7.91797c-.27614 0-.5.22386-.5.5s.22386.5.5.5h5.33335c.2761 0 .5-.22386.5-.5s-.2239-.5-.5-.5zm0 1.83333c-.27614 0-.5.22386-.5.5 0 .2761.22386.5.5.5h4.66665c.2762 0 .5-.2239.5-.5 0-.27614-.2238-.5-.5-.5z" fill="#fff" fill-rule="evenodd"/></mask><g opacity=".9"><path clip-rule="evenodd" d="m10 20c5.5228 0 10-4.4772 10-10 0-5.52285-4.4772-10-10-10-5.52285 0-10 4.47715-10 10 0 5.5228 4.47715 10 10 10z" fill="#d8d8d8" fill-rule="evenodd"/><g mask="url(#a)"><path d="m0 0h20v20h-20z" fill="#ff661a"/><path d="m7.33341 5.33203h5.33329c1.4728 0 2.6667 1.19391 2.6667 2.66667v2.6667c0 1.4727-1.1939 2.6666-2.6667 2.6666h-2.6666l-2.66669 2v-2c-1.47275 0-2.66666-1.1939-2.66666-2.6666v-2.6667c0-1.47276 1.19391-2.66667 2.66666-2.66667z" stroke="#f3f3fa" stroke-linecap="round" stroke-linejoin="round"/><path d="m7.83325 8.41797c0 .27614-.22386.5-.5.5v-2c-.82843 0-1.5.67157-1.5 1.5zm-.5-.5c.27614 0 .5.22386.5.5h-2c0 .82843.67157 1.5 1.5 1.5zm5.33335 0h-5.33335v2h5.33335zm-.5.5c0-.27614.2238-.5.5-.5v2c.8284 0 1.5-.67157 1.5-1.5zm.5.5c-.2762 0-.5-.22386-.5-.5h2c0-.82843-.6716-1.5-1.5-1.5zm-5.33335 0h5.33335v-2h-5.33335zm.5 1.33333c0 .2761-.22386.5-.5.5v-2c-.82843 0-1.5.67157-1.5 1.5zm-.5-.5c.27614 0 .5.22386.5.5h-2c0 .8284.67157 1.5 1.5 1.5zm4.66665 0h-4.66665v2h4.66665zm-.5.5c0-.27614.2239-.5.5-.5v2c.8284 0 1.5-.6716 1.5-1.5zm.5.5c-.2761 0-.5-.2239-.5-.5h2c0-.82843-.6716-1.5-1.5-1.5zm-4.66665 0h4.66665v-2h-4.66665z" fill="#f3f3fa" mask="url(#b)" opacity=".5"/></g></g></svg>
<svg fill="none" viewBox="0 0 20 20" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><mask id="a" height="20" maskUnits="userSpaceOnUse" width="20" x="0" y="0"><path clip-rule="evenodd" d="m10 20c5.5228 0 10-4.4772 10-10 0-5.52285-4.4772-10-10-10-5.52285 0-10 4.47715-10 10 0 5.5228 4.47715 10 10 10z" fill="#fff" fill-rule="evenodd"/></mask><mask id="b" fill="#fff"><path clip-rule="evenodd" d="m7.33325 7.91797c-.27614 0-.5.22386-.5.5s.22386.5.5.5h5.33335c.2761 0 .5-.22386.5-.5s-.2239-.5-.5-.5zm0 1.83333c-.27614 0-.5.22386-.5.5 0 .2761.22386.5.5.5h4.66665c.2762 0 .5-.2239.5-.5 0-.27614-.2238-.5-.5-.5z" fill="#fff" fill-rule="evenodd"/></mask><g opacity=".9"><path clip-rule="evenodd" d="m10 20c5.5228 0 10-4.4772 10-10 0-5.52285-4.4772-10-10-10-5.52285 0-10 4.47715-10 10 0 5.5228 4.47715 10 10 10z" fill="#d8d8d8" fill-rule="evenodd"/><g mask="url(#a)"><path d="m0 0h20v20h-20z" fill="#1b841d"/><path d="m7.33341 5.33203h5.33329c1.4728 0 2.6667 1.19391 2.6667 2.66667v2.6667c0 1.4727-1.1939 2.6666-2.6667 2.6666h-2.6666l-2.66669 2v-2c-1.47275 0-2.66666-1.1939-2.66666-2.6666v-2.6667c0-1.47276 1.19391-2.66667 2.66666-2.66667z" stroke="#f3f3fa" stroke-linecap="round" stroke-linejoin="round"/><path d="m7.83325 8.41797c0 .27614-.22386.5-.5.5v-2c-.82843 0-1.5.67157-1.5 1.5zm-.5-.5c.27614 0 .5.22386.5.5h-2c0 .82843.67157 1.5 1.5 1.5zm5.33335 0h-5.33335v2h5.33335zm-.5.5c0-.27614.2238-.5.5-.5v2c.8284 0 1.5-.67157 1.5-1.5zm.5.5c-.2762 0-.5-.22386-.5-.5h2c0-.82843-.6716-1.5-1.5-1.5zm-5.33335 0h5.33335v-2h-5.33335zm.5 1.33333c0 .2761-.22386.5-.5.5v-2c-.82843 0-1.5.67157-1.5 1.5zm-.5-.5c.27614 0 .5.22386.5.5h-2c0 .8284.67157 1.5 1.5 1.5zm4.66665 0h-4.66665v2h4.66665zm-.5.5c0-.27614.2239-.5.5-.5v2c.8284 0 1.5-.6716 1.5-1.5zm.5.5c-.2761 0-.5-.2239-.5-.5h2c0-.82843-.6716-1.5-1.5-1.5zm-4.66665 0h4.66665v-2h-4.66665z" fill="#f3f3fa" mask="url(#b)" opacity=".5"/></g></g></svg>

Before

Width:  |  Height:  |  Size: 1.9 KiB

After

Width:  |  Height:  |  Size: 1.9 KiB

View File

@ -465,14 +465,12 @@
border-bottom-right-radius: 4px;
max-width: calc(var(--page-container-width) - 48px);
&.hidden {
height: var(--block-height);
content-visibility: auto;
contain-intrinsic-size: auto var(--line-height);
* {
display: none !important;
}
.offscreenText {
font-size: 12px;
white-space: normal;
line-height: 20px;
margin: 0;
color: transparent;
}
}
}

View File

@ -26,8 +26,8 @@ export declare const expandCollapseDiffBtn: string
export declare const fileChanged: string
export declare const fname: string
export declare const fnamePopover: string
export declare const hidden: string
export declare const main: string
export declare const offscreenText: string
export declare const popover: string
export declare const readOnly: string
export declare const selectoSelection: string

View File

@ -41,15 +41,15 @@ import { useStrings } from 'framework/strings'
import { CodeIcon, GitInfoProps } from 'utils/GitUtils'
import type { DiffFileEntry } from 'utils/types'
import { useAppContext } from 'AppContext'
import type { GitFileDiff, TypesPullReq } from 'services/code'
import type { GitFileDiff, TypesPullReq, TypesPullReqActivity } from 'services/code'
import { CopyButton } from 'components/CopyButton/CopyButton'
import { NavigationCheck } from 'components/NavigationCheck/NavigationCheck'
import type { UseGetPullRequestInfoResult } from 'pages/PullRequest/useGetPullRequestInfo'
import { useQueryParams } from 'hooks/useQueryParams'
import { useCustomEventListener } from 'hooks/useEventListener'
import { useCustomEventListener, useEventListener } from 'hooks/useEventListener'
import { useShowRequestError } from 'hooks/useShowRequestError'
import { getErrorMessage, isInViewport } from 'utils/Utils'
import { createRequestIdleCallbackTaskPool } from 'utils/Task'
import { createRequestAnimationFrameTaskPool } from 'utils/Task'
import { useResizeObserver } from 'hooks/useResizeObserver'
import { useFindGitBranch } from 'hooks/useFindGitBranch'
import Config from 'Config'
@ -58,7 +58,8 @@ import {
DIFF_VIEWER_HEADER_HEIGHT,
ViewStyle,
getFileViewedState,
FileViewedState
FileViewedState,
DiffCommentItem
} from './DiffViewerUtils'
import { usePullReqComments } from './usePullReqComments'
import Collapse from '../../icons/collapse.svg'
@ -165,6 +166,7 @@ const DiffViewerInternal: React.FC<DiffViewerProps> = ({
},
[ref]
)
const contentHTML = useRef<string | null>(null)
useResizeObserver(
contentRef,
@ -178,20 +180,6 @@ const DiffViewerInternal: React.FC<DiffViewerProps> = ({
)
)
useEffect(() => {
let taskId = 0
if (inView) {
taskId = scheduleLowPriorityTask(() => {
if (isMounted.current && contentRef.current) contentRef.current.classList.remove(css.hidden)
})
} else {
taskId = scheduleLowPriorityTask(() => {
if (isMounted.current && contentRef.current) contentRef.current.classList.add(css.hidden)
})
}
return () => cancelTask(taskId)
}, [inView, isMounted])
//
// Handling custom events sent to DiffViewer from external components/features
// such as "jump to file", "jump to comment", etc...
@ -202,6 +190,21 @@ const DiffViewerInternal: React.FC<DiffViewerProps> = ({
const { action, commentId } = event.detail
const containerDOM = document.getElementById(diff.containerId) as HTMLDivElement
function scrollToComment(count = 0) {
const commentDOM = containerDOM.querySelector(`[data-comment-id="${commentId}"]`) as HTMLDivElement
if (!isMounted.current || count > 100) {
return
}
if (commentDOM) {
const dom = commentDOM?.parentElement?.parentElement?.parentElement?.parentElement
if (dom) dom.lastElementChild?.scrollIntoView({ block: 'center' })
} else {
setTimeout(() => scrollToComment(count + 1), 100)
}
}
function scrollToContainer() {
if (!isMounted.current) return
@ -215,10 +218,7 @@ const DiffViewerInternal: React.FC<DiffViewerProps> = ({
scrollElement.scroll({ top: (scrollElement.scrollTop || window.scrollY) + scrollGap })
}
} else {
const commentDOM = containerDOM.querySelector(`[data-comment-id="${commentId}"]`) as HTMLDivElement
// dom is the great grand parent of the comment DOM (CommentBox)
const dom = commentDOM?.parentElement?.parentElement?.parentElement?.parentElement
if (dom) dom.lastElementChild?.scrollIntoView({ block: 'center' })
scrollToComment()
}
}
@ -247,7 +247,8 @@ const DiffViewerInternal: React.FC<DiffViewerProps> = ({
containerRef,
contentRef,
refetchActivities,
setDirty
setDirty,
memorizedState
})
useEffect(
@ -290,7 +291,7 @@ const DiffViewerInternal: React.FC<DiffViewerProps> = ({
if (isInViewport(containerRef.current as Element, 1000)) {
renderDiffAndComments()
} else {
taskId = scheduleLowPriorityTask(renderDiffAndComments)
taskId = scheduleTask(renderDiffAndComments)
}
}
@ -316,6 +317,69 @@ const DiffViewerInternal: React.FC<DiffViewerProps> = ({
const branchInfo = useFindGitBranch(pullReqMetadata?.source_branch)
useEffect(
function serializeDeserializeContent() {
const dom = contentRef.current
if (inView) {
if (isMounted.current && dom && contentHTML.current) {
dom.innerHTML = contentHTML.current
contentHTML.current = null
// Remove all signs from the raw HTML that CommentBox was mounted so
// it can be mounted/re-rendered again freshly
dom.querySelectorAll('tr[data-source-line-number]').forEach(row => {
row.removeAttribute('data-source-line-number')
row.removeAttribute('data-comment-ids')
row.querySelector('button[data-toggle-comment="true"]')?.remove?.()
})
dom.querySelectorAll('tr[data-annotated-line],tr[data-place-holder-for-line]').forEach(row => {
row.remove?.()
})
// Attach comments again
commentsHook.current.attachAllCommentThreads()
}
} else {
if (isMounted.current && dom && !contentHTML.current) {
const { clientHeight, textContent, innerHTML } = dom
// Detach comments since they are no longer in sync in DOM as
// all DOMs are removed
commentsHook.current.detachAllCommentThreads()
// Save current innerHTML
contentHTML.current = innerHTML
const pre = document.createElement('pre')
pre.style.height = clientHeight + 'px'
pre.textContent = textContent
pre.classList.add(css.offscreenText)
dom.textContent = ''
dom.appendChild(pre)
// TODO: Might be good to clean textContent a bit to not include
// diff header info, line numbers, hunk headers, etc...
}
}
},
[inView, isMounted, commentsHook]
)
// Add click event listener from contentRef to handle click event on "Show Diff" button
// This can't be done from the button itself because it got serialized / deserialized from
// text during off-screen optimization (handler will be gone/destroyed)
useEventListener(
'click',
useCallback(function showDiff(event) {
if (((event.target as HTMLElement)?.closest('button') as HTMLElement)?.dataset?.action === ACTION_SHOW_DIFF) {
setRenderCustomContent(false)
}
}, []),
contentRef.current as HTMLDivElement
)
useShowRequestError(fullDiffError, 0)
useEffect(
@ -336,7 +400,10 @@ const DiffViewerInternal: React.FC<DiffViewerProps> = ({
if (memorizedState.get(diff.filePath)?.collapsed) {
setCollapsed(false)
memorizedState.set(diff.filePath, { ...memorizedState.get(diff.filePath), collapsed: false })
memorizedState.set(diff.filePath, {
...memorizedState.get(diff.filePath),
collapsed: false
})
}
} catch (exception) {
showError(getErrorMessage(exception), 0)
@ -505,28 +572,32 @@ const DiffViewerInternal: React.FC<DiffViewerProps> = ({
</Container>
<Container id={diff.contentId} data-path={diff.filePath} className={css.diffContent} ref={contentRef}>
<Render when={renderCustomContent && !collapsed}>
<Container height={200} flex={{ align: 'center-center' }}>
<Layout.Vertical padding="xlarge" style={{ alignItems: 'center' }}>
<Render when={fileDeleted || isDiffTooLarge || diffHasVeryLongLine}>
<Button variation={ButtonVariation.LINK} onClick={() => setRenderCustomContent(false)}>
{getString('pr.showDiff')}
</Button>
</Render>
<Text>
{getString(
fileDeleted
? 'pr.fileDeleted'
: isDiffTooLarge || diffHasVeryLongLine
? 'pr.diffTooLarge'
: isBinary
? 'pr.fileBinary'
: 'pr.fileUnchanged'
)}
</Text>
</Layout.Vertical>
</Container>
</Render>
{/* Note: This parent container is needed to make sure "Show Diff" work correctly
with content converted between textContent and innerHTML */}
<Container>
<Render when={renderCustomContent && !collapsed}>
<Container height={200} flex={{ align: 'center-center' }}>
<Layout.Vertical padding="xlarge" style={{ alignItems: 'center' }}>
<Render when={fileDeleted || isDiffTooLarge || diffHasVeryLongLine}>
<Button variation={ButtonVariation.LINK} data-action={ACTION_SHOW_DIFF}>
{getString('pr.showDiff')}
</Button>
</Render>
<Text>
{getString(
fileDeleted
? 'pr.fileDeleted'
: isDiffTooLarge || diffHasVeryLongLine
? 'pr.diffTooLarge'
: isBinary
? 'pr.fileBinary'
: 'pr.fileUnchanged'
)}
</Text>
</Layout.Vertical>
</Container>
</Render>
</Container>
</Container>
</Layout.Vertical>
<NavigationCheck when={dirty} />
@ -535,6 +606,7 @@ const DiffViewerInternal: React.FC<DiffViewerProps> = ({
}
const BLOCK_HEIGHT = '--block-height'
const ACTION_SHOW_DIFF = 'showDiff'
export enum DiffViewerEvent {
SCROLL_INTO_VIEW = 'scrollIntoView'
@ -549,8 +621,16 @@ export interface DiffViewerExchangeState {
collapsed?: boolean
useFullDiff?: boolean
fullDiff?: DiffFileEntry
comments?: Map<number, CommentRestorationTrackingState>
commentsVisibilityAtLineNumber?: Map<number, boolean>
}
const { scheduleTask: scheduleLowPriorityTask, cancelTask } = createRequestIdleCallbackTaskPool()
export interface CommentRestorationTrackingState extends DiffCommentItem<TypesPullReqActivity> {
uncommittedText?: string
showReplyPlaceHolder?: boolean
uncommittedEditComments?: Map<number, string>
}
const { scheduleTask, cancelTask } = createRequestAnimationFrameTaskPool()
export const DiffViewer = React.memo(DiffViewerInternal)

View File

@ -19,7 +19,6 @@ import cx from 'classnames'
import { Container, useIsMounted } from '@harnessio/uicore'
import { useInView } from 'react-intersection-observer'
import type { FCWithChildren } from 'utils/types'
import Config from 'Config'
import { useResizeObserver } from 'hooks/useResizeObserver'
import css from './InViewDiffBlockRenderer.module.scss'
@ -27,19 +26,21 @@ interface InViewDiffBlockRendererProps {
root: RefObject<Element>
blockName: string
shouldRetainChildren: (containerDOM: HTMLElement | null) => boolean
detectionMargin: number
}
const InViewDiffBlockRendererInternal: FCWithChildren<InViewDiffBlockRendererProps> = ({
root,
blockName,
children,
shouldRetainChildren
shouldRetainChildren,
detectionMargin
}) => {
const containerRef = useRef<HTMLDivElement | null>(null)
const isMounted = useIsMounted()
const { ref, inView } = useInView({
root: root.current,
rootMargin: `${Config.IN_VIEWPORT_DETECTION_MARGIN}px 0px ${Config.IN_VIEWPORT_DETECTION_MARGIN}px 0px`,
rootMargin: `${detectionMargin}px 0px ${detectionMargin}px 0px`,
initialInView: false
})
const setContainerRef = useCallback(

View File

@ -14,7 +14,7 @@
* limitations under the License.
*/
import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'
import React, { useCallback, useEffect, useLayoutEffect, useMemo, useRef } from 'react'
import { useMutate } from 'restful-react'
import Selecto from 'selecto'
import ReactDOM from 'react-dom'
@ -35,6 +35,7 @@ import { CodeCommentStatusSelect } from 'components/CodeCommentStatusSelect/Code
import { dispatchCustomEvent } from 'hooks/useEventListener'
import { UseGetPullRequestInfoResult, usePullReqActivities } from 'pages/PullRequest/useGetPullRequestInfo'
import { CommentThreadTopDecoration } from 'components/CommentThreadTopDecoration/CommentThreadTopDecoration'
import type { DiffViewerExchangeState } from './DiffViewer'
import {
activitiesToDiffCommentItems,
activityToCommentItem,
@ -68,6 +69,7 @@ interface UsePullReqCommentsProps extends Pick<GitInfoProps, 'repoMetadata'> {
contentRef: React.RefObject<HTMLDivElement | null>
refetchActivities?: UseGetPullRequestInfoResult['refetchActivities']
setDirty?: React.Dispatch<React.SetStateAction<boolean>>
memorizedState: Map<string, DiffViewerExchangeState>
}
export function usePullReqComments({
@ -85,7 +87,8 @@ export function usePullReqComments({
containerRef,
contentRef,
refetchActivities,
setDirty
setDirty,
memorizedState
}: UsePullReqCommentsProps) {
const activities = usePullReqActivities()
const { getString } = useStrings()
@ -98,7 +101,18 @@ export function usePullReqComments({
)
const { save, update, remove } = useCommentAPI(commentPath)
const location = useLocation()
const [comments] = useState(new Map<number, DiffCommentItem<TypesPullReqActivity>>())
const comments = useMemo(() => {
let _comments = memorizedState.get(diff.filePath)?.comments
if (!_comments) {
_comments = new Map<number, DiffCommentItem<TypesPullReqActivity>>()
memorizedState.set(diff.filePath, { ...memorizedState.get(diff.filePath), comments: _comments })
}
return _comments
}, [diff.filePath, memorizedState])
const copyLinkToComment = useCallback(
(id, commentItem) => {
const path = `${routes.toCODEPullRequest({
@ -252,8 +266,18 @@ export function usePullReqComments({
commentRowElement.innerHTML = `<td colspan="2"></td>`
lineInfo.rowElement.after(commentRowElement)
if (!memorizedState.get(diff.filePath)?.commentsVisibilityAtLineNumber) {
memorizedState.set(diff.filePath, {
...memorizedState.get(diff.filePath),
commentsVisibilityAtLineNumber: new Map()
})
}
// Get show commments from memorizedState
const showComments = memorizedState.get(diff.filePath)?.commentsVisibilityAtLineNumber?.get(comment.lineNumberEnd)
// Set both place-holder and comment box hidden when comment thread is resolved
if (isCommentThreadResolved) {
if ((showComments === undefined && isCommentThreadResolved) || showComments === false) {
oppositeRowPlaceHolder.setAttribute('hidden', '')
commentRowElement.setAttribute('hidden', '')
}
@ -300,6 +324,12 @@ export function usePullReqComments({
lang: filenameToLanguage(diff.filePath.split('/').pop())
}
// Vars to verified if a CommentBox is restored from innerHTML/textContent optimization
const _memorizedState = memorizedState.get(diff.filePath)?.comments?.get(commentThreadId)
const isCommentBoxRestored =
(commentThreadId && commentThreadId < 0 && _memorizedState?.uncommittedText !== undefined) ||
_memorizedState?.showReplyPlaceHolder === false
// Note: CommentBox is rendered as an independent React component.
// Everything passed to it must be either values, or refs.
// If you pass callbacks or states, they won't be updated and
@ -311,6 +341,7 @@ export function usePullReqComments({
routingId={routingId}
standalone={standalone}
repoMetadata={repoMetadata}
commentThreadId={commentThreadId}
commentItems={comment._commentItems as CommentItem<TypesPullReqActivity>[]}
initialContent={''}
width={getCommentBoxWidth(isSideBySide)}
@ -323,7 +354,7 @@ export function usePullReqComments({
last.style.height = `${boxHeight}px`
}
}}
autoFocusAndPosition={true}
autoFocusAndPosition={isCommentBoxRestored ? false : true}
enableReplyPlaceHolder={(comment._commentItems as CommentItem<TypesPullReqActivity>[])?.length > 0}
onCancel={comment.destroy}
setDirty={setDirty || noop}
@ -470,6 +501,8 @@ export function usePullReqComments({
/>
)
}}
memorizedState={_memorizedState}
commentsVisibilityAtLineNumber={memorizedState.get(diff.filePath)?.commentsVisibilityAtLineNumber}
/>
</AppWrapper>,
element
@ -498,7 +531,8 @@ export function usePullReqComments({
refetchActivities,
copyLinkToComment,
markSelectedLines,
updateDataCommentIds
updateDataCommentIds,
memorizedState
]
)

View File

@ -49,7 +49,7 @@ export function useCollapseHarnessNav() {
if (handled.current) {
const nav = document.getElementById('main-side-nav')
const pullReqNavItem = nav?.querySelector('[data-code-repo-section="pull-requests"]')
const toggleNavButton = nav?.querySelector('span[icon][class*="SideNavToggleButton"]') as HTMLElement
const toggleNavButton = nav?.querySelector('span[icon][class*="icon-symbol-triangle"]') as HTMLElement
if (pullReqNavItem && toggleNavButton) {
const isCollapsed = pullReqNavItem.clientWidth <= 64
@ -63,7 +63,7 @@ export function useCollapseHarnessNav() {
}
return () => {
if (handled.current) {
if (handled.current && toggleNavButton) {
toggleNavButton.click()
}
}