From 821d79d53cdf42b749bdda343cb19bc4a2d6be15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Ctan-nhu=E2=80=9D?= <“tan@harness.io”> Date: Fri, 28 Apr 2023 10:23:12 -0700 Subject: [PATCH 1/2] Implememnt diff view in editing file --- web/src/components/Changes/Changes.tsx | 17 +-- web/src/components/DiffViewer/DiffViewer.tsx | 50 ++++---- .../components/DiffViewer/DiffViewerUtils.tsx | 74 ------------ .../OptionsMenuButton/OptionsMenuButton.tsx | 3 +- .../MonacoSourceCodeEditor.tsx | 107 +++++++++++++----- web/src/framework/strings/stringTypes.ts | 2 + web/src/i18n/strings.en.yaml | 4 +- .../FileEditor/FileEditor.module.scss | 47 ++++++-- .../FileEditor/FileEditor.module.scss.d.ts | 3 +- .../FileEditor/FileEditor.tsx | 45 +++++--- web/src/utils/Utils.ts | 1 - web/src/utils/types.ts | 2 +- 12 files changed, 193 insertions(+), 162 deletions(-) diff --git a/web/src/components/Changes/Changes.tsx b/web/src/components/Changes/Changes.tsx index 2cd56501a..be510854f 100644 --- a/web/src/components/Changes/Changes.tsx +++ b/web/src/components/Changes/Changes.tsx @@ -23,12 +23,7 @@ import { useEventListener } from 'hooks/useEventListener' import { UserPreference, useUserPreference } from 'hooks/useUserPreference' import { PipeSeparator } from 'components/PipeSeparator/PipeSeparator' import type { DiffFileEntry } from 'utils/types' -import { - DIFF2HTML_CONFIG, - PR_CODE_COMMENT_PAYLOAD_VERSION, - PullRequestCodeCommentPayload, - ViewStyle -} from 'components/DiffViewer/DiffViewerUtils' +import { DIFF2HTML_CONFIG, PullRequestCodeCommentPayload, ViewStyle } from 'components/DiffViewer/DiffViewerUtils' import { NoResultCard } from 'components/NoResultCard/NoResultCard' import type { TypesPullReq, TypesPullReqActivity } from 'services/code' import { useShowRequestError } from 'hooks/useShowRequestError' @@ -118,14 +113,10 @@ export const Changes: React.FC = ({ const fileId = changedFileId([diff.oldName, diff.newName]) const containerId = `container-${fileId}` const contentId = `content-${fileId}` - const fileTitle = diff.isDeleted - ? diff.oldName - : diff.isRename - ? `${diff.oldName} -> ${diff.newName}` - : diff.newName + const filePath = diff.isDeleted ? diff.oldName : diff.newName const fileActivities: TypesPullReqActivity[] | undefined = activities?.filter(activity => { const payload = activity.payload as PullRequestCodeCommentPayload - return payload?.file_id === fileId && payload?.version === PR_CODE_COMMENT_PAYLOAD_VERSION + return payload?.file_id === fileId }) return { @@ -133,7 +124,7 @@ export const Changes: React.FC = ({ containerId, contentId, fileId, - fileTitle, + filePath, fileActivities: fileActivities || [], activities: activities || [] } diff --git a/web/src/components/DiffViewer/DiffViewer.tsx b/web/src/components/DiffViewer/DiffViewer.tsx index 305930fca..ca69a0061 100644 --- a/web/src/components/DiffViewer/DiffViewer.tsx +++ b/web/src/components/DiffViewer/DiffViewer.tsx @@ -24,7 +24,7 @@ import type { DiffFileEntry } from 'utils/types' import { useConfirmAct } from 'hooks/useConfirmAction' import { PipeSeparator } from 'components/PipeSeparator/PipeSeparator' import { useAppContext } from 'AppContext' -import type { TypesPullReq, TypesPullReqActivity } from 'services/code' +import type { OpenapiCommentCreatePullReqRequest, TypesPullReq, TypesPullReqActivity } from 'services/code' import { getErrorMessage } from 'utils/Utils' import { CopyButton } from 'components/CopyButton/CopyButton' import { AppWrapper } from 'App' @@ -36,9 +36,6 @@ import { DiffCommentItem, DIFF_VIEWER_HEADER_HEIGHT, getCommentLineInfo, - getDiffHTMLSnapshotFromRow, - getRawTextInRange, - PR_CODE_COMMENT_PAYLOAD_VERSION, PullRequestCodeCommentPayload, renderCommentOppositePlaceHolder, ViewStyle @@ -308,22 +305,33 @@ export const DiffViewer: React.FC = ({ switch (action) { case CommentAction.NEW: { - // lineNumberRange can be used to allow multiple-line selection when commenting in the future - const lineNumberRange = [comment.lineNumber] - const payload: PullRequestCodeCommentPayload = { - type: CommentType.CODE_COMMENT, - version: PR_CODE_COMMENT_PAYLOAD_VERSION, - file_id: diff.fileId, - file_title: diff.fileTitle, - language: diff.language || '', - is_on_left: comment.left, - at_line_number: comment.lineNumber, - line_number_range: lineNumberRange, - range_text_content: getRawTextInRange(diff, lineNumberRange), - diff_html_snapshot: getDiffHTMLSnapshotFromRow(rowElement) + const commentPayload: 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: pullRequestMetadata?.merge_sha || '', + target_commit_sha: pullRequestMetadata?.merge_target_sha || '', + text: value } - await saveComment({ type: CommentType.CODE_COMMENT, text: value, payload }) + // lineNumberRange can be used to allow multiple-line selection when commenting in the future + // const lineNumberRange = [comment.lineNumber] + // const payload: PullRequestCodeCommentPayload = { + // type: CommentType.CODE_COMMENT, + // version: PR_CODE_COMMENT_PAYLOAD_VERSION, + // file_id: diff.fileId, + // file_title: diff.filePath, + // language: diff.language || '', + // is_on_left: comment.left, + // at_line_number: comment.lineNumber, + // line_number_range: lineNumberRange, + // range_text_content: getRawTextInRange(diff, lineNumberRange), + // diff_html_snapshot: getDiffHTMLSnapshotFromRow(rowElement) + // } + + await saveComment({ type: CommentType.CODE_COMMENT, ...commentPayload }) .then((newComment: TypesPullReqActivity) => { updatedItem = activityToCommentItem(newComment) }) @@ -472,12 +480,12 @@ export const DiffViewer: React.FC = ({ to={routes.toCODERepository({ repoPath: repoMetadata.path as string, gitRef: pullRequestMetadata?.source_branch, - resourcePath: diff.fileTitle + resourcePath: diff.isRename ? diff.newName : diff.filePath })}> - {diff.fileTitle} + {diff.isRename ? `${diff.oldName} -> ${diff.newName}` : diff.filePath} - + diff --git a/web/src/components/DiffViewer/DiffViewerUtils.tsx b/web/src/components/DiffViewer/DiffViewerUtils.tsx index 95dd4237c..e30127693 100644 --- a/web/src/components/DiffViewer/DiffViewerUtils.tsx +++ b/web/src/components/DiffViewer/DiffViewerUtils.tsx @@ -21,8 +21,6 @@ export enum CommentType { STATE_CHANGE = 'state-change' } -export const PR_CODE_COMMENT_PAYLOAD_VERSION = '0.1' - export interface PullRequestCodeCommentPayload { type: CommentType version: string // used to avoid rendering old payload structure @@ -200,78 +198,6 @@ export const activityToCommentItem = (activity: TypesPullReqActivity): CommentIt payload: activity }) -/** - * Take a small HTML snapshot of a diff in order to render code comment. - * @param atRow Row element where the comment is placed. - * @param maxNumberOfLines Maximum number of lines to take. - * @returns HTML content of the diff. - */ -export function getDiffHTMLSnapshotFromRow(atRow: HTMLTableRowElement, maxNumberOfLines = 5) { - let linesCapturedCount = 0 - const diffSnapshot = [atRow.outerHTML.trim()] - - let prev = atRow.previousElementSibling - - while (prev && linesCapturedCount < maxNumberOfLines) { - if (!prev.hasAttribute('data-annotated-line') && !prev.hasAttribute('data-place-holder-for-line')) { - // Don't count empty lines - const textContent = prev.textContent?.replace(/\s/g, '') - if (textContent?.length && textContent !== '+') { - linesCapturedCount++ - } - - if (textContent !== '+') { - diffSnapshot.unshift((prev.outerHTML || '').trim()) - } - } - prev = prev.previousElementSibling - } - - return diffSnapshot.join('') -} - -// export function getDiffHTMLSnapshotFromDiff(diff: DiffFileEntry, lineNumberRange: number[], isOnLeft: boolean) { -// const lines = diff?.blocks.reduce((group, item) => { -// group = group.concat(item.lines) -// return group -// }, [] as DiffLine[]) - -// const lastIndex = lines.findIndex(line => -// lineNumberRange.includes((isOnLeft ? line.oldNumber : line.newNumber) as number) -// ) -// const startIndex = Math.max(0, lastIndex - 5) - -// const copiedLines = lines.slice(startIndex, lastIndex) -// const copiedDiff: DiffFileEntry = { -// ...diff, -// blocks: [ -// { -// header: '', -// lines: copiedLines, -// newStartLine: copiedLines[0].newNumber as number, -// oldStartLine: copiedLines[0].oldNumber as number -// } -// ] -// } - -// // console.log({ isOnLeft, startIndex, lastIndex, copiedLines, lines, lineNumberRange }) - -// const div = document.createElement('div') -// new Diff2HtmlUI(div, [copiedDiff], Object.assign({}, DIFF2HTML_CONFIG, { outputFormat: 'line-by-line' })).draw() - -// return div.querySelector('table')?.outerHTML || '' -// } - -export function getRawTextInRange(diff: DiffFileEntry, lineNumberRange: number[]) { - return ( - // TODO: This is wrong, blocks can have multiple items, not one - (diff?.blocks[0]?.lines || []) - .filter(line => lineNumberRange.includes(line.newNumber as number)) - .map(line => line.content) - .join('\n') || '' - ) -} - export function activitiesToDiffCommentItems(diff: DiffFileEntry): DiffCommentItem[] { return ( diff.fileActivities?.map(activity => { diff --git a/web/src/components/OptionsMenuButton/OptionsMenuButton.tsx b/web/src/components/OptionsMenuButton/OptionsMenuButton.tsx index 99dc29888..3d13ff28d 100644 --- a/web/src/components/OptionsMenuButton/OptionsMenuButton.tsx +++ b/web/src/components/OptionsMenuButton/OptionsMenuButton.tsx @@ -63,7 +63,8 @@ export const OptionsMenuButton = ({ item as IMenuItemProps & React.AnchorHTMLAttributes, 'isDanger', 'hasIcon', - 'iconName' + 'iconName', + 'iconSize' )} /> ) diff --git a/web/src/components/SourceCodeEditor/MonacoSourceCodeEditor.tsx b/web/src/components/SourceCodeEditor/MonacoSourceCodeEditor.tsx index 9abc286c3..46ac697ce 100644 --- a/web/src/components/SourceCodeEditor/MonacoSourceCodeEditor.tsx +++ b/web/src/components/SourceCodeEditor/MonacoSourceCodeEditor.tsx @@ -1,9 +1,9 @@ -import React, { useEffect } from 'react' -import { Container } from '@harness/uicore' +import React, { useEffect, useState } from 'react' import type monacoEditor from 'monaco-editor/esm/vs/editor/editor.api' -import MonacoEditor from 'react-monaco-editor' +import MonacoEditor, { MonacoDiffEditor } from 'react-monaco-editor' import { noop } from 'lodash-es' import { SourceCodeEditorProps, PLAIN_TEXT } from 'utils/Utils' +import { useEventListener } from 'hooks/useEventListener' export const MonacoEditorOptions = { ignoreTrimWhitespace: true, @@ -58,12 +58,12 @@ export default function MonacoSourceCodeEditor({ language = PLAIN_TEXT, lineNumbers = true, readOnly = false, - className, height, autoHeight, wordWrap = true, onChange = noop }: SourceCodeEditorProps) { + const [editor, setEditor] = useState() const scrollbar = autoHeight ? 'hidden' : 'auto' useEffect(() => { @@ -72,31 +72,80 @@ export default function MonacoSourceCodeEditor({ monaco.languages.typescript?.typescriptDefaults?.setCompilerOptions?.(compilerOptions) }, []) + useEventListener('resize', () => { + editor?.layout({ width: 0, height: 0 }) + window.requestAnimationFrame(() => editor?.layout()) + }) + return ( - - { - if (autoHeight) { - autoAdjustEditorHeight(_editor) - } - }} - onChange={onChange} - /> - + { + if (autoHeight) { + autoAdjustEditorHeight(_editor) + } + setEditor(_editor) + }} + onChange={onChange} + /> + ) +} + +interface DiffEditorProps extends Omit { + original: string +} + +export function DiffEditor({ + source, + original, + language = PLAIN_TEXT, + lineNumbers = true, + readOnly = false, + height, + wordWrap = true, + onChange = noop +}: DiffEditorProps) { + const [editor, setEditor] = useState() + + useEventListener('resize', () => { + editor?.layout({ width: 0, height: 0 }) + window.requestAnimationFrame(() => editor?.layout()) + }) + + return ( + ) } diff --git a/web/src/framework/strings/stringTypes.ts b/web/src/framework/strings/stringTypes.ts index 9728fee7e..96eacc100 100644 --- a/web/src/framework/strings/stringTypes.ts +++ b/web/src/framework/strings/stringTypes.ts @@ -38,6 +38,7 @@ export interface StringsMap { branches: string browse: string cancel: string + changes: string checkRuns: string checkSuites: string checks: string @@ -62,6 +63,7 @@ export interface StringsMap { confirmDeleteWebhook: string confirmation: string content: string + contents: string conversation: string copy: string copyCommitSHA: string diff --git a/web/src/i18n/strings.en.yaml b/web/src/i18n/strings.en.yaml index 2077c65ff..3aad3b792 100644 --- a/web/src/i18n/strings.en.yaml +++ b/web/src/i18n/strings.en.yaml @@ -192,7 +192,7 @@ pr: failedToDeleteComment: Failed to delete comment. Please try again. prMerged: This Pull Request was merged reviewSubmitted: Review submitted. - prReviewSubmit: '{user} {state|approved:approved, rejected:rejected,changereq:requested changes to, reviewed} the pull request. {time}' + prReviewSubmit: '{user} {state|approved:approved, rejected:rejected,changereq:requested to, reviewed} the pull request. {time}' prMergedInfo: '{user} merged branch {source} into {target} {time}.' prBranchPushInfo: '{user} pushed a new commit {commit}.' prStateChanged: '{user} changed pull request state from {old} to {new}.' @@ -375,3 +375,5 @@ manageCredText: You can also manage your git credential {URL} blame: Blame viewRaw: View Raw download: Download +changes: Changes +contents: Contents diff --git a/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.module.scss b/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.module.scss index 360f13a08..0deab39b9 100644 --- a/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.module.scss +++ b/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.module.scss @@ -1,9 +1,13 @@ .container { + --header-height: 96px; + --heading-height: 58px; + --tabs-height: 36px; + background-color: var(--white) !important; margin-top: 0 !important; // box-shadow: 0px 0px 1px rgba(40, 41, 61, 0.08), 0px 0.5px 2px rgba(96, 97, 112, 0.16); // border-radius: 4px; - height: calc(100vh - 96px); + height: calc(100vh - var(--header-height)); overflow: hidden; .heading { @@ -11,7 +15,7 @@ // border-top-right-radius: 4px; align-items: center; padding: 0 var(--spacing-xlarge) !important; - height: 58px; + height: var(--heading-height); background-color: var(--grey-100); box-shadow: 0px 0px 1px rgba(40, 41, 61, 0.08), 0px 0.5px 2px rgba(96, 97, 112, 0.16); border-bottom: 1px solid var(--grey-200); @@ -40,13 +44,42 @@ } } - .content { + .tabs { + height: var(--tabs-height); + display: flex; + align-items: center; + justify-content: center; + background-color: var(--grey-50); + --tab-height: 18px; + + .selectedView { + height: var(--tab-height); + + > div { + align-items: center; + display: flex; + height: var(--tab-height) !important; + width: auto; + padding: 0 var(--spacing-large); + font-weight: 700; + font-size: 10px; + text-transform: uppercase; + + &[class*='selected'] { + background-color: var(--primary-9); + } + + &:not([class*='selected']) { + color: var(--primary-9); + } + } + } + } + + .editorContainer { padding-left: var(--spacing-medium) !important; overflow: hidden; - .editorContainer { - height: calc(100vh - 96px - 58px); - overflow: hidden; - } + height: calc(100vh - var(--header-height) - var(--heading-height) - var(--tabs-height)); } } diff --git a/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.module.scss.d.ts b/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.module.scss.d.ts index 2e3e51811..2853f09db 100644 --- a/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.module.scss.d.ts +++ b/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.module.scss.d.ts @@ -6,7 +6,8 @@ declare const styles: { readonly path: string readonly inputContainer: string readonly refLink: string - readonly content: string + readonly tabs: string + readonly selectedView: string readonly editorContainer: string } export default styles diff --git a/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.tsx b/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.tsx index 9e9aa3d95..e6a2553dc 100644 --- a/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.tsx +++ b/web/src/pages/RepositoryFileEdit/FileEditor/FileEditor.tsx @@ -1,5 +1,17 @@ import React, { ChangeEvent, useCallback, useEffect, useMemo, useRef, useState } from 'react' -import { Button, ButtonVariation, Color, Container, FlexExpander, Icon, Layout, Text, TextInput } from '@harness/uicore' +import { + Button, + ButtonVariation, + Color, + Container, + FlexExpander, + Icon, + Layout, + Text, + TextInput, + VisualYamlSelectedView, + VisualYamlToggle +} from '@harness/uicore' import { Link, useHistory } from 'react-router-dom' import ReactJoin from 'react-join' import cx from 'classnames' @@ -11,6 +23,7 @@ import { useStrings } from 'framework/strings' import { filenameToLanguage, FILE_SEPERATOR } from 'utils/Utils' import { useGetResourceContent } from 'hooks/useGetResourceContent' import { CommitModalButton } from 'components/CommitModalButton/CommitModalButton' +import { DiffEditor } from 'components/SourceCodeEditor/MonacoSourceCodeEditor' import css from './FileEditor.module.scss' interface EditorProps extends Pick { @@ -83,6 +96,7 @@ function Editor({ resourceContent, repoMetadata, gitRef, resourcePath, isReposit // Make API call to verify if fileResourcePath is an existing folder verifyFolder().then(() => setStartVerifyFolder(true)) }, [fileName, parentPath, language, content, verifyFolder]) + const [selectedView, setSelectedView] = useState(VisualYamlSelectedView.VISUAL) // Calculate file name input field width based on number of characters inside useEffect(() => { @@ -111,16 +125,14 @@ function Editor({ resourceContent, repoMetadata, gitRef, resourcePath, isReposit } } }, [isNew, name]) + return ( - - {/* - {repoMetadata.uid} - */} + {parentPath && ( @@ -148,7 +160,7 @@ function Editor({ resourceContent, repoMetadata, gitRef, resourcePath, isReposit (inputRef.current = ref)} + inputRef={_ref => (inputRef.current = _ref)} wrapperClassName={css.inputContainer} placeholder={getString('nameYourFile')} onInput={(event: ChangeEvent) => { @@ -231,15 +243,22 @@ function Editor({ resourceContent, repoMetadata, gitRef, resourcePath, isReposit - - + + + + {selectedView === VisualYamlSelectedView.VISUAL ? ( + + ) : ( + + )} + ) } diff --git a/web/src/utils/Utils.ts b/web/src/utils/Utils.ts index dfd3362f3..a014289e5 100644 --- a/web/src/utils/Utils.ts +++ b/web/src/utils/Utils.ts @@ -49,7 +49,6 @@ export interface SourceCodeEditorProps { lineNumbers?: boolean readOnly?: boolean highlightLines?: string // i.e: {1,3-4}, TODO: not yet supported - className?: string height?: number | string autoHeight?: boolean wordWrap?: boolean diff --git a/web/src/utils/types.ts b/web/src/utils/types.ts index 6080b466b..233ef0f2b 100644 --- a/web/src/utils/types.ts +++ b/web/src/utils/types.ts @@ -3,7 +3,7 @@ import type { TypesPullReqActivity } from 'services/code' export interface DiffFileEntry extends DiffFile { fileId: string - fileTitle: string + filePath: string containerId: string contentId: string fileActivities?: TypesPullReqActivity[] From 3840b36285535e2117e6901438d5d88780231eb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Ctan-nhu=E2=80=9D?= <“tan@harness.io”> Date: Fri, 28 Apr 2023 14:59:04 -0700 Subject: [PATCH 2/2] feat: [CODE-263]: API integration for new PR Code Comment API --- web/src/components/Changes/Changes.tsx | 20 ++--- web/src/components/CommentBox/CommentBox.tsx | 2 +- web/src/components/DiffViewer/DiffViewer.tsx | 44 ++++------- .../components/DiffViewer/DiffViewerUtils.tsx | 12 ++- .../MarkdownEditorWithPreview.module.scss | 4 - .../MarkdownViewer/MarkdownViewer.module.scss | 9 +++ .../Conversation/Conversation.module.scss | 5 ++ .../PullRequest/Conversation/Conversation.tsx | 78 +++++++++---------- web/src/services/code/index.tsx | 2 +- web/src/services/code/swagger.yaml | 1 - web/src/utils/utils.scss | 4 +- 11 files changed, 87 insertions(+), 94 deletions(-) diff --git a/web/src/components/Changes/Changes.tsx b/web/src/components/Changes/Changes.tsx index be510854f..0f262b2e6 100644 --- a/web/src/components/Changes/Changes.tsx +++ b/web/src/components/Changes/Changes.tsx @@ -23,11 +23,10 @@ import { useEventListener } from 'hooks/useEventListener' import { UserPreference, useUserPreference } from 'hooks/useUserPreference' import { PipeSeparator } from 'components/PipeSeparator/PipeSeparator' import type { DiffFileEntry } from 'utils/types' -import { DIFF2HTML_CONFIG, PullRequestCodeCommentPayload, ViewStyle } from 'components/DiffViewer/DiffViewerUtils' +import { DIFF2HTML_CONFIG, ViewStyle } from 'components/DiffViewer/DiffViewerUtils' import { NoResultCard } from 'components/NoResultCard/NoResultCard' import type { TypesPullReq, TypesPullReqActivity } from 'services/code' import { useShowRequestError } from 'hooks/useShowRequestError' -// import { Render } from 'components/Render/Render' import { LoadingSpinner } from 'components/LoadingSpinner/LoadingSpinner' import { ChangesDropdown } from './ChangesDropdown' import { DiffViewConfiguration } from './DiffViewConfiguration' @@ -74,7 +73,8 @@ export const Changes: React.FC = ({ data: rawDiff, error, loading, - refetch + refetch, + response } = useGet({ path: `/api/v1/repos/${repoMetadata?.path}/+/${ pullRequestMetadata ? `pullreq/${pullRequestMetadata.number}/diff` : `compare/${targetBranch}...${sourceBranch}` @@ -114,10 +114,9 @@ export const Changes: React.FC = ({ const containerId = `container-${fileId}` const contentId = `content-${fileId}` const filePath = diff.isDeleted ? diff.oldName : diff.newName - const fileActivities: TypesPullReqActivity[] | undefined = activities?.filter(activity => { - const payload = activity.payload as PullRequestCodeCommentPayload - return payload?.file_id === fileId - }) + const fileActivities: TypesPullReqActivity[] | undefined = activities?.filter( + activity => filePath === activity.code_comment_path + ) return { ...diff, @@ -208,11 +207,6 @@ export const Changes: React.FC = ({ pullRequestMetadata={pullRequestMetadata} refreshPr={voidFn(noop)} /> - {/* */} @@ -235,6 +229,8 @@ export const Changes: React.FC = ({ repoMetadata={repoMetadata} pullRequestMetadata={pullRequestMetadata} onCommentUpdate={onCommentUpdate} + mergeBaseSHA={response?.headers?.get('x-merge-base-sha') || ''} + sourceSHA={response?.headers?.get('x-source-sha') || ''} /> ))} diff --git a/web/src/components/CommentBox/CommentBox.tsx b/web/src/components/CommentBox/CommentBox.tsx index 88904d61a..d72723e83 100644 --- a/web/src/components/CommentBox/CommentBox.tsx +++ b/web/src/components/CommentBox/CommentBox.tsx @@ -310,7 +310,7 @@ const CommentsThread = ({ hideGutter={isLastItem}> diff --git a/web/src/components/DiffViewer/DiffViewer.tsx b/web/src/components/DiffViewer/DiffViewer.tsx index ca69a0061..447781c49 100644 --- a/web/src/components/DiffViewer/DiffViewer.tsx +++ b/web/src/components/DiffViewer/DiffViewer.tsx @@ -36,7 +36,6 @@ import { DiffCommentItem, DIFF_VIEWER_HEADER_HEIGHT, getCommentLineInfo, - PullRequestCodeCommentPayload, renderCommentOppositePlaceHolder, ViewStyle } from './DiffViewerUtils' @@ -50,6 +49,8 @@ interface DiffViewerProps extends Pick { readOnly?: boolean pullRequestMetadata?: TypesPullReq onCommentUpdate: () => void + mergeBaseSHA?: string + sourceSHA?: string } // @@ -64,7 +65,9 @@ export const DiffViewer: React.FC = ({ readOnly, repoMetadata, pullRequestMetadata, - onCommentUpdate + onCommentUpdate, + mergeBaseSHA, + sourceSHA }) => { const { routes } = useAppContext() const { getString } = useStrings() @@ -268,12 +271,10 @@ export const DiffViewer: React.FC = ({ const element = commentRowElement.firstElementChild as HTMLTableCellElement - // Note: 1. CommentBox is rendered as an independent React component + // 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 - // 2. If you use a component inside CommentBox, make sure it follow - // the above rules as well (i.e useString as a prop instead of importing) ReactDOM.unmountComponentAtNode(element as HTMLDivElement) ReactDOM.render( @@ -305,33 +306,18 @@ export const DiffViewer: React.FC = ({ switch (action) { case CommentAction.NEW: { - const commentPayload: OpenapiCommentCreatePullReqRequest = { + 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: pullRequestMetadata?.merge_sha || '', - target_commit_sha: pullRequestMetadata?.merge_target_sha || '', + source_commit_sha: sourceSHA, + target_commit_sha: mergeBaseSHA, text: value } - // lineNumberRange can be used to allow multiple-line selection when commenting in the future - // const lineNumberRange = [comment.lineNumber] - // const payload: PullRequestCodeCommentPayload = { - // type: CommentType.CODE_COMMENT, - // version: PR_CODE_COMMENT_PAYLOAD_VERSION, - // file_id: diff.fileId, - // file_title: diff.filePath, - // language: diff.language || '', - // is_on_left: comment.left, - // at_line_number: comment.lineNumber, - // line_number_range: lineNumberRange, - // range_text_content: getRawTextInRange(diff, lineNumberRange), - // diff_html_snapshot: getDiffHTMLSnapshotFromRow(rowElement) - // } - - await saveComment({ type: CommentType.CODE_COMMENT, ...commentPayload }) + await saveComment(payload) .then((newComment: TypesPullReqActivity) => { updatedItem = activityToCommentItem(newComment) }) @@ -344,7 +330,7 @@ export const DiffViewer: React.FC = ({ case CommentAction.REPLY: { const parentComment = diff.fileActivities?.find( - activity => (activity.payload as PullRequestCodeCommentPayload).file_id === diff.fileId + activity => diff.filePath === activity.code_comment_path ) if (parentComment) { @@ -412,10 +398,6 @@ export const DiffViewer: React.FC = ({ } } } - // Comment no longer has UI relevant anchors to be rendered - // else { - // console.info('Comment is discarded due to no UI relevant anchors', { comment, lineInfo }) - // } }) }, [ @@ -430,7 +412,9 @@ export const DiffViewer: React.FC = ({ updateComment, deleteComment, confirmAct, - onCommentUpdate + onCommentUpdate, + mergeBaseSHA, + sourceSHA ] ) diff --git a/web/src/components/DiffViewer/DiffViewerUtils.tsx b/web/src/components/DiffViewer/DiffViewerUtils.tsx index e30127693..5ba70237c 100644 --- a/web/src/components/DiffViewer/DiffViewerUtils.tsx +++ b/web/src/components/DiffViewer/DiffViewerUtils.tsx @@ -21,6 +21,9 @@ export enum CommentType { STATE_CHANGE = 'state-change' } +/** + * @deprecated + */ export interface PullRequestCodeCommentPayload { type: CommentType version: string // used to avoid rendering old payload structure @@ -201,17 +204,18 @@ export const activityToCommentItem = (activity: TypesPullReqActivity): CommentIt export function activitiesToDiffCommentItems(diff: DiffFileEntry): DiffCommentItem[] { return ( diff.fileActivities?.map(activity => { - const payload = activity.payload as PullRequestCodeCommentPayload const replyComments = diff.activities ?.filter(replyActivity => replyActivity.parent_id === activity.id) .map(_activity => activityToCommentItem(_activity)) || [] + // TODO: Use backend support when it's ready https://harness.slack.com/archives/C03Q1Q4C9J8/p1682609265294089 + const left = activity.payload?.line_start_new || false return { - left: payload.is_on_left, - right: !payload.is_on_left, + left, + right: !left, height: 0, - lineNumber: payload.at_line_number, + lineNumber: (left ? activity.code_comment_line_old : activity.code_comment_line_new) as number, commentItems: [activityToCommentItem(activity)].concat(replyComments) } }) || [] diff --git a/web/src/components/MarkdownEditorWithPreview/MarkdownEditorWithPreview.module.scss b/web/src/components/MarkdownEditorWithPreview/MarkdownEditorWithPreview.module.scss index 06dc0e889..0406790d9 100644 --- a/web/src/components/MarkdownEditorWithPreview/MarkdownEditorWithPreview.module.scss +++ b/web/src/components/MarkdownEditorWithPreview/MarkdownEditorWithPreview.module.scss @@ -91,10 +91,6 @@ .anchor { display: none; } - - pre { - position: relative; - } } } } diff --git a/web/src/components/MarkdownViewer/MarkdownViewer.module.scss b/web/src/components/MarkdownViewer/MarkdownViewer.module.scss index 9eb1cc4ce..7beae28e2 100644 --- a/web/src/components/MarkdownViewer/MarkdownViewer.module.scss +++ b/web/src/components/MarkdownViewer/MarkdownViewer.module.scss @@ -1,8 +1,12 @@ +@import 'src/utils/utils'; + .main { overflow: auto; :global { .wmde-markdown { + @include markdown-font; + pre { position: relative; @@ -11,6 +15,11 @@ } } + tt, + code { + @include mono-font; + } + // Customize https://wangchujiang.com/rehype-video/ details.octicon.octicon-video { display: block; diff --git a/web/src/pages/PullRequest/Conversation/Conversation.module.scss b/web/src/pages/PullRequest/Conversation/Conversation.module.scss index 113cd6ef2..875a4a893 100644 --- a/web/src/pages/PullRequest/Conversation/Conversation.module.scss +++ b/web/src/pages/PullRequest/Conversation/Conversation.module.scss @@ -83,6 +83,7 @@ .snapshot { --border-color: var(--grey-200); --radius: 4px; + margin-top: var(--spacing-small) !important; .title { display: grid; @@ -110,6 +111,10 @@ border-bottom-right-radius: var(--radius); :global { + .d2h-file-header { + display: none; + } + .d2h-wrapper > div { margin-bottom: 0; } diff --git a/web/src/pages/PullRequest/Conversation/Conversation.tsx b/web/src/pages/PullRequest/Conversation/Conversation.tsx index e98dadbc0..7ef63b18b 100644 --- a/web/src/pages/PullRequest/Conversation/Conversation.tsx +++ b/web/src/pages/PullRequest/Conversation/Conversation.tsx @@ -19,8 +19,10 @@ import { } from '@harness/uicore' import cx from 'classnames' import { useGet, useMutate } from 'restful-react' +import { Diff2HtmlUI } from 'diff2html/lib-esm/ui/js/diff2html-ui' import ReactTimeago from 'react-timeago' -import { orderBy } from 'lodash-es' +import * as Diff2Html from 'diff2html' +import { get, orderBy } from 'lodash-es' import { Render } from 'react-jsx-match' import { CodeIcon, GitInfoProps } from 'utils/GitUtils' import { MarkdownViewer } from 'components/MarkdownViewer/MarkdownViewer' @@ -30,11 +32,7 @@ import type { TypesPullReqActivity } from 'services/code' import { CommentAction, CommentBox, CommentBoxOutletPosition, CommentItem } from 'components/CommentBox/CommentBox' import { useConfirmAct } from 'hooks/useConfirmAction' import { commentState, formatDate, formatTime, getErrorMessage, orderSortDate, dayAgoInMS } from 'utils/Utils' -import { - activityToCommentItem, - CommentType, - PullRequestCodeCommentPayload -} from 'components/DiffViewer/DiffViewerUtils' +import { activityToCommentItem, CommentType, DIFF2HTML_CONFIG, ViewStyle } from 'components/DiffViewer/DiffViewerUtils' import { ThreadSection } from 'components/ThreadSection/ThreadSection' import { PullRequestTabContentWrapper } from '../PullRequestTabContentWrapper' import { DescriptionBox } from './DescriptionBox' @@ -447,7 +445,7 @@ export const Conversation: React.FC = ({ } function isCodeComment(commentItems: CommentItem[]) { - return (commentItems[0]?.payload?.payload as Unknown)?.type === CommentType.CODE_COMMENT + return commentItems[0]?.payload?.type === CommentType.CODE_COMMENT } interface CodeCommentHeaderProps { @@ -455,39 +453,41 @@ interface CodeCommentHeaderProps { } const CodeCommentHeader: React.FC = ({ commentItems }) => { - if (isCodeComment(commentItems)) { - const payload = commentItems[0]?.payload?.payload as PullRequestCodeCommentPayload + const _isCodeComment = isCodeComment(commentItems) + const id = `code-comment-snapshot-${commentItems[0]?.payload?.code_comment_path}` - return ( - - - - - {payload?.file_title} - - - - - - - - - -
- - - - - - - - ) - } - return null + useEffect(() => { + if (_isCodeComment) { + const codeDiffSnapshot = [ + `diff --git a/hello-world.md b/hello-world.md`, + `new file mode 100644`, + 'index 0000000..0000000', + '--- /dev/null', + '+++ b/hello-world.md', + get(commentItems[0], 'payload.payload.title', ''), + ...get(commentItems[0], 'payload.payload.lines', []) + ].join('\n') + + new Diff2HtmlUI( + document.getElementById(id) as HTMLElement, + Diff2Html.parse(codeDiffSnapshot, DIFF2HTML_CONFIG), + Object.assign({}, DIFF2HTML_CONFIG, { outputFormat: ViewStyle.LINE_BY_LINE }) + ).draw() + } + }, [id, commentItems, _isCodeComment]) + + return _isCodeComment ? ( + + + + + {commentItems[0].payload?.code_comment_path} + + + + + + ) : null } function isSystemComment(commentItems: CommentItem[]) { diff --git a/web/src/services/code/index.tsx b/web/src/services/code/index.tsx index bd25c61c8..702dbf2d7 100644 --- a/web/src/services/code/index.tsx +++ b/web/src/services/code/index.tsx @@ -398,7 +398,7 @@ export interface TypesPullReq { description?: string edited?: number is_draft?: boolean - merge_base_sha?: string | null + merge_base_sha?: string merge_check_status?: EnumMergeCheckStatus merge_conflicts?: string | null merge_method?: EnumMergeMethod diff --git a/web/src/services/code/swagger.yaml b/web/src/services/code/swagger.yaml index 035c789c2..0c2f7c16a 100644 --- a/web/src/services/code/swagger.yaml +++ b/web/src/services/code/swagger.yaml @@ -4344,7 +4344,6 @@ components: is_draft: type: boolean merge_base_sha: - nullable: true type: string merge_check_status: $ref: '#/components/schemas/EnumMergeCheckStatus' diff --git a/web/src/utils/utils.scss b/web/src/utils/utils.scss index 83f6e25e4..dd1093631 100644 --- a/web/src/utils/utils.scss +++ b/web/src/utils/utils.scss @@ -3,7 +3,7 @@ $code-editor-font-family: ui-monospace, 'Cascadia Code', 'Source Code Pro', Menl @mixin mono-font { font-family: var(--font-family-mono) !important; - font-size: 12px !important; + font-size: 13px !important; font-feature-settings: 'liga' 0, 'calt' 0; line-height: 18px; letter-spacing: 0px; @@ -11,5 +11,5 @@ $code-editor-font-family: ui-monospace, 'Cascadia Code', 'Source Code Pro', Menl @mixin markdown-font { font-family: var(--font-family) !important; - font-size: 12px !important; + font-size: 13px !important; }