Sort PR activities + add animation when creating a new comment (#243)

Co-authored-by: Tan Nhu <tnhu@users.noreply.github.com>
2025-05-10 22:21:22 +08:00 · 2023-01-23 23:48:37 -08:00 · 2023-01-23 23:48:37 -08:00 · b700603fef
commit b700603fef
parent 09902b02a3
11 changed files with 214 additions and 51 deletions
--- a/web/src/components/CommentBox/CommentBox.tsx
+++ b/web/src/components/CommentBox/CommentBox.tsx
@ -42,6 +42,7 @@ export enum CommentBoxOutletPosition {
 }

 interface CommentBoxProps<T> {
+  className?: string
  getString: UseStringsReturn['getString']
  onHeightChange?: (height: number) => void
  initialContent?: string
@ -61,6 +62,7 @@ interface CommentBoxProps<T> {
 }

 export const CommentBox = <T = unknown,>({
+  className,
  getString,
  onHeightChange = noop,
  initialContent = '',
@ -107,7 +109,7 @@ export const CommentBox = <T = unknown,>({

  return (
    <Container
-      className={cx(css.main, fluid ? css.fluid : '')}
+      className={cx(css.main, { [css.fluid]: fluid }, className)}
      padding={!fluid ? 'medium' : undefined}
      width={width}
      ref={ref}>
@ -131,7 +133,6 @@ export const CommentBox = <T = unknown,>({
            }}
            outlets={outlets}
          />
-
          <Match expr={showReplyPlaceHolder}>
            <Truthy>
              <Container>
--- a/web/src/components/DiffViewer/DiffViewer.module.scss
+++ b/web/src/components/DiffViewer/DiffViewer.module.scss
@ -25,6 +25,21 @@
            // border-top: 1px solid var(--grey-200);
            // border-bottom: 1px solid var(--grey-200);
          }
+
+          &.selected {
+            // TODO: Talk to design about these selection colors
+            &.first {
+              border-top: 1px solid var(--border-color);
+            }
+
+            &.last {
+              border-bottom: 1px solid var(--border-color);
+            }
+
+            td {
+              background-color: #e7ffab91 !important;
+            }
+          }
        }
      }

--- a/web/src/framework/strings/stringTypes.ts
+++ b/web/src/framework/strings/stringTypes.ts
@ -171,6 +171,8 @@ export interface StringsMap {
  'pr.split': string
  'pr.state': string
  'pr.statusLine': string
+  'pr.titleChanged': string
+  'pr.titleChangedTable': string
  'pr.titlePlaceHolder': string
  'pr.unified': string
  prefixBase: string
--- a/web/src/i18n/strings.en.yaml
+++ b/web/src/i18n/strings.en.yaml
@ -187,8 +187,13 @@ pr:
  failedToSaveComment: Failed to save comment. Please try again.
  failedToDeleteComment: Failed to delete comment. Please try again.
  prMerged: This Pull Request was merged
-  prMergedInfo: '{user} merged branch {source} into {target} {time}.'
  reviewSubmitted: Review submitted.
+  prMergedInfo: '{user} merged branch {source} into {target} {time}.'
+  titleChanged: '{user} changed title from {old} to {new}.'
+  titleChangedTable: |
+    ### Other title changes in history
+    | Author      | Old Name | New Name | Date |
+    | ----------- | -------- | -------- | ---- |
 webhookListingContent: 'create,delete,deployment ...'
 general: 'General'
 webhooks: 'Webhooks'
--- a/web/src/pages/PullRequest/Conversation/Conversation.module.scss
+++ b/web/src/pages/PullRequest/Conversation/Conversation.module.scss
@ -66,3 +66,13 @@
    display: none !important;
  }
 }
+
+.newCommentCreated {
+  box-shadow: 0px 0px 5px rgb(37 41 192);
+  border-radius: 4px;
+  transition: box-shadow 1s ease-in-out;
+
+  &.clear {
+    box-shadow: none;
+  }
+}
--- a/web/src/pages/PullRequest/Conversation/Conversation.module.scss.d.ts
+++ b/web/src/pages/PullRequest/Conversation/Conversation.module.scss.d.ts
@ -6,5 +6,7 @@ declare const styles: {
  readonly title: string
  readonly fname: string
  readonly snapshotContent: string
+  readonly newCommentCreated: string
+  readonly clear: string
 }
 export default styles
--- a/web/src/pages/PullRequest/Conversation/Conversation.tsx
+++ b/web/src/pages/PullRequest/Conversation/Conversation.tsx
@ -1,4 +1,4 @@
-import React, { useMemo, useState } from 'react'
+import React, { useEffect, useMemo, useState } from 'react'
 import {
  Avatar,
  Color,
@ -11,8 +11,11 @@ import {
  Text,
  useToaster
 } from '@harness/uicore'
+import cx from 'classnames'
 import { useGet, useMutate } from 'restful-react'
 import ReactTimeago from 'react-timeago'
+import { orderBy } from 'lodash-es'
+import { Render } from 'react-jsx-match'
 import { CodeIcon, GitInfoProps } from 'utils/GitUtils'
 import { MarkdownViewer } from 'components/SourceCodeViewer/SourceCodeViewer'
 import { useStrings } from 'framework/strings'
@ -23,7 +26,7 @@ import { PipeSeparator } from 'components/PipeSeparator/PipeSeparator'
 import { OptionsMenuButton } from 'components/OptionsMenuButton/OptionsMenuButton'
 import { MarkdownEditorWithPreview } from 'components/MarkdownEditorWithPreview/MarkdownEditorWithPreview'
 import { useConfirmAct } from 'hooks/useConfirmAction'
-import { getErrorMessage } from 'utils/Utils'
+import { formatDate, formatTime, getErrorMessage } from 'utils/Utils'
 import {
  activityToCommentItem,
  CommentType,
@ -54,25 +57,47 @@ export const Conversation: React.FC<ConversationProps> = ({
  })
  const { showError } = useToaster()
  const [newComments, setNewComments] = useState<TypesPullReqActivity[]>([])
-  const commentThreads = useMemo(() => {
-    const threads: Record<number, CommentItem<TypesPullReqActivity>[]> = {}
+  const activityBlocks = useMemo(() => {
+    // Each block may have one or more activities which are grouped into it. For example, one comment block
+    // contains a parent comment and multiple replied comments
+    const blocks: CommentItem<TypesPullReqActivity>[][] = []

-    activities?.forEach(activity => {
-      const thread: CommentItem<TypesPullReqActivity> = activityToCommentItem(activity)
+    if (newComments.length) {
+      blocks.push(orderBy(newComments, 'edited', 'desc').map(activityToCommentItem))
+    }

-      if (activity.parent_id) {
-        threads[activity.parent_id].push(thread)
-      } else {
-        threads[activity.id as number] = threads[activity.id as number] || []
-        threads[activity.id as number].push(thread)
+    // Determine all parent activities
+    const parentActivities = orderBy(activities?.filter(activity => !activity.parent_id) || [], 'edited', 'desc').map(
+      _comment => [_comment]
+    )
+
+    // Then add their children as follow-up elements (same array)
+    parentActivities?.forEach(parentActivity => {
+      const childActivities = activities?.filter(activity => activity.parent_id === parentActivity[0].id)
+
+      childActivities?.forEach(childComment => {
+        parentActivity.push(childComment)
+      })
+    })
+
+    parentActivities?.forEach(parentActivity => {
+      blocks.push(parentActivity.map(activityToCommentItem))
+    })
+
+    // Group title-change events into one single block
+    const titleChangeItems =
+      blocks.filter(
+        _activities => isSystemComment(_activities) && _activities[0].payload?.type === CommentType.TITLE_CHANGE
+      ) || []
+
+    titleChangeItems.forEach((value, index) => {
+      if (index > 0) {
+        titleChangeItems[0].push(...value)
      }
    })
+    titleChangeItems.shift()

-    newComments.forEach(newComment => {
-      threads[newComment.id as number] = [activityToCommentItem(newComment)]
-    })
-
-    return threads
+    return blocks.filter(_activities => !titleChangeItems.includes(_activities))
  }, [activities, newComments])
  const path = useMemo(
    () => `/api/v1/repos/${repoMetadata.path}/+/pullreq/${pullRequestMetadata.number}/comments`,
@ -82,6 +107,9 @@ export const Conversation: React.FC<ConversationProps> = ({
  const { mutate: updateComment } = useMutate({ verb: 'PATCH', path: ({ id }) => `${path}/${id}` })
  const { mutate: deleteComment } = useMutate({ verb: 'DELETE', path: ({ id }) => `${path}/${id}` })
  const confirmAct = useConfirmAct()
+  const [commentCreated, setCommentCreated] = useState(false)
+
+  useAnimateNewCommentBox(commentCreated, setCommentCreated)

  return (
    <PullRequestTabContentWrapper loading={loading} error={error} onRetry={refetchActivities}>
@ -103,7 +131,10 @@ export const Conversation: React.FC<ConversationProps> = ({
                refreshPullRequestMetadata={refreshPullRequestMetadata}
              />

-              {Object.entries(commentThreads).map(([threadId, commentItems]) => {
+              {activityBlocks?.map((blocks, index) => {
+                const threadId = blocks[0].payload?.id
+                const commentItems = blocks
+
                if (isSystemComment(commentItems)) {
                  return (
                    <SystemBox key={threadId} pullRequestMetadata={pullRequestMetadata} commentItems={commentItems} />
@ -114,6 +145,7 @@ export const Conversation: React.FC<ConversationProps> = ({
                  <CommentBox
                    key={threadId}
                    fluid
+                    className={cx({ [css.newCommentCreated]: commentCreated && !index })}
                    getString={getString}
                    commentItems={commentItems}
                    currentUserName={currentUser.display_name}
@ -166,7 +198,9 @@ export const Conversation: React.FC<ConversationProps> = ({
                      return [result, updatedItem]
                    }}
                    outlets={{
-                      [CommentBoxOutletPosition.TOP_OF_FIRST_COMMENT]: <CodeCommentHeader commentItems={commentItems} />
+                      [CommentBoxOutletPosition.TOP_OF_FIRST_COMMENT]: isCodeComment(commentItems) && (
+                        <CodeCommentHeader commentItems={commentItems} />
+                      )
                    }}
                  />
                )
@ -187,10 +221,11 @@ export const Conversation: React.FC<ConversationProps> = ({
                    .then((newComment: TypesPullReqActivity) => {
                      updatedItem = activityToCommentItem(newComment)
                      setNewComments([...newComments, newComment])
+                      setCommentCreated(true)
                    })
                    .catch(exception => {
                      result = false
-                      showError(getErrorMessage(exception), 0, getString('pr.failedToSaveComment'))
+                      showError(getErrorMessage(exception), 0)
                    })
                  return [result, updatedItem]
                }}
@ -303,7 +338,7 @@ const CodeCommentHeader: React.FC<CodeCommentHeaderProps> = ({ commentItems }) =
        <Layout.Vertical>
          <Container className={css.title}>
            <Text inline className={css.fname}>
-              {payload?.file_title || ''}
+              {payload?.file_title}
            </Text>
          </Container>
          <Container className={css.snapshotContent}>
@ -331,7 +366,7 @@ const CodeCommentHeader: React.FC<CodeCommentHeaderProps> = ({ commentItems }) =
 }

 function isSystemComment(commentItems: CommentItem<TypesPullReqActivity>[]) {
-  return commentItems.length === 1 && commentItems[0].payload?.kind === 'system'
+  return commentItems[0].payload?.kind === 'system'
 }

 interface SystemBoxProps extends Pick<GitInfoProps, 'pullRequestMetadata'> {
@ -340,25 +375,79 @@ interface SystemBoxProps extends Pick<GitInfoProps, 'pullRequestMetadata'> {

 const SystemBox: React.FC<SystemBoxProps> = ({ pullRequestMetadata, commentItems }) => {
  const { getString } = useStrings()
-  const type = commentItems[0].payload?.type
+  const payload = commentItems[0].payload
+  const type = payload?.type

  switch (type) {
    case CommentType.MERGE: {
      return (
-        <Text className={css.box}>
-          <Icon name={CodeIcon.PullRequest} color={Color.PURPLE_700} padding={{ right: 'small' }} />
-          <StringSubstitute
-            str={getString('pr.prMergedInfo')}
-            vars={{
-              user: <strong>{pullRequestMetadata.merger?.display_name}</strong>,
-              source: <strong>{pullRequestMetadata.source_branch}</strong>,
-              target: <strong>{pullRequestMetadata.target_branch} </strong>,
-              time: <ReactTimeago date={pullRequestMetadata.merged as number} />
-            }}
-          />
-        </Text>
+        <Container>
+          <Layout.Horizontal spacing="small" style={{ alignItems: 'center' }} className={css.box}>
+            <Avatar name={pullRequestMetadata.merger?.display_name} size="small" hoverCard={false} />
+            <Text>
+              <StringSubstitute
+                str={getString('pr.prMergedInfo')}
+                vars={{
+                  user: <strong>{pullRequestMetadata.merger?.display_name}</strong>,
+                  source: <strong>{pullRequestMetadata.source_branch}</strong>,
+                  target: <strong>{pullRequestMetadata.target_branch} </strong>,
+                  time: <ReactTimeago date={pullRequestMetadata.merged as number} />
+                }}
+              />
+            </Text>
+          </Layout.Horizontal>
+        </Container>
      )
    }
+
+    case CommentType.TITLE_CHANGE: {
+      return (
+        <Container className={css.box}>
+          <Layout.Horizontal spacing="small" style={{ alignItems: 'center' }}>
+            <Avatar name={payload?.author?.display_name} size="small" hoverCard={false} />
+            <Text tag="div">
+              <StringSubstitute
+                str={getString('pr.titleChanged')}
+                vars={{
+                  user: <strong>{payload?.author?.display_name}</strong>,
+                  old: (
+                    <strong>
+                      <s>{payload?.payload?.old}</s>
+                    </strong>
+                  ),
+                  new: <strong>{payload?.payload?.new}</strong>
+                }}
+              />
+            </Text>
+            <FlexExpander />
+            <Text inline font={{ variation: FontVariation.SMALL }} color={Color.GREY_400}>
+              <ReactTimeago date={payload?.created as number} />
+            </Text>
+          </Layout.Horizontal>
+          <Render when={commentItems.length > 1}>
+            <Container
+              margin={{ top: 'medium', left: 'xxxlarge' }}
+              style={{ maxWidth: 'calc(100vw - 450px)', overflow: 'auto' }}>
+              <MarkdownViewer
+                source={[getString('pr.titleChangedTable').replace(/\n$/, '')]
+                  .concat(
+                    commentItems
+                      .filter((_, index) => index > 0)
+                      .map(
+                        item =>
+                          `|${item.author}|<s>${item.payload?.payload?.old}</s>|${
+                            item.payload?.payload?.new
+                          }|${formatDate(item.updated)} ${formatTime(item.updated)}|`
+                      )
+                  )
+                  .join('\n')}
+              />
+            </Container>
+          </Render>
+        </Container>
+      )
+    }
+
    default: {
      // eslint-disable-next-line no-console
      console.warn('Unable to render system type activity', commentItems)
@ -371,3 +460,29 @@ const SystemBox: React.FC<SystemBoxProps> = ({ pullRequestMetadata, commentItems
    }
  }
 }
+
+function useAnimateNewCommentBox(
+  commentCreated: boolean,
+  setCommentCreated: React.Dispatch<React.SetStateAction<boolean>>
+) {
+  useEffect(() => {
+    let timeoutId = 0
+
+    if (commentCreated) {
+      timeoutId = window.setTimeout(() => {
+        const box = document.querySelector(`.${css.newCommentCreated}`)
+
+        box?.scrollIntoView({ behavior: 'smooth', block: 'center' })
+
+        timeoutId = window.setTimeout(() => {
+          box?.classList.add(css.clear)
+          timeoutId = window.setTimeout(() => setCommentCreated(false), 2000)
+        }, 5000)
+      }, 300)
+    }
+
+    return () => {
+      clearTimeout(timeoutId)
+    }
+  }, [commentCreated, setCommentCreated])
+}
--- a/web/src/pages/PullRequest/PullRequest.module.scss
+++ b/web/src/pages/PullRequest/PullRequest.module.scss
@ -75,7 +75,7 @@
    margin-bottom: 0 !important;

    input {
-      width: 400px;
+      width: 800px;
      font-weight: 600;
      padding: 0 var(--spacing-small) !important;
      line-height: 22px !important;
--- a/web/src/pages/PullRequest/PullRequest.tsx
+++ b/web/src/pages/PullRequest/PullRequest.tsx
@ -1,4 +1,4 @@
-import React, { useMemo, useState } from 'react'
+import React, { useCallback, useMemo, useState } from 'react'
 import {
  Container,
  PageBody,
@ -163,6 +163,17 @@ const PullRequestTitle: React.FC<PullRequestTitleProps> = ({ repoMetadata, title
    verb: 'PATCH',
    path: `/api/v1/repos/${repoMetadata.path}/+/pullreq/${number}`
  })
+  const submitChange = useCallback(() => {
+    mutate({
+      title: val,
+      description
+    })
+      .then(() => {
+        setEdit(false)
+        setOriginal(val)
+      })
+      .catch(exception => showError(getErrorMessage(exception), 0))
+  }, [description, val, mutate, showError])

  return (
    <Layout.Horizontal spacing="xsmall" className={css.prTitle}>
@ -173,25 +184,26 @@ const PullRequestTitle: React.FC<PullRequestTitleProps> = ({ repoMetadata, title
              <TextInput
                wrapperClassName={css.input}
                value={val}
+                onFocus={event => event.target.select()}
                onInput={event => setVal(event.currentTarget.value)}
                autoFocus
+                onKeyDown={event => {
+                  switch (event.key) {
+                    case 'Enter':
+                      submitChange()
+                      break
+                    case 'Escape': // does not work, maybe TextInput cancels ESC?
+                      setEdit(false)
+                      break
+                  }
+                }}
              />
              <Button
                variation={ButtonVariation.PRIMARY}
                text={getString('save')}
                size={ButtonSize.MEDIUM}
                disabled={(val || '').trim().length === 0 || title === val}
-                onClick={() => {
-                  mutate({
-                    title: val,
-                    description
-                  })
-                    .then(() => {
-                      setEdit(false)
-                      setOriginal(val)
-                    })
-                    .catch(exception => showError(getErrorMessage(exception), 0))
-                }}
+                onClick={submitChange}
              />
              <Button
                variation={ButtonVariation.TERTIARY}
--- a/web/src/pages/RepositoryBranches/RepositoryBranchesContent/BranchesContent/BranchesContent.tsx
+++ b/web/src/pages/RepositoryBranches/RepositoryBranchesContent/BranchesContent/BranchesContent.tsx
@ -101,7 +101,7 @@ export function BranchesContent({ repoMetadata, searchTerm = '', branches, onDel
        width: '200px',
        Cell: ({ row }: CellProps<RepoBranch>) => {
          return (
-            <Text className={css.rowText} color={Color.BLACK}>
+            <Text className={css.rowText} color={Color.BLACK} tag="div">
              <Avatar hoverCard={false} size="small" name={row.original.commit?.author?.identity?.name || ''} />
              <span className={css.spacer} />
              {formatDate(row.original.commit?.author?.when as string)}
--- a/web/src/utils/GitUtils.ts
+++ b/web/src/utils/GitUtils.ts
@ -63,6 +63,7 @@ export const PullRequestFilterOption = {
 }

 export const CodeIcon = {
+  Logo: 'code' as IconName,
  PullRequest: 'git-pull' as IconName,
  PullRequestRejected: 'main-close' as IconName,
  Add: 'plus' as IconName,