diff --git a/web/package.json b/web/package.json index 5fa257d5e..ace489193 100644 --- a/web/package.json +++ b/web/package.json @@ -37,7 +37,7 @@ "@blueprintjs/datetime": "3.13.0", "@blueprintjs/select": "3.12.3", "@harness/design-system": "1.4.0", - "@harness/icons": "1.95.1`", + "@harness/icons": "1.95.1", "@harness/ng-tooltip": ">=1.31.25", "@harness/telemetry": ">=1.0.42", "@harness/uicore": "3.95.1", diff --git a/web/src/components/DiffViewer/CommentBox/CommentBox.tsx b/web/src/components/DiffViewer/CommentBox/CommentBox.tsx index 7aa8694af..99bd8d930 100644 --- a/web/src/components/DiffViewer/CommentBox/CommentBox.tsx +++ b/web/src/components/DiffViewer/CommentBox/CommentBox.tsx @@ -28,7 +28,7 @@ interface CommentBoxProps { getString: UseStringsReturn['getString'] onHeightChange: (height: number | 'auto') => void onCancel: () => void - width: string + width?: string contents?: string[] currentUser: CurrentUser } @@ -55,7 +55,7 @@ export const CommentBox: React.FC = ({ onHeightChange(ref.current?.offsetHeight) } }) - // Note: Send 'auto' to avoid rendering flickering + // Note: Send 'auto' to avoid render flickering const onCancelBtnClick = useCallback(() => { if (!contents.length) { onCancel() diff --git a/web/src/components/DiffViewer/DiffViewer.module.scss b/web/src/components/DiffViewer/DiffViewer.module.scss index bc3e89d34..274c48598 100644 --- a/web/src/components/DiffViewer/DiffViewer.module.scss +++ b/web/src/components/DiffViewer/DiffViewer.module.scss @@ -18,6 +18,10 @@ position: relative; &[data-annotated-line] { + background-color: var(--white); + + // These cause one or two pixels mismatched + // calculation due to table spacing gaps // border-top: 1px solid var(--grey-200); // border-bottom: 1px solid var(--grey-200); } @@ -53,7 +57,7 @@ height: 14px; width: 14px; font-weight: 600; - background: var(--purple-500); + background: var(--primary-7); color: var(--white); text-align: center; border-radius: 5px; @@ -65,8 +69,11 @@ &:hover [data-annotation-for-line] { visibility: visible; - transform: scale(1.5); - transition: transform 0.75s; + + &:hover { + transform: scale(1.3); + transition: transform 0.75s; + } } } diff --git a/web/src/components/DiffViewer/DiffViewer.tsx b/web/src/components/DiffViewer/DiffViewer.tsx index 66fdb6299..9191778ce 100644 --- a/web/src/components/DiffViewer/DiffViewer.tsx +++ b/web/src/components/DiffViewer/DiffViewer.tsx @@ -55,10 +55,10 @@ export const DiffViewer: React.FC = ({ diff, index, viewStyle, height: 0, lineNumber: 101, contents: [ - `## Example\nLogs will looks similar to\nimage\n\ngitrpc logs using the \`ctx\` will have the following annotations:\n- \`grpc.service=rpc.ReferenceService\`\n- \`grpc.method=CreateBranch\`\n- \`grpc.peer=127.0.0.1:49364\`\n- \`grpc.request_id=cedrl6p1eqltblt13mgg\`\n\nAdditionally, there will be one extra log entry that logs completion of a grpc call.\n\nIt will contain the additional field:\n- \`grpc.status_code=AlreadyExists\`\n- \`grpc.elapsed_ms=424.349542\``, + `Logs will looks similar to\n\nimage\n\n\ngitrpc logs using the \`ctx\` will have the following annotations:\n- \`grpc.service=rpc.ReferenceService\`\n- \`grpc.method=CreateBranch\`\n- \`grpc.peer=127.0.0.1:49364\`\n- \`grpc.request_id=cedrl6p1eqltblt13mgg\`\n\nAdditionally, there will be one extra log entry that logs completion of a grpc call.\n\nIt will contain the additional field:\n- \`grpc.status_code=AlreadyExists\`\n- \`grpc.elapsed_ms=424.349542\``, `it seems we don't actually do anything with the explicit error type other than calling .Error(), which technically we could do on the original err object too? unless I'm missing something, could we then use errors.Is instead? (would avoid the extra var definitions at the top)`, - `If error is not converted then it will be detailed error: in BranchDelete: Branch doesn't exists. What we want is human readable error: Branch 'name' doesn't exists.`, - `* GitRPC isolated errors, bcoz this will be probably separate repo in future and we dont want every where to include grpc status codes in our main app\n* Errors are explicit for repsonses based on error passing by types`, + //`If error is not converted then it will be detailed error: in BranchDelete: Branch doesn't exists. What we want is human readable error: Branch 'name' doesn't exists.`, + // `* GitRPC isolated errors, bcoz this will be probably separate repo in future and we dont want every where to include grpc status codes in our main app\n* Errors are explicit for repsonses based on error passing by types`, `> global ctx in wire will kill all routines, right? is this affect middlewares and interceptors? because requests should finish they work, right?\n\nI've changed the code now to pass the config directly instead of the systemstore and context, to avoid confusion (what we discussed yesterday - I remove systemstore itself another time).\n\nThe context that was passed didn't impact any go routines, it was only used to load the config from the systmstore.` ] } @@ -182,9 +182,10 @@ export const DiffViewer: React.FC = ({ diff, index, viewStyle, const lineNum1 = lineInfoTD?.querySelector('.line-num1') const lineNum2 = lineInfoTD?.querySelector('.line-num2') - commentItem.left = !!lineNum1?.textContent - commentItem.right = !commentItem.left - commentItem.lineNumber = Number(lineNum1?.textContent || lineNum2?.textContent) + // Right has priority + commentItem.right = !!lineNum2?.textContent + commentItem.left = !commentItem.right + commentItem.lineNumber = Number(lineNum2?.textContent || lineNum1?.textContent) } setComments([...comments, commentItem]) @@ -236,7 +237,7 @@ export const DiffViewer: React.FC = ({ diff, index, viewStyle, const element = commentRowElement.firstElementChild as HTMLTableCellElement - // Note: Since 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 @@ -245,7 +246,7 @@ export const DiffViewer: React.FC = ({ diff, index, viewStyle, { if (typeof boxHeight === 'string') { element.style.height = boxHeight @@ -277,6 +278,9 @@ export const DiffViewer: React.FC = ({ diff, index, viewStyle, renderCommentOppositePlaceHolder(comment, lineInfo.oppositeRowElement) } } + } else { + // Comment no longer has UI relevant anchors to be rendered + console.info('Comment is discarded due to no UI relevant anchors', { comment, lineInfo }) } }) }, diff --git a/web/src/i18n/strings.en.yaml b/web/src/i18n/strings.en.yaml index cef95b639..958c79698 100644 --- a/web/src/i18n/strings.en.yaml +++ b/web/src/i18n/strings.en.yaml @@ -183,7 +183,7 @@ rejected: Rejected yours: Yours all: All scrollToTop: Scroll to top -filesChanged: Files Changed +filesChanged: Changes viewed: Viewed comment: Comment addComment: Add comment diff --git a/web/src/pages/PullRequest/PullRequest.tsx b/web/src/pages/PullRequest/PullRequest.tsx index 7d95421d4..2e3886744 100644 --- a/web/src/pages/PullRequest/PullRequest.tsx +++ b/web/src/pages/PullRequest/PullRequest.tsx @@ -18,7 +18,7 @@ import css from './PullRequest.module.scss' enum PullRequestSection { CONVERSATION = 'conversation', COMMITS = 'commits', - FILES_CHANGED = 'files' + FILES_CHANGED = 'changes' } export default function PullRequest() {