diff --git a/packages/app/src/app/overmind/effects/vscode/ModelsHandler.ts b/packages/app/src/app/overmind/effects/vscode/ModelsHandler.ts index ae16458d538..51fa42eef9a 100644 --- a/packages/app/src/app/overmind/effects/vscode/ModelsHandler.ts +++ b/packages/app/src/app/overmind/effects/vscode/ModelsHandler.ts @@ -51,6 +51,7 @@ export type OnOperationAppliedCallback = (data: OnOperationAppliedData) => void; export type ModuleModel = { changeListener: { dispose: Function }; selections: any[]; + currentLine: number; path: string; model: Promise; comments: Array<{ commentId: string; range: [number, number] }>; @@ -116,6 +117,7 @@ export class ModelsHandler { this.moduleModels[fullPath] = this.moduleModels[fullPath] || { changeListener: null, model: null, + currentLine: 0, path: fullPath, selections: [], comments: [], @@ -125,6 +127,23 @@ export class ModelsHandler { return this.moduleModels[fullPath]; } + public updateLineCommentIndication(model: any, lineNumber: number) { + const moduleModel = this.moduleModels[model.uri.path]; + + moduleModel.currentLine = lineNumber; + + const newDecorationComments = this.createCommentDecorations( + moduleModel.comments, + model, + this.currentCommentThreadId, + moduleModel.currentLine + ); + moduleModel.currentCommentDecorations = model.deltaDecorations( + moduleModel.currentCommentDecorations, + newDecorationComments + ); + } + public changeModule = async (module: Module) => { const moduleModel = this.getModuleModelByPath(module.path); @@ -144,7 +163,8 @@ export class ModelsHandler { const newDecorationComments = this.createCommentDecorations( moduleModel.comments, model, - this.currentCommentThreadId + this.currentCommentThreadId, + moduleModel.currentLine ); moduleModel.currentCommentDecorations = model.deltaDecorations( moduleModel.currentCommentDecorations, @@ -188,13 +208,16 @@ export class ModelsHandler { const newDecorationComments = this.createCommentDecorations( commentThreads, model, - currentCommentThreadId + currentCommentThreadId, + moduleModel.currentLine ); moduleModel.currentCommentDecorations = model.deltaDecorations( existingDecorationComments, newDecorationComments ); + + moduleModel.comments = commentThreads; }); } @@ -673,7 +696,8 @@ export class ModelsHandler { range: [number, number]; }>, model: any, - currentCommentThreadId: string | null + currentCommentThreadId: string | null, + currentLineNumber: number ) { const commentDecorationsByLineNumber = commentThreadDecorations.reduce<{ [lineNumber: string]: Array<{ @@ -695,6 +719,24 @@ export class ModelsHandler { return aggr; }, {}); + const initialDecorations: any[] = + currentLineNumber === -1 + ? [] + : [ + { + range: new this.monaco.Range( + currentLineNumber, + 1, + currentLineNumber, + 1 + ), + options: { + isWholeLine: true, + glyphMarginClassName: `editor-comments-glyph editor-comments-multi editor-comments-add`, + }, + }, + ]; + return Object.keys(commentDecorationsByLineNumber).reduce( (aggr, lineNumberKey) => { const lineCommentDecorations = @@ -721,12 +763,10 @@ export class ModelsHandler { isWholeLine: true, // comment-id- class needs to be the LAST class! glyphMarginClassName: `editor-comments-glyph ${ - activeCommentDecoration ? 'active-comment ' : '' + activeCommentDecoration ? 'editor-comments-active ' : '' }${ - ids.length > 1 - ? `multi-comment multi-comment-${ids.length} ` - : '' - }comment-ids-${ids.join('_')}`, + ids.length > 1 ? `editor-comments-multi` : '' + }editor-comments-ids-${ids.join('_')}`, }, }, { @@ -740,7 +780,7 @@ export class ModelsHandler { } ); }, - [] + initialDecorations ); } } diff --git a/packages/app/src/app/overmind/effects/vscode/icons.css b/packages/app/src/app/overmind/effects/vscode/icons.css index 2ed741c041f..8343302a49b 100644 --- a/packages/app/src/app/overmind/effects/vscode/icons.css +++ b/packages/app/src/app/overmind/effects/vscode/icons.css @@ -18,70 +18,81 @@ opacity: 1; } -.editor-comments-glyph.active-comment.multi-comment:before { +.editor-comments-glyph.active-comment.editor-comments-multi:before { color: #fff; } -.multi-comment:before { +.editor-comments-multi:before { content: '...'; color: #fff; position: absolute; top: -2px; - left: 6px; + left: 7px; z-index: 10; font-size: 8px; font-weight: bold; - letter-spacing: -2px; height: 13px; font-family: 'Inter', sans-serif; } -.vs-dark .multi-comment:before { + +.vs-dark .editor-comments-multi:before { color: #242424; } -.multi-comment-2:before { +.editor-comments-add:before { + content: '+'; + font-size: 10px; + font-weight: bold; + top: 0; + left: 8px; +} + +.editor-comments-multi-2:before { content: '2'; top: 0px; left: 9px; } -.multi-comment-3:before { +.editor-comments-multi-3:before { content: '3'; top: 0px; left: 9px; } -.multi-comment-4:before { +.editor-comments-multi-4:before { content: '4'; top: 0px; left: 9px; } -.multi-comment-5:before { +.editor-comments-multi-5:before { content: '5'; top: 0px; left: 9px; } -.multi-comment-6:before { +.editor-comments-multi-6:before { content: '6'; top: 0px; left: 9px; } -.multi-comment-7:before { +.editor-comments-multi-7:before { content: '7'; top: 0px; left: 9px; } -.multi-comment-8:before { +.editor-comments-multi-8:before { content: '8'; top: 0px; left: 9px; } -.multi-comment-9:before { +.editor-comments-multi-9:before { content: '9'; top: 0px; left: 9px; } -.editor-comments-glyph.active-comment { +.editor-comments-glyph.editor-comments-active:before { + content: ''; +} +.editor-comments-glyph.editor-comments-active { opacity: 1; background-image: url('data:image/svg+xml,%3Csvg width="10" height="10" viewBox="0 0 10 10" fill="none" xmlns="http://www.w3.org/2000/svg"%3E%3Cpath d="M0 5C0 7.76142 2.23858 10 5 10C7.76142 10 10 7.76142 10 5C10 2.23858 7.76142 0 5 0C2.23858 0 0 2.23858 0 5Z" fill="%230971F1"/%3E%3Cpath d="M5 5H0V10H5V5Z" fill="%230971F1"/%3E%3C/svg%3E%0A'); } diff --git a/packages/app/src/app/overmind/effects/vscode/index.ts b/packages/app/src/app/overmind/effects/vscode/index.ts index 2ee83fc54dd..c52d27fcb74 100644 --- a/packages/app/src/app/overmind/effects/vscode/index.ts +++ b/packages/app/src/app/overmind/effects/vscode/index.ts @@ -119,6 +119,7 @@ export class VSCodeEffect { private linter: Linter | null; private modelsHandler: ModelsHandler; private modelSelectionListener: { dispose: Function }; + private modelCursorPositionListener: { dispose: Function }; private modelViewRangeListener: { dispose: Function }; private readOnly: boolean; private elements = { @@ -232,9 +233,14 @@ export class VSCodeEffect { } setTimeout(() => { - const el = document.querySelector('.active-comment'); + const commentGlyphs = document.querySelectorAll( + '.editor-comments-glyph' + ); + const el = Array.from(commentGlyphs).find(glyphEl => + glyphEl.className.includes(commentId) + ); - if (el && el.className.includes(commentId)) { + if (el) { resolve(el.getBoundingClientRect()); } else { findActiveComment(); @@ -1026,6 +1032,10 @@ export class VSCodeEffect { this.modelSelectionListener.dispose(); } + if (this.modelCursorPositionListener) { + this.modelCursorPositionListener.dispose(); + } + if (this.modelViewRangeListener) { this.modelViewRangeListener.dispose(); } @@ -1091,9 +1101,21 @@ export class VSCodeEffect { } }); + this.modelCursorPositionListener = activeEditor.onDidChangeCursorPosition( + cursor => { + const model = activeEditor.getModel(); + + this.modelsHandler.updateLineCommentIndication( + model, + cursor.position.lineNumber + ); + } + ); + this.modelSelectionListener = activeEditor.onDidChangeCursorSelection( selectionChange => { - const lines = activeEditor.getModel().getLinesContent() || []; + const model = activeEditor.getModel(); + const lines = model.getLinesContent() || []; const data: onSelectionChangeData = { primary: getSelection(lines, selectionChange.selection), secondary: selectionChange.secondarySelections.map(s => @@ -1211,11 +1233,13 @@ export class VSCodeEffect { We grab the id of the commenthread by getting the last classname. The last part of the classname is the id. */ - const commentIds = Array.from(target.classList) - .pop() - .split('comment-ids-') - .pop() - .split('_'); + const lastClass = Array.from(target.classList).pop(); + const commentIds = lastClass.startsWith('editor-comments-ids-') + ? lastClass + .split('editor-comments-ids-') + .pop() + .split('_') + : []; const boundingRect = target.getBoundingClientRect(); this.options.onCommentClick({ commentIds, diff --git a/packages/app/src/app/overmind/namespaces/comments/actions.ts b/packages/app/src/app/overmind/namespaces/comments/actions.ts index 19fb754ac17..2c6e63eb5c6 100644 --- a/packages/app/src/app/overmind/namespaces/comments/actions.ts +++ b/packages/app/src/app/overmind/namespaces/comments/actions.ts @@ -81,7 +81,14 @@ export const onCommentClick: Action<{ bottom: number; }; }> = ({ state, actions }, { commentIds, bounds }) => { - if (commentIds.length === 1) { + if (commentIds.includes(state.comments.currentCommentId)) { + actions.comments.closeComment(); + return; + } + + if (!commentIds.length) { + actions.comments.createComment(); + } else if (commentIds.length === 1) { actions.comments.selectComment({ commentId: commentIds[0], bounds, @@ -139,17 +146,15 @@ export const selectComment: AsyncAction<{ path: comment.references[0].metadata.path, }); - state.comments.currentCommentId = commentId; - // update comment position with precise info const referenceBounds = await effects.vscode.getCodeReferenceBoundary( commentId, comment.references[0] ); - const existingDialogBounds = - state.comments.currentCommentPositions?.dialog; + + state.comments.currentCommentId = commentId; state.comments.currentCommentPositions = { - trigger: existingDialogBounds || bounds, + trigger: bounds, dialog: { left: referenceBounds.left, top: referenceBounds.top, @@ -179,12 +184,15 @@ export const createComment: AsyncAction = async ({ state, effects }) => { const selection = state.live.currentSelection; if (selection) { codeReference = { - anchor: selection.primary.selection[0], - head: selection.primary.selection[1], - code: state.editor.currentModule.code.substr( - selection.primary.selection[0], - selection.primary.selection[1] - selection.primary.selection[0] - ), + anchor: + selection.primary.selection[0] || selection.primary.cursorPosition, + head: selection.primary.selection[1] || selection.primary.cursorPosition, + code: selection.primary.selection.length + ? state.editor.currentModule.code.substr( + selection.primary.selection[0], + selection.primary.selection[1] - selection.primary.selection[0] + ) + : '', path: state.editor.currentModule.path, }; } @@ -217,19 +225,29 @@ export const createComment: AsyncAction = async ({ state, effects }) => { const comments = state.comments.comments; comments[sandboxId][id] = optimisticComment; - state.comments.currentCommentId = id; // placeholder value until we know the correct values - const referenceBounds = await effects.vscode.getCodeReferenceBoundary( + const { + left, + right, + top, + bottom, + } = await effects.vscode.getCodeReferenceBoundary( id, optimisticComment.references[0] ); + state.comments.currentCommentId = id; state.comments.currentCommentPositions = { - trigger: referenceBounds, + trigger: { + left, + top, + bottom, + right, + }, dialog: { - left: referenceBounds.left, - top: referenceBounds.top, - bottom: referenceBounds.bottom, - right: referenceBounds.right, + left, + top, + bottom, + right, }, }; }; @@ -245,48 +263,30 @@ export const addComment: AsyncAction<{ const id = OPTIMISTIC_COMMENT_ID; const sandboxId = state.editor.currentSandbox.id; const now = new Date().toString(); - let codeReference: CodeReference = null; - const selection = state.live.currentSelection; - if (selection && selection.primary.selection.length) { - codeReference = { - anchor: selection.primary.selection[0], - head: selection.primary.selection[1], - code: state.editor.currentModule.code.substr( - selection.primary.selection[0], - selection.primary.selection[1] - selection.primary.selection[0] - ), - path: state.editor.currentModule.path, - }; - } - - const optimisticComment: CommentFragment = { - parentComment: parentCommentId ? { id: parentCommentId } : null, - id, - insertedAt: now, - updatedAt: now, - content, - isResolved: false, - user: { - id: state.user.id, - name: state.user.name, - username: state.user.username, - avatarUrl: state.user.avatarUrl, - }, - references: codeReference - ? [ - { - id: '__OPTIMISTIC__', - type: 'code', - metadata: codeReference, - resource: state.editor.currentModule.path, - }, - ] - : [], - comments: [], - }; const comments = state.comments.comments; - comments[sandboxId][id] = optimisticComment; + let optimisticComment: CommentFragment; + if (state.comments.comments[sandboxId][id]) { + optimisticComment = state.comments.comments[sandboxId][id]; + } else { + optimisticComment = { + parentComment: parentCommentId ? { id: parentCommentId } : null, + id, + insertedAt: now, + updatedAt: now, + content, + isResolved: false, + user: { + id: state.user.id, + name: state.user.name, + username: state.user.username, + avatarUrl: state.user.avatarUrl, + }, + references: [], + comments: [], + }; + comments[sandboxId][id] = optimisticComment; + } if (optimisticComment.parentComment) { comments[sandboxId][optimisticComment.parentComment.id].comments.push({ @@ -297,11 +297,11 @@ export const addComment: AsyncAction<{ try { let comment: CommentFragment; - if (codeReference) { + if (optimisticComment.references.length) { const response = await effects.gql.mutations.createCodeComment({ sandboxId, content, - codeReference, + codeReference: optimisticComment.references[0].metadata, }); comment = response.createCodeComment; } else { diff --git a/packages/app/src/app/overmind/namespaces/comments/state.ts b/packages/app/src/app/overmind/namespaces/comments/state.ts index e9c452d99c7..4ed9f3a32f4 100644 --- a/packages/app/src/app/overmind/namespaces/comments/state.ts +++ b/packages/app/src/app/overmind/namespaces/comments/state.ts @@ -3,7 +3,7 @@ import { CommentFragment, CommentWithRepliesFragment } from 'app/graphql/types'; import isToday from 'date-fns/isToday'; import { Derive } from 'app/overmind'; -export const OPTIMISTIC_COMMENT_ID = '__OPTIMISTIC_COMMENT_ID__'; +export const OPTIMISTIC_COMMENT_ID = 'OptimisticCommentId'; type State = { comments: { @@ -47,8 +47,15 @@ export const state: State = { currentCommentPositions: null, comments: {}, currentCommentId: null, - fileComments: ({ currentComments }) => - currentComments.reduce<{ + fileComments: ({ comments }, { editor: { currentSandbox } }) => { + if (!currentSandbox || !comments[currentSandbox.id]) { + return {}; + } + const rootComments = Object.values(comments[currentSandbox.id]).filter( + comment => comment.parentComment == null + ); + + return rootComments.reduce<{ [path: string]: Array<{ commentId: string; range: [number, number]; @@ -67,7 +74,8 @@ export const state: State = { }); return aggr; - }, {}), + }, {}); + }, currentComment: ( { comments, currentCommentId }, { editor: { currentSandbox } } @@ -111,7 +119,8 @@ export const state: State = { } const rootComments = Object.values(comments[currentSandbox.id]).filter( - comment => comment.parentComment == null + comment => + comment.parentComment == null && comment.id !== OPTIMISTIC_COMMENT_ID ); switch (selectedCommentsFilter) { diff --git a/packages/app/src/app/overmind/namespaces/live/state.ts b/packages/app/src/app/overmind/namespaces/live/state.ts index 2c5ce2bf670..44751af9322 100755 --- a/packages/app/src/app/overmind/namespaces/live/state.ts +++ b/packages/app/src/app/overmind/namespaces/live/state.ts @@ -44,7 +44,14 @@ export const state: State = { error: null, liveUserId: null, roomInfo: null, - currentSelection: null, + currentSelection: { + primary: { + cursorPosition: 0, + selection: [], + }, + secondary: [], + source: 'overmind', + }, currentViewRange: null, joinSource: 'sandbox', liveUser: currentState => diff --git a/packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Comment.tsx b/packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Comment.tsx index a7cfc2f3067..ba33113f58c 100644 --- a/packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Comment.tsx +++ b/packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Comment.tsx @@ -46,15 +46,16 @@ export const Comment = React.memo<{ })} id={comment.id} onClick={event => { - const target = event.currentTarget as HTMLElement; // don't trigger comment if you click on the menu // we have to handle this because of an upstream // bug in reach/menu-button + const target = event.target as HTMLElement; if (target.tagName === 'button' || target.tagName === 'svg') { return; } - const boundingRect = target.getBoundingClientRect(); + const currentTarget = event.currentTarget as HTMLElement; + const boundingRect = currentTarget.getBoundingClientRect(); actions.comments.selectComment({ commentId: comment.id, bounds: { @@ -94,7 +95,7 @@ export const Comment = React.memo<{ }) } > - Mark as {comment.isResolved ? 'Unr' : 'r'}esolved + Mark as {comment.isResolved ? 'unresolved' : 'resolved'} diff --git a/packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Dialog/Reply.tsx b/packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Dialog/Reply.tsx index c00f3889192..915a97037e4 100644 --- a/packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Dialog/Reply.tsx +++ b/packages/app/src/app/pages/Sandbox/Editor/Workspace/screens/Comments/Dialog/Reply.tsx @@ -46,7 +46,6 @@ export const Reply = ({ reply }: ReplyProps) => { ReactDOM.createPortal(, document.body); +const DIALOG_WIDTH = 420; + export const Dialog: React.FC = () => { - const { state, actions } = useOvermind(); - const [value, setValue] = useState(''); + const { state } = useOvermind(); + const controller = useAnimation(); + const comment = state.comments.currentComment; - const currentCommentPositions = state.comments.currentCommentPositions; + + // This comment doens't exist in the database, it's an optimistic comment const isNewComment = comment.id === OPTIMISTIC_COMMENT_ID; + const [editing, setEditing] = useState(isNewComment); const { ref: listRef, scrollTop } = useScrollTop(); + const dialogRef = React.useRef(); - const closeDialog = () => actions.comments.closeComment(); - const onSubmitReply = () => { - setValue(''); - actions.comments.addComment({ - content: value, - parentCommentId: comment.id, - }); - }; + const currentCommentPositions = state.comments.currentCommentPositions; + const isCodeComment = + comment.references[0] && comment.references[0].type === 'code'; - const onSubmitNewComment = async () => { - await actions.comments.updateComment({ - commentId: comment.id, - content: value, - }); - setValue(''); - setEditing(false); - }; + // this could rather be `getInitialPosition` + const positions = getPositions( + currentCommentPositions, + isCodeComment, + dialogRef + ); // reset editing when comment changes React.useEffect(() => { setEditing(isNewComment); - }, [comment.id, isNewComment]); + // this could rather be `getAnimatedPosition` + const updatedPositions = getPositions( + currentCommentPositions, + isCodeComment, + dialogRef + ); + controller.start({ + ...updatedPositions.dialogPosition, + scale: 1, + opacity: 1, + }); + }, [ + comment.id, + controller, + currentCommentPositions, + isCodeComment, + isNewComment, + ]); if (!currentCommentPositions) { return null; } - const isCodeComment = - comment.references[0] && comment.references[0].type === 'code'; - - const { animateFrom, dialogPosition } = getPositions( - currentCommentPositions, - isCodeComment - ); - return ( { border: '1px solid', borderColor: 'dialog.border', borderRadius: 4, - width: 420, + width: DIALOG_WIDTH, height: 'auto', maxHeight: '80vh', fontFamily: 'Inter, sans-serif', @@ -97,179 +106,23 @@ export const Dialog: React.FC = () => { })} > {isNewComment && editing ? ( - - - - - - {comment.user.username} - - - - -