From ec45e70770eb7c35e872e8fc408a8c8545b1a4c7 Mon Sep 17 00:00:00 2001
From: Johannes Batzill <johannes.batzill@harness.io>
Date: Mon, 18 Sep 2023 20:56:14 +0000
Subject: [PATCH] [UI] Fix PR Refreshing, Code Comment Canceling (#519)

---
 web/src/components/Changes/Changes.tsx        |  33 +-
 .../CommitRangeDropdown.tsx                   |  15 +-
 .../components/CommitsView/CommitsView.tsx    |   6 +-
 web/src/components/DiffViewer/DiffViewer.tsx  | 456 +++++++++---------
 .../components/DiffViewer/DiffViewerUtils.tsx |  18 +-
 web/src/pages/Compare/Compare.tsx             |   3 +-
 .../PullRequest/Conversation/Conversation.tsx |   9 +-
 web/src/pages/PullRequest/PullRequest.tsx     |  88 ++--
 .../PullRequestCommits/PullRequestCommits.tsx |   6 +-
 .../RepositoryCommit/RepositoryCommit.tsx     |   3 +-
 10 files changed, 321 insertions(+), 316 deletions(-)

diff --git a/web/src/components/Changes/Changes.tsx b/web/src/components/Changes/Changes.tsx
index 22cbabf02..9ee1312d6 100644
--- a/web/src/components/Changes/Changes.tsx
+++ b/web/src/components/Changes/Changes.tsx
@@ -14,7 +14,7 @@ import * as Diff2Html from 'diff2html'
 import cx from 'classnames'
 import { useHistory } from 'react-router-dom'
 import { useGet } from 'restful-react'
-import { noop } from 'lodash-es'
+import { isEqual, noop } from 'lodash-es'
 import { useStrings } from 'framework/strings'
 import type { GitInfoProps } from 'utils/GitUtils'
 import { PullRequestSection, formatNumber, getErrorMessage, voidFn } from 'utils/Utils'
@@ -48,7 +48,7 @@ interface ChangesProps extends Pick<GitInfoProps, 'repoMetadata'> {
   pullRequestMetadata?: TypesPullReq
   className?: string
   onCommentUpdate: () => void
-  prHasChanged?: boolean
+  prStatsChanged: Number
   onDataReady?: (data: DiffFileEntry[]) => void
   defaultCommitRange?: string[]
   scrollElement: HTMLElement
@@ -62,9 +62,9 @@ export const Changes: React.FC<ChangesProps> = ({
   emptyTitle,
   emptyMessage,
   pullRequestMetadata,
-  onCommentUpdate,
   className,
-  prHasChanged,
+  onCommentUpdate,
+  prStatsChanged,
   onDataReady,
   defaultCommitRange,
   scrollElement
@@ -197,24 +197,31 @@ export const Changes: React.FC<ChangesProps> = ({
   // happens after some comments are authored.
   useEffect(
     function setActivitiesIfNotSet() {
-      if (prActivities) {
-        setActivities(prActivities)
+      if (!prActivities || isEqual(activities, prActivities)) {
+        return
       }
+
+      setActivities(prActivities)
+
     },
     [prActivities]
   )
 
   useEffect(() => {
-    if (prHasChanged) {
-      refetchActivities()
+    if (readOnly) {
+      return
     }
-  }, [prHasChanged, refetchActivities])
+
+    refetchActivities()
+  }, [prStatsChanged])
 
   useEffect(() => {
-    if (prHasChanged) {
-      refetchFileViews()
+    if (readOnly) {
+      return
     }
-  }, [prHasChanged, refetchFileViews])
+
+    refetchFileViews()
+  }, [prStatsChanged])
 
   useEffect(() => {
     const _raw = rawDiff && typeof rawDiff === 'string' ? rawDiff : ''
@@ -335,7 +342,7 @@ export const Changes: React.FC<ChangesProps> = ({
                   // is changed. Making it easier to control states inside DiffView itself, as it does not
                   //  have to deal with any view configuration
                   <DiffViewer
-                    readOnly={readOnly}
+                    readOnly={readOnly || (commitRange?.length || 0) > 0} // render in readonly mode in case a commit is selected
                     key={viewStyle + index + lineBreaks}
                     diff={diff}
                     viewStyle={viewStyle}
diff --git a/web/src/components/Changes/CommitRangeDropdown/CommitRangeDropdown.tsx b/web/src/components/Changes/CommitRangeDropdown/CommitRangeDropdown.tsx
index 2900c5496..39370bdaf 100644
--- a/web/src/components/Changes/CommitRangeDropdown/CommitRangeDropdown.tsx
+++ b/web/src/components/Changes/CommitRangeDropdown/CommitRangeDropdown.tsx
@@ -79,11 +79,8 @@ const CommitRangeDropdown: React.FC<CommitRangeDropdownProps> = ({
       // clicked commit is outside of current range - extend it!
       const extendedArray = getCommitRange([...current, selectedCommitSHA], allCommitsSHA)
 
-      //  Are all commits selected, then return AllCommits explicitly
-      if (extendedArray.length === allCommits.length) {
-        return []
-      }
-
+      // NOTE: this CAN contain all commits - we let it through for consistent user experience.
+      // This way, the user sees selected exactly what they clicked on (+ we don't have to handle single commit pr differently)
       return extendedArray
     })
   }
@@ -142,9 +139,11 @@ const CommitRangeDropdown: React.FC<CommitRangeDropdownProps> = ({
         color={Color.GREY_700}
         font={{ variation: FontVariation.BODY2 }}
         margin={{ right: 'medium' }}>
-        {selectedCommits.length && selectedCommits.length !== allCommitsSHA.length
-          ? `${selectedCommits.length} ${selectedCommits.length > 1 ? getString('commits') : getString('commit')}`
-          : getString('allCommits')}
+        {
+          areAllCommitsSelected
+            ? getString('allCommits')
+            : `${selectedCommits.length} ${selectedCommits.length > 1 ? getString('commits') : getString('commit')}`
+        }
       </Text>
     </Popover>
   )
diff --git a/web/src/components/CommitsView/CommitsView.tsx b/web/src/components/CommitsView/CommitsView.tsx
index 1945bd5e0..a017decfc 100644
--- a/web/src/components/CommitsView/CommitsView.tsx
+++ b/web/src/components/CommitsView/CommitsView.tsx
@@ -32,7 +32,7 @@ interface CommitsViewProps extends Pick<GitInfoProps, 'repoMetadata'> {
   commits: TypesCommit[] | null
   emptyTitle: string
   emptyMessage: string
-  prHasChanged?: boolean
+  prStatsChanged?: Number
   handleRefresh?: () => void
   showFileHistoryIcons?: boolean
   resourcePath?: string
@@ -46,7 +46,7 @@ export function CommitsView({
   emptyTitle,
   emptyMessage,
   handleRefresh = noop,
-  prHasChanged,
+  prStatsChanged,
   showFileHistoryIcons = false,
   resourcePath = '',
   setActiveTab,
@@ -186,7 +186,7 @@ export function CommitsView({
     <Container className={css.container}>
       <Layout.Horizontal>
         <FlexExpander />
-        {!prHasChanged ? null : (
+        {!prStatsChanged ? null : (
           <Button
             onClick={handleRefresh}
             iconProps={{ className: css.refreshIcon, size: 12 }}
diff --git a/web/src/components/DiffViewer/DiffViewer.tsx b/web/src/components/DiffViewer/DiffViewer.tsx
index c3ca91b2f..d8756e222 100644
--- a/web/src/components/DiffViewer/DiffViewer.tsx
+++ b/web/src/components/DiffViewer/DiffViewer.tsx
@@ -40,8 +40,9 @@ import {
   DiffCommentItem,
   DIFF_VIEWER_HEADER_HEIGHT,
   getCommentLineInfo,
-  renderCommentOppositePlaceHolder,
-  ViewStyle
+  createCommentOppositePlaceHolder,
+  ViewStyle,
+  contentDOMHasData
 } from './DiffViewerUtils'
 import { CommentAction, CommentBox, CommentBoxOutletPosition, CommentItem } from '../CommentBox/CommentBox'
 import css from './DiffViewer.module.scss'
@@ -116,7 +117,31 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
   const { mutate: saveComment } = useMutate({ verb: 'POST', path: commentPath })
   const { mutate: updateComment } = useMutate({ verb: 'PATCH', path: ({ id }) => `${commentPath}/${id}` })
   const { mutate: deleteComment } = useMutate({ verb: 'DELETE', path: ({ id }) => `${commentPath}/${id}` })
-  const [comments, setComments] = useState<DiffCommentItem<TypesPullReqActivity>[]>([])
+
+  const [comments, _setComments] = useState<DiffCommentItem<TypesPullReqActivity>[]>([])
+  function setComments(c: DiffCommentItem<TypesPullReqActivity>[]) {
+    // no changes in comments? nothing to do
+    // NOTE: we only react to new comments as of now, not changes on existing comments or replies, so that's good enough
+    if (c.length == comments.length) {
+      return
+    }
+
+    _setComments(c)
+    triggerCodeCommentRendering()
+  }
+  // use separate flag for monitoring comment rendering as opposed to updating comments to void spamming comment changes
+  const [renderComments, _setRenderComments] = useState(0)
+  function triggerCodeCommentRendering() {
+    _setRenderComments(Date.now())
+  }
+  useMemo(() => {
+    triggerCodeCommentRendering()
+  }, [
+    viewStyle,
+    inView,
+    commitRange
+  ])
+
   const [dirty, setDirty] = useState(false)
   const commentsRef = useRef<DiffCommentItem<TypesPullReqActivity>[]>(comments)
   const setContainerRef = useCallback(
@@ -145,7 +170,9 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
         if (!renderCustomContent || enforced) {
           containerDOM.style.height = 'auto'
           diffRenderer?.draw()
+          triggerCodeCommentRendering()
         }
+        
         contentDOM.dataset.rendered = 'true'
       }
     },
@@ -153,22 +180,16 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
   )
 
   useEffect(() => {
-    if (commitRange) {
+    // no activities or commit range view? no comments!
+    if (!diff?.fileActivities || (commitRange?.length || 0) > 0) {
       setComments([])
+      return  
     }
-  }, [commitRange])
-
-  useEffect(() => {
-    // For some unknown reason, comments is [] when we switch to Changes tab very quickly sometimes,
-    // but diff is not empty, and activitiesToDiffCommentItems(diff) is not []. So assigning
-    // comments = activitiesToDiffCommentItems(diff) from the useState() is not enough.
-    if (diff) {
-      const _comments = activitiesToDiffCommentItems(diff)
-      if (_comments.length > 0 && !comments.length) {
-        setComments(_comments)
-      }
+    const _comments = activitiesToDiffCommentItems(diff)
+    if (_comments.length > 0) {
+      setComments(_comments)
     }
-  }, [diff, comments])
+  }, [diff?.fileActivities, diff?.fileActivities?.length, commitRange])
 
   useEffect(
     function createDiffRenderer() {
@@ -215,7 +236,6 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
         containerClassList.remove(css.collapsed)
 
         const newHeight = Number(containerDOM.scrollHeight)
-
         if (parseInt(containerStyle.height) != newHeight) {
           containerStyle.height = `${newHeight}px`
         }
@@ -264,7 +284,7 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
           setComments([...comments, commentItem])
         }
       },
-      [viewStyle, comments, readOnly]
+      [viewStyle, readOnly]
     ),
     containerRef.current as HTMLDivElement
   )
@@ -275,229 +295,205 @@ export const DiffViewer: React.FC<DiffViewerProps> = ({
         return
       }
 
+      // early exit if there's nothing to render on
+      if (!contentRef.current || !contentDOMHasData(contentRef.current)) {
+        return
+      }
+
       const isSideBySide = viewStyle === ViewStyle.SIDE_BY_SIDE
 
       // Update latest commentsRef to use it inside CommentBox callbacks
       commentsRef.current = comments
 
-      comments.forEach(comment => {
+      comments.forEach(comment => {        
         const lineInfo = getCommentLineInfo(contentRef.current, comment, viewStyle)
-        if (lineInfo.rowElement) {
-          const { rowElement } = lineInfo
 
-          if (lineInfo.hasCommentsRendered) {
-            if (isSideBySide) {
-              const filesDiff = rowElement?.closest('.d2h-files-diff') as HTMLElement
-              const sideDiff = filesDiff?.querySelector(`div.${comment.left ? 'right' : 'left'}`) as HTMLElement
-              const oppositeRowPlaceHolder = sideDiff?.querySelector(
-                `tr[data-place-holder-for-line="${comment.lineNumber}"]`
-              )
-
-              const first = oppositeRowPlaceHolder?.firstElementChild as HTMLTableCellElement
-              const last = oppositeRowPlaceHolder?.lastElementChild as HTMLTableCellElement
-
-              if (first && last) {
-                first.style.height = `${comment.height}px`
-                last.style.height = `${comment.height}px`
-              }
-            }
-          } else {
-            // Mark row that it has comment/annotation
-            rowElement.dataset.annotated = 'true'
-
-            // Create a new row below it and render CommentBox inside
-            const commentRowElement = document.createElement('tr')
-            commentRowElement.dataset.annotatedLine = String(comment.lineNumber)
-            commentRowElement.innerHTML = `<td colspan="2"></td>`
-            rowElement.after(commentRowElement)
-
-            const element = commentRowElement.firstElementChild as HTMLTableCellElement
-            const resetCommentState = (ignoreCurrentComment = true) => {
-              // Clean up CommentBox rendering and reset states bound to lineInfo
-              ReactDOM.unmountComponentAtNode(element as HTMLDivElement)
-              commentRowElement.parentElement?.removeChild(commentRowElement)
-              lineInfo.oppositeRowElement?.parentElement?.removeChild(
-                lineInfo.oppositeRowElement?.nextElementSibling as Element
-              )
-              delete lineInfo.rowElement.dataset.annotated
-
-              setTimeout(
-                () =>
-                  setComments(
-                    commentsRef.current.filter(item => {
-                      if (ignoreCurrentComment) {
-                        return item !== comment
-                      }
-                      return true
-                    })
-                  ),
-                0
-              )
-            }
-
-            // 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 might
-            //       cause unexpected bugs
-            ReactDOM.unmountComponentAtNode(element as HTMLDivElement)
-            ReactDOM.render(
-              <AppWrapper>
-                <CommentBox
-                  commentItems={comment.commentItems}
-                  initialContent={''}
-                  width={isSideBySide ? 'calc(100vw / 2 - 163px)' : undefined} // TODO: Re-calcualte for standalone version
-                  onHeightChange={boxHeight => {
-                    if (comment.height !== boxHeight) {
-                      comment.height = boxHeight
-                      setTimeout(() => setComments([...commentsRef.current]), 0)
-                    }
-                  }}
-                  onCancel={resetCommentState}
-                  setDirty={setDirty}
-                  currentUserName={currentUser.display_name}
-                  handleAction={async (action, value, commentItem) => {
-                    let result = true
-                    let updatedItem: CommentItem<TypesPullReqActivity> | undefined = undefined
-                    const id = (commentItem as CommentItem<TypesPullReqActivity>)?.payload?.id
-
-                    switch (action) {
-                      case CommentAction.NEW: {
-                        const payload: OpenapiCommentCreatePullReqRequest = {
-                          line_start: comment.lineNumber,
-                          line_end: comment.lineNumber,
-                          line_start_new: !comment.left,
-                          line_end_new: !comment.left,
-                          path: diff.filePath,
-                          source_commit_sha: sourceRef,
-                          target_commit_sha: targetRef,
-                          text: value
-                        }
-
-                        await saveComment(payload)
-                          .then((newComment: TypesPullReqActivity) => {
-                            updatedItem = activityToCommentItem(newComment)
-                            diff.fileActivities?.push(newComment)
-                            comment.commentItems.push(updatedItem)
-                            resetCommentState(false)
-                          })
-                          .catch(exception => {
-                            result = false
-                            showError(getErrorMessage(exception), 0)
-                          })
-                        break
-                      }
-
-                      case CommentAction.REPLY: {
-                        await saveComment({
-                          type: CommentType.CODE_COMMENT,
-                          text: value,
-                          parent_id: Number(commentItem?.payload?.id as number)
-                        })
-                          .then(newComment => {
-                            updatedItem = activityToCommentItem(newComment)
-                            diff.fileActivities?.push(newComment)
-                          })
-                          .catch(exception => {
-                            result = false
-                            showError(getErrorMessage(exception), 0)
-                          })
-                        break
-                      }
-
-                      case CommentAction.DELETE: {
-                        result = false
-                        await confirmAct({
-                          message: getString('deleteCommentConfirm'),
-                          action: async () => {
-                            await deleteComment({}, { pathParams: { id } })
-                              .then(() => {
-                                result = true
-                              })
-                              .catch(exception => {
-                                result = false
-                                showError(getErrorMessage(exception), 0, getString('pr.failedToDeleteComment'))
-                              })
-                          }
-                        })
-                        break
-                      }
-
-                      case CommentAction.UPDATE: {
-                        await updateComment({ text: value }, { pathParams: { id } })
-                          .then(newComment => {
-                            updatedItem = activityToCommentItem(newComment)
-                          })
-                          .catch(exception => {
-                            result = false
-                            showError(getErrorMessage(exception), 0)
-                          })
-                        break
-                      }
-                    }
-
-                    if (result) {
-                      onCommentUpdate()
-                    }
-
-                    return [result, updatedItem]
-                  }}
-                  outlets={{
-                    [CommentBoxOutletPosition.LEFT_OF_OPTIONS_MENU]: (
-                      <CodeCommentStatusSelect
-                        repoMetadata={repoMetadata}
-                        pullRequestMetadata={pullRequestMetadata as TypesPullReq}
-                        onCommentUpdate={onCommentUpdate}
-                        commentItems={comment.commentItems}
-                      />
-                    ),
-                    [CommentBoxOutletPosition.LEFT_OF_REPLY_PLACEHOLDER]: (
-                      <CodeCommentStatusButton
-                        repoMetadata={repoMetadata}
-                        pullRequestMetadata={pullRequestMetadata as TypesPullReq}
-                        onCommentUpdate={onCommentUpdate}
-                        commentItems={comment.commentItems}
-                      />
-                    ),
-                    [CommentBoxOutletPosition.BETWEEN_SAVE_AND_CANCEL_BUTTONS]: (props: ButtonProps) => (
-                      <CodeCommentSecondarySaveButton
-                        repoMetadata={repoMetadata}
-                        pullRequestMetadata={pullRequestMetadata as TypesPullReq}
-                        commentItems={comment.commentItems}
-                        {...props}
-                      />
-                    )
-                  }}
-                  autoFocusAndPositioning
-                />
-              </AppWrapper>,
-              element
-            )
-
-            // Split view: Calculate, inject, and adjust an empty place-holder row in the opposite pane
-            if (isSideBySide && lineInfo.oppositeRowElement) {
-              renderCommentOppositePlaceHolder(comment, lineInfo.oppositeRowElement)
-            }
-          }
+        // TODO: add support for live updating changes and replies to comment!
+        if (!lineInfo.rowElement || lineInfo.hasCommentsRendered) {
+          return
         }
+        const { rowElement } = lineInfo
+
+        // Mark row that it has comment/annotation
+        rowElement.dataset.annotated = 'true'
+
+        // always create placeholder (in memory)
+        const oppositeRowPlaceHolder = createCommentOppositePlaceHolder(comment)
+
+        // in split view, actually attach the placeholder
+        if (isSideBySide && lineInfo.oppositeRowElement != null) {
+            lineInfo.oppositeRowElement.after(oppositeRowPlaceHolder)
+        }
+
+        // Create a new row below it and render CommentBox inside
+        const commentRowElement = document.createElement('tr')
+        commentRowElement.dataset.annotatedLine = String(comment.lineNumber)
+        commentRowElement.innerHTML = `<td colspan="2"></td>`
+        rowElement.after(commentRowElement)
+
+        const element = commentRowElement.firstElementChild as HTMLTableCellElement
+        const resetCommentState = () => {
+          // Clean up CommentBox rendering and reset states bound to lineInfo
+          ReactDOM.unmountComponentAtNode(element as HTMLDivElement)
+          commentRowElement.parentElement?.removeChild(commentRowElement)
+          lineInfo.oppositeRowElement?.parentElement?.removeChild(
+            oppositeRowPlaceHolder as Element
+          )
+          delete lineInfo.rowElement.dataset.annotated
+
+          setComments(
+              commentsRef.current.filter(item => {
+                return item !== comment
+              })
+            )
+        }
+
+        // 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 might
+        //       cause unexpected bugs
+        ReactDOM.unmountComponentAtNode(element as HTMLDivElement)
+        ReactDOM.render(
+          <AppWrapper>
+            <CommentBox
+              commentItems={comment.commentItems}
+              initialContent={''}
+              width={isSideBySide ? 'calc(100vw / 2 - 163px)' : undefined} // TODO: Re-calcualte for standalone version
+              onHeightChange={boxHeight => {
+                  const first = oppositeRowPlaceHolder?.firstElementChild as HTMLTableCellElement
+                  const last = oppositeRowPlaceHolder?.lastElementChild as HTMLTableCellElement
+                  if (first && last) {
+                    first.style.height = `${boxHeight}px`
+                    last.style.height = `${boxHeight}px`
+                  }
+              }}
+              onCancel={resetCommentState}
+              setDirty={setDirty}
+              currentUserName={currentUser.display_name}
+              handleAction={async (action, value, commentItem) => {
+                let result = true
+                let updatedItem: CommentItem<TypesPullReqActivity> | undefined = undefined
+                const id = (commentItem as CommentItem<TypesPullReqActivity>)?.payload?.id
+
+                switch (action) {
+                  case CommentAction.NEW: {
+                    const payload: OpenapiCommentCreatePullReqRequest = {
+                      line_start: comment.lineNumber,
+                      line_end: comment.lineNumber,
+                      line_start_new: !comment.left,
+                      line_end_new: !comment.left,
+                      path: diff.filePath,
+                      source_commit_sha: sourceRef,
+                      target_commit_sha: targetRef,
+                      text: value
+                    }
+
+                    await saveComment(payload)
+                      .then((newComment: TypesPullReqActivity) => {
+                        updatedItem = activityToCommentItem(newComment)
+
+                        // remove item (to refresh all comment refrences and remove it from rendering)
+                        resetCommentState()
+
+                        // add comment to file activities (will re-create comments and render new one)
+                        diff.fileActivities?.push(newComment)
+                      })
+                      .catch(exception => {
+                        result = false
+                        showError(getErrorMessage(exception), 0)
+                      })
+                    break
+                  }
+
+                  case CommentAction.REPLY: {
+                    await saveComment({
+                      type: CommentType.CODE_COMMENT,
+                      text: value,
+                      parent_id: Number(commentItem?.payload?.id as number)
+                    })
+                      .then(newComment => {
+                        updatedItem = activityToCommentItem(newComment)
+                        diff.fileActivities?.push(newComment)
+                      })
+                      .catch(exception => {
+                        result = false
+                        showError(getErrorMessage(exception), 0)
+                      })
+                    break
+                  }
+
+                  case CommentAction.DELETE: {
+                    result = false
+                    await confirmAct({
+                      message: getString('deleteCommentConfirm'),
+                      action: async () => {
+                        await deleteComment({}, { pathParams: { id } })
+                          .then(() => {
+                            result = true
+                          })
+                          .catch(exception => {
+                            result = false
+                            showError(getErrorMessage(exception), 0, getString('pr.failedToDeleteComment'))
+                          })
+                      }
+                    })
+                    break
+                  }
+
+                  case CommentAction.UPDATE: {
+                    await updateComment({ text: value }, { pathParams: { id } })
+                      .then(newComment => {
+                        updatedItem = activityToCommentItem(newComment)
+                      })
+                      .catch(exception => {
+                        result = false
+                        showError(getErrorMessage(exception), 0)
+                      })
+                    break
+                  }
+                }
+
+                if (result) {
+                  onCommentUpdate()
+                }
+
+                return [result, updatedItem]
+              }}
+              outlets={{
+                [CommentBoxOutletPosition.LEFT_OF_OPTIONS_MENU]: (
+                  <CodeCommentStatusSelect
+                    repoMetadata={repoMetadata}
+                    pullRequestMetadata={pullRequestMetadata as TypesPullReq}
+                    onCommentUpdate={onCommentUpdate}
+                    commentItems={comment.commentItems}
+                  />
+                ),
+                [CommentBoxOutletPosition.LEFT_OF_REPLY_PLACEHOLDER]: (
+                  <CodeCommentStatusButton
+                    repoMetadata={repoMetadata}
+                    pullRequestMetadata={pullRequestMetadata as TypesPullReq}
+                    onCommentUpdate={onCommentUpdate}
+                    commentItems={comment.commentItems}
+                  />
+                ),
+                [CommentBoxOutletPosition.BETWEEN_SAVE_AND_CANCEL_BUTTONS]: (props: ButtonProps) => (
+                  <CodeCommentSecondarySaveButton
+                    repoMetadata={repoMetadata}
+                    pullRequestMetadata={pullRequestMetadata as TypesPullReq}
+                    commentItems={comment.commentItems}
+                    {...props}
+                  />
+                )
+              }}
+              autoFocusAndPositioning
+            />
+          </AppWrapper>,
+          element
+        )
       })
     },
     [
-      comments,
-      viewStyle,
-      getString,
-      currentUser,
-      readOnly,
-      diff,
-      saveComment,
-      showError,
-      updateComment,
-      deleteComment,
-      confirmAct,
-      onCommentUpdate,
-      targetRef,
-      sourceRef,
-      pullRequestMetadata,
-      repoMetadata
+      renderComments,
     ]
   )
 
diff --git a/web/src/components/DiffViewer/DiffViewerUtils.tsx b/web/src/components/DiffViewer/DiffViewerUtils.tsx
index f8f798e88..13e233c2e 100644
--- a/web/src/components/DiffViewer/DiffViewerUtils.tsx
+++ b/web/src/components/DiffViewer/DiffViewerUtils.tsx
@@ -136,6 +136,10 @@ export const DIFF2HTML_CONFIG = {
   }
 } as Readonly<Diff2Html.Diff2HtmlConfig>
 
+export function contentDOMHasData(contentDOM: HTMLDivElement): boolean {
+  return contentDOM?.querySelector('[data]') != null
+}
+
 export function getCommentLineInfo(
   contentDOM: HTMLDivElement | null,
   commentEntry: DiffCommentItem,
@@ -143,7 +147,7 @@ export function getCommentLineInfo(
 ) {
   const isSideBySideView = viewStyle === ViewStyle.SIDE_BY_SIDE
   const { left, lineNumber, filePath } = commentEntry
-  const filePathBody = filePath ? contentDOM?.querySelector(`[data="${filePath}"`) : contentDOM
+  const filePathBody = (filePath ? contentDOM?.querySelector(`[data="${filePath}"`) : contentDOM)
 
   const diffBody = filePathBody?.querySelector(
     `${isSideBySideView ? `.d2h-file-side-diff${left ? '.left' : '.right'} ` : ''}.d2h-diff-tbody`
@@ -178,7 +182,7 @@ export function getCommentLineInfo(
   }
 }
 
-export function renderCommentOppositePlaceHolder(annotation: DiffCommentItem, oppositeRowElement: HTMLTableRowElement) {
+export function createCommentOppositePlaceHolder(annotation: DiffCommentItem): HTMLTableRowElement {
   const placeHolderRow = document.createElement('tr')
 
   placeHolderRow.dataset.placeHolderForLine = String(annotation.lineNumber)
@@ -191,7 +195,8 @@ export function renderCommentOppositePlaceHolder(annotation: DiffCommentItem, op
       </div>
     </td>
   `
-  oppositeRowElement.after(placeHolderRow)
+
+  return placeHolderRow
 }
 
 export const activityToCommentItem = (activity: TypesPullReqActivity): CommentItem<TypesPullReqActivity> => ({
@@ -205,7 +210,7 @@ export const activityToCommentItem = (activity: TypesPullReqActivity): CommentIt
 })
 
 export function activitiesToDiffCommentItems(diff: DiffFileEntry): DiffCommentItem<TypesPullReqActivity>[] {
-  return (
+  const commentThreads = (
     diff.fileActivities?.map(activity => {
       const replyComments =
         diff.activities
@@ -219,8 +224,11 @@ export function activitiesToDiffCommentItems(diff: DiffFileEntry): DiffCommentIt
         height: 0,
         lineNumber: (right ? activity.code_comment?.line_new : activity.code_comment?.line_old) as number,
         commentItems: [activityToCommentItem(activity)].concat(replyComments),
-        filePath: diff.filePath
+        filePath: diff.filePath,
       }
     }) || []
   )
+
+  // filter out threads where all comments are deleted
+  return commentThreads.filter(({commentItems: commentItems}) => commentItems.map(item => !item.deleted).reduce((a,b) => a || b), false)
 }
diff --git a/web/src/pages/Compare/Compare.tsx b/web/src/pages/Compare/Compare.tsx
index 5f9a58015..2d6edef86 100644
--- a/web/src/pages/Compare/Compare.tsx
+++ b/web/src/pages/Compare/Compare.tsx
@@ -277,13 +277,14 @@ export default function Compare() {
                   panel: (
                     <TabContentWrapper loading={loading} error={error} onRetry={noop} className={css.changesContainer}>
                       <Changes
-                        readOnly
+                        readOnly={true}
                         repoMetadata={repoMetadata}
                         targetRef={targetGitRef}
                         sourceRef={sourceGitRef}
                         emptyTitle={getString('noChanges')}
                         emptyMessage={getString('noChangesCompare')}
                         onCommentUpdate={noop}
+                        prStatsChanged={0}
                         scrollElement={(standalone ? document.querySelector(`.${css.main}`)?.parentElement || window : window) as HTMLElement}
                       />
                     </TabContentWrapper>
diff --git a/web/src/pages/PullRequest/Conversation/Conversation.tsx b/web/src/pages/PullRequest/Conversation/Conversation.tsx
index 28ea82341..9cbb059e5 100644
--- a/web/src/pages/PullRequest/Conversation/Conversation.tsx
+++ b/web/src/pages/PullRequest/Conversation/Conversation.tsx
@@ -29,7 +29,7 @@ import css from './Conversation.module.scss'
 
 export interface ConversationProps extends Pick<GitInfoProps, 'repoMetadata' | 'pullRequestMetadata'> {
   onCommentUpdate: () => void
-  prHasChanged?: boolean
+  prStatsChanged: Number
   showEditDescription?: boolean
   onCancelEditDescription: () => void
   prChecksDecisionResult?: PRChecksDecisionResult
@@ -39,7 +39,7 @@ export const Conversation: React.FC<ConversationProps> = ({
   repoMetadata,
   pullRequestMetadata,
   onCommentUpdate,
-  prHasChanged,
+  prStatsChanged,
   showEditDescription,
   onCancelEditDescription,
   prChecksDecisionResult
@@ -143,10 +143,10 @@ export const Conversation: React.FC<ConversationProps> = ({
   )
 
   useEffect(() => {
-    if (prHasChanged) {
+    if (prStatsChanged) {
       refetchActivities()
     }
-  }, [prHasChanged, refetchActivities, refetchReviewers])
+  }, [prStatsChanged, refetchActivities])
 
   return (
     <PullRequestTabContentWrapper loading={showSpinner} error={error} onRetry={refetchActivities}>
@@ -176,6 +176,7 @@ export const Conversation: React.FC<ConversationProps> = ({
                       pullRequestMetadata={pullRequestMetadata}
                       onCommentUpdate={onCommentUpdate}
                       onCancelEditDescription={onCancelEditDescription}
+                      prStatsChanged={prStatsChanged}
                     />
                   )}
 
diff --git a/web/src/pages/PullRequest/PullRequest.tsx b/web/src/pages/PullRequest/PullRequest.tsx
index c6018417e..18defecf0 100644
--- a/web/src/pages/PullRequest/PullRequest.tsx
+++ b/web/src/pages/PullRequest/PullRequest.tsx
@@ -4,7 +4,7 @@ import { FontVariation } from '@harnessio/design-system'
 import { useGet, useMutate } from 'restful-react'
 import { Render } from 'react-jsx-match'
 import { useHistory } from 'react-router-dom'
-import { compact } from 'lodash-es'
+import { compact, isEqual } from 'lodash-es'
 import { useAppContext } from 'AppContext'
 import { useGetRepositoryMetadata } from 'hooks/useGetRepositoryMetadata'
 import { useStrings } from 'framework/strings'
@@ -58,21 +58,20 @@ export default function PullRequest() {
   const showSpinner = useMemo(() => {
     return loading || (prLoading && !prData)
   }, [loading, prLoading, prData])
-  const [stats, setStats] = useState<TypesPullReqStats>()
   const [showEditDescription, setShowEditDescription] = useState(false)
-  const prHasChanged = useMemo(() => {
-    if (stats && prData?.stats) {
-      if (
-        stats.commits !== prData.stats.commits ||
-        stats.conversations !== prData.stats.conversations ||
-        stats.files_changed !== prData.stats.files_changed
-      ) {
-        window.setTimeout(() => setStats(prData.stats), 50)
-        return true
-      }
+  
+  const [stats, setStats] = useState<TypesPullReqStats>()
+  // simple value one can listen on to react on stats changes (boolean is NOT enough)
+  const [prStatsChanged, setPrStatsChanged] = useState(0)
+  useMemo(() => {
+    if (isEqual(stats, prData?.stats)) {
+      return
     }
-    return false
+
+    setStats(stats)
+    setPrStatsChanged(Date.now())
   }, [prData?.stats, stats])
+
   const onAddDescriptionClick = useCallback(() => {
     setShowEditDescription(true)
     history.replace(
@@ -92,43 +91,36 @@ export default function PullRequest() {
     path: recheckPath,
   })
 
-  useEffect(
-    function setStatsIfNotSet() {
-      if (!stats && prData?.stats) {
-        setStats(prData.stats)
-      }
-    },
-    [prData?.stats, stats]
-  )
-
   // prData holds the latest good PR data to make sure page is not broken
   // when polling fails
   useEffect(
     function setPrDataIfNotSet() {
-      if (pullRequestData) {
-        // recheck pr (merge-check, ...) in case it's unavailable
-        // Approximation of identifying target branch update:
-        //   1. branch got updated before page was loaded (status is unchecked and prData is empty)
-        //      NOTE: This doesn't guarantee the status is UNCHECKED due to target branch update and can cause duplicate
-        //      PR merge checks being run on PR creation or source branch update.
-        //   2. branch got updated while we are on the page (same source_sha but status changed to UNCHECKED)
-        //      NOTE: This doesn't cover the case in which the status changed back to UNCHECKED before the PR is refetched.
-        //      In that case, the user will have to re-open the PR - better than us spamming the backend with rechecks.
-        // This is a TEMPORARY SOLUTION and will most likely change in the future (more so on backend side)
-        if (pullRequestData.state == 'open' &&
-            pullRequestData.merge_check_status == MergeCheckStatus.UNCHECKED &&
-            (
-              // case 1:
-              !prData || 
-              // case 2:
-              (prData?.merge_check_status != MergeCheckStatus.UNCHECKED && prData?.source_sha == pullRequestData.source_sha)
-            ) && !loadingRecheckPR) {
-              // best effort attempt to recheck PR - fail silently
-              recheckPR({})
-        }
-
-        setPrData(pullRequestData)
+      if (!pullRequestData || (prData && isEqual(prData, pullRequestData))) {
+        return
       }
+
+      // recheck pr (merge-check, ...) in case it's unavailable
+      // Approximation of identifying target branch update:
+      //   1. branch got updated before page was loaded (status is unchecked and prData is empty)
+      //      NOTE: This doesn't guarantee the status is UNCHECKED due to target branch update and can cause duplicate
+      //      PR merge checks being run on PR creation or source branch update.
+      //   2. branch got updated while we are on the page (same source_sha but status changed to UNCHECKED)
+      //      NOTE: This doesn't cover the case in which the status changed back to UNCHECKED before the PR is refetched.
+      //      In that case, the user will have to re-open the PR - better than us spamming the backend with rechecks.
+      // This is a TEMPORARY SOLUTION and will most likely change in the future (more so on backend side)
+      if (pullRequestData.state == 'open' &&
+          pullRequestData.merge_check_status == MergeCheckStatus.UNCHECKED &&
+          (
+            // case 1:
+            !prData || 
+            // case 2:
+            (prData?.merge_check_status != MergeCheckStatus.UNCHECKED && prData?.source_sha == pullRequestData.source_sha)
+          ) && !loadingRecheckPR) {
+            // best effort attempt to recheck PR - fail silently
+            recheckPR({})
+      }
+
+      setPrData(pullRequestData)
     },
     [pullRequestData]
   )
@@ -217,7 +209,7 @@ export default function PullRequest() {
                           setShowEditDescription(false)
                           refetchPullRequest()
                         }}
-                        prHasChanged={prHasChanged}
+                        prStatsChanged={prStatsChanged}
                         showEditDescription={showEditDescription}
                         onCancelEditDescription={() => setShowEditDescription(false)}
                       />
@@ -237,7 +229,7 @@ export default function PullRequest() {
                       <PullRequestCommits
                         repoMetadata={repoMetadata as TypesRepository}
                         pullRequestMetadata={prData as TypesPullReq}
-                        prHasChanged={prHasChanged}
+                        prStatsChanged={prStatsChanged}
                         handleRefresh={voidFn(refetchPullRequest)}
                       />
                     )
@@ -263,7 +255,7 @@ export default function PullRequest() {
                           emptyTitle={getString('noChanges')}
                           emptyMessage={getString('noChangesPR')}
                           onCommentUpdate={voidFn(refetchPullRequest)}
-                          prHasChanged={prHasChanged}
+                          prStatsChanged={prStatsChanged}
                           scrollElement={(standalone ? document.querySelector(`.${css.main}`)?.parentElement || window : window) as HTMLElement}
                         />
                       </Container>
diff --git a/web/src/pages/PullRequest/PullRequestCommits/PullRequestCommits.tsx b/web/src/pages/PullRequest/PullRequestCommits/PullRequestCommits.tsx
index e9b1fbf1e..f4b5c9a94 100644
--- a/web/src/pages/PullRequest/PullRequestCommits/PullRequestCommits.tsx
+++ b/web/src/pages/PullRequest/PullRequestCommits/PullRequestCommits.tsx
@@ -10,14 +10,14 @@ import { CommitsView } from 'components/CommitsView/CommitsView'
 import { PullRequestTabContentWrapper } from '../PullRequestTabContentWrapper'
 
 interface CommitProps extends Pick<GitInfoProps, 'repoMetadata' | 'pullRequestMetadata'> {
-  prHasChanged: boolean
+  prStatsChanged: Number
   handleRefresh: () => void
 }
 
 export const PullRequestCommits: React.FC<CommitProps> = ({
   repoMetadata,
   pullRequestMetadata,
-  prHasChanged,
+  prStatsChanged,
   handleRefresh
 }) => {
   const limit = LIST_FETCHING_LIMIT
@@ -43,7 +43,7 @@ export const PullRequestCommits: React.FC<CommitProps> = ({
         repoMetadata={repoMetadata}
         emptyTitle={getString('noCommits')}
         emptyMessage={getString('noCommitsPR')}
-        prHasChanged={prHasChanged}
+        prStatsChanged={prStatsChanged}
         handleRefresh={voidFn(handleRefresh)}
         pullRequestMetadata={pullRequestMetadata}
       />
diff --git a/web/src/pages/RepositoryCommit/RepositoryCommit.tsx b/web/src/pages/RepositoryCommit/RepositoryCommit.tsx
index 05b961733..1a031fb9c 100644
--- a/web/src/pages/RepositoryCommit/RepositoryCommit.tsx
+++ b/web/src/pages/RepositoryCommit/RepositoryCommit.tsx
@@ -36,13 +36,14 @@ export default function RepositoryCommits() {
       return (
         <Container className={css.changesContainer}>
           <Changes
-            readOnly
+            readOnly={true}
             repoMetadata={repoMetadata}
             targetRef={`${commitRef}~1`}
             sourceRef={commitRef}
             emptyTitle={getString('noChanges')}
             emptyMessage={getString('noChangesCompare')}
             onCommentUpdate={noop}
+            prStatsChanged={0}
             scrollElement={(standalone ? document.querySelector(`.${css.main}`)?.parentElement || window : window) as HTMLElement}
           />
         </Container>