diff --git a/web/src/components/CommentBox/CommentBox.tsx b/web/src/components/CommentBox/CommentBox.tsx index 00f30d076..da8359941 100644 --- a/web/src/components/CommentBox/CommentBox.tsx +++ b/web/src/components/CommentBox/CommentBox.tsx @@ -42,6 +42,7 @@ export enum CommentBoxOutletPosition { } interface CommentBoxProps { + className?: string getString: UseStringsReturn['getString'] onHeightChange?: (height: number) => void initialContent?: string @@ -61,6 +62,7 @@ interface CommentBoxProps { } export const CommentBox = ({ + className, getString, onHeightChange = noop, initialContent = '', @@ -107,7 +109,7 @@ export const CommentBox = ({ return ( @@ -131,7 +133,6 @@ export const CommentBox = ({ }} outlets={outlets} /> - diff --git a/web/src/components/DiffViewer/DiffViewer.module.scss b/web/src/components/DiffViewer/DiffViewer.module.scss index 93e7683eb..800677c51 100644 --- 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; + } + } } } diff --git a/web/src/framework/strings/stringTypes.ts b/web/src/framework/strings/stringTypes.ts index 68dcdeaa0..4f0ac048d 100644 --- 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 diff --git a/web/src/i18n/strings.en.yaml b/web/src/i18n/strings.en.yaml index 6d4c90d28..0834c8563 100644 --- 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' diff --git a/web/src/pages/PullRequest/Conversation/Conversation.module.scss b/web/src/pages/PullRequest/Conversation/Conversation.module.scss index ace7440ff..f1e080a53 100644 --- 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; + } +} diff --git a/web/src/pages/PullRequest/Conversation/Conversation.module.scss.d.ts b/web/src/pages/PullRequest/Conversation/Conversation.module.scss.d.ts index 475f4e130..6e0b1ac48 100644 --- 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 diff --git a/web/src/pages/PullRequest/Conversation/Conversation.tsx b/web/src/pages/PullRequest/Conversation/Conversation.tsx index 55f68b508..e96de3c3f 100644 --- 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 = ({ }) const { showError } = useToaster() const [newComments, setNewComments] = useState([]) - const commentThreads = useMemo(() => { - const threads: Record[]> = {} + 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[][] = [] - activities?.forEach(activity => { - const thread: CommentItem = 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 = ({ 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 ( @@ -103,7 +131,10 @@ export const Conversation: React.FC = ({ 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 ( @@ -114,6 +145,7 @@ export const Conversation: React.FC = ({ = ({ return [result, updatedItem] }} outlets={{ - [CommentBoxOutletPosition.TOP_OF_FIRST_COMMENT]: + [CommentBoxOutletPosition.TOP_OF_FIRST_COMMENT]: isCodeComment(commentItems) && ( + + ) }} /> ) @@ -187,10 +221,11 @@ export const Conversation: React.FC = ({ .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 = ({ commentItems }) = - {payload?.file_title || ''} + {payload?.file_title} @@ -331,7 +366,7 @@ const CodeCommentHeader: React.FC = ({ commentItems }) = } function isSystemComment(commentItems: CommentItem[]) { - return commentItems.length === 1 && commentItems[0].payload?.kind === 'system' + return commentItems[0].payload?.kind === 'system' } interface SystemBoxProps extends Pick { @@ -340,25 +375,79 @@ interface SystemBoxProps extends Pick { const SystemBox: React.FC = ({ 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 ( - - - {pullRequestMetadata.merger?.display_name}, - source: {pullRequestMetadata.source_branch}, - target: {pullRequestMetadata.target_branch} , - time: - }} - /> - + + + + + {pullRequestMetadata.merger?.display_name}, + source: {pullRequestMetadata.source_branch}, + target: {pullRequestMetadata.target_branch} , + time: + }} + /> + + + ) } + + case CommentType.TITLE_CHANGE: { + return ( + + + + + {payload?.author?.display_name}, + old: ( + + {payload?.payload?.old} + + ), + new: {payload?.payload?.new} + }} + /> + + + + + + + 1}> + + index > 0) + .map( + item => + `|${item.author}|${item.payload?.payload?.old}|${ + item.payload?.payload?.new + }|${formatDate(item.updated)} ${formatTime(item.updated)}|` + ) + ) + .join('\n')} + /> + + + + ) + } + default: { // eslint-disable-next-line no-console console.warn('Unable to render system type activity', commentItems) @@ -371,3 +460,29 @@ const SystemBox: React.FC = ({ pullRequestMetadata, commentItems } } } + +function useAnimateNewCommentBox( + commentCreated: boolean, + setCommentCreated: React.Dispatch> +) { + 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]) +} diff --git a/web/src/pages/PullRequest/PullRequest.module.scss b/web/src/pages/PullRequest/PullRequest.module.scss index b3f3011dd..47270ed3d 100644 --- 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; diff --git a/web/src/pages/PullRequest/PullRequest.tsx b/web/src/pages/PullRequest/PullRequest.tsx index 7d8b21b07..74a168bdb 100644 --- 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 = ({ 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 ( @@ -173,25 +184,26 @@ const PullRequestTitle: React.FC = ({ repoMetadata, title 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 + } + }} />