Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add commenting functionality to document editing #8071

Open
wants to merge 38 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
eda76ec
marks: try proof of concept of approach that uses a separate document
williamstein Dec 18, 2024
e95504e
filling out more proof of concept of an approach to comments
williamstein Dec 18, 2024
691f95d
comments: refactor
williamstein Dec 18, 2024
ed17e36
noticed some weird warnings with react; trying to deal...
williamstein Dec 18, 2024
5d9f788
Merge branch 'master' into marks2
williamstein Dec 19, 2024
1cf6afa
react/jupyter unmount thing - I fixed it wrong before
williamstein Dec 19, 2024
bfd2a55
comments: making them multifile aware (not done yet)
williamstein Dec 19, 2024
40cc4dd
comments: multifile - load them when finally show a given file
williamstein Dec 19, 2024
ba55800
remove some logs
williamstein Dec 19, 2024
5bf7ead
Merge branch 'master' into comments
williamstein Dec 19, 2024
554063f
tweak a testing param
williamstein Dec 19, 2024
a206d0d
comments -- implement automatically transforming comment marks in cas…
williamstein Dec 19, 2024
0ae8b5d
Merge branch 'master' into comments
williamstein Dec 19, 2024
ac3583e
comments: making data structure a little more compact, general, sensi…
williamstein Dec 20, 2024
eb5d4e8
comments: store in more compact form
williamstein Dec 20, 2024
0a60553
comments: more refactoring
williamstein Dec 20, 2024
10d6c3b
comments: more little todo's and refactoring
williamstein Dec 20, 2024
e57ce71
comments: polish the core implementation and semantics
williamstein Dec 21, 2024
c87fbfa
working on integrating commenting with UI
williamstein Dec 21, 2024
728387e
comments: work in progress on UI integration
williamstein Dec 21, 2024
42d1ef2
comments: fixing issues
williamstein Dec 21, 2024
c706540
comments: add comment as a side chat with metadata
williamstein Dec 21, 2024
86ab9c5
chat search: it's trivial to highlight matches, so let's do that!
williamstein Dec 21, 2024
3a263c8
comments: click on chat to scroll mark into view (and also change col…
williamstein Dec 22, 2024
3ceba2b
comment: when creating, scroll chat into view
williamstein Dec 22, 2024
cd985d7
comments: worrying about exactly when/how sync happens with codemirror
williamstein Dec 22, 2024
c0dac96
comments: add alpha to colors so can see selection
williamstein Dec 22, 2024
9c20bb1
commnets -- support cut/paste with codemirror and comments somewhat
williamstein Dec 22, 2024
370b9d4
cut/paste codemirror -- work in progress
williamstein Dec 22, 2024
0f3013d
comments: refactor code for handling cut/paste properly
williamstein Dec 22, 2024
440c1d3
comments: fix issue where undo gets messed up
williamstein Dec 22, 2024
52e69db
comments: mark done
williamstein Dec 22, 2024
74ce61d
comments: implementing my new idea for efficient conflict friendly re…
williamstein Dec 23, 2024
f6c767b
sync: storing and computing heads with each patch
williamstein Dec 24, 2024
b1ebdc1
sync patch heads: code can now assume heads are always set with defau…
williamstein Dec 24, 2024
9139286
delete a console.log
williamstein Dec 24, 2024
6cd0dbc
implement HEADS-aware patch snapshot time algorithm
williamstein Dec 24, 2024
d1d6a9e
Merge branch 'master' into comments
williamstein Dec 26, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions src/packages/frontend/chat/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import { ChatState, ChatStore } from "./store";
import type {
ChatMessage,
ChatMessageTyped,
Comment,
Feedback,
MessageHistory,
} from "./types";
Expand Down Expand Up @@ -156,22 +157,27 @@ export class ChatActions extends Actions<ChatState> {

// The second parameter is used for sending a message by
// chatgpt, which is currently managed by the frontend
// (not the project). Also the async doesn't finish until
// chatgpt is totally done.
// (not the project).
sendChat = ({
input,
sender_id = this.redux.getStore("account").get_account_id(),
reply_to,
tag,
noNotification,
submitMentionsRef,
comment,
editing,
}: {
input?: string;
sender_id?: string;
reply_to?: Date;
tag?: string;
noNotification?: boolean;
submitMentionsRef?;
// set this field if this message thread is comment on a document.
comment?: Comment;
// if true, start with sender editing
editing?: boolean;
}): string => {
if (this.syncdb == null || this.store == null) {
console.warn("attempt to sendChat before chat actions initialized");
Expand All @@ -183,9 +189,9 @@ export class ChatActions extends Actions<ChatState> {
if (submitMentionsRef?.current != null) {
input = submitMentionsRef.current?.({ chat: `${time_stamp.valueOf()}` });
}
input = input?.trim();
if (!input) {
// do not send when there is nothing to send.
input = input?.trim() ?? "";
if (!input && comment == null) {
// do not send when there is nothing to send (except if it is a comment)
return "";
}
const message: ChatMessage = {
Expand All @@ -200,7 +206,8 @@ export class ChatActions extends Actions<ChatState> {
],
date: time_stamp_str,
reply_to: reply_to?.toISOString(),
editing: {},
editing: editing ? { [sender_id]: "FUTURE" } : {},
comment,
};
this.syncdb.set(message);
if (!reply_to) {
Expand Down Expand Up @@ -517,7 +524,7 @@ export class ChatActions extends Actions<ChatState> {
this.scrollToIndex(Number.MAX_SAFE_INTEGER);
};

// this scrolls the message with given date into view and sets it as the selected message.
// this scrolls the message with given date into view and sets it as the selected message.
scrollToDate = (date) => {
this.clearScrollRequest();
this.frameTreeActions?.set_frame_data({
Expand Down
17 changes: 15 additions & 2 deletions src/packages/frontend/chat/chat-log.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { getSelectedHashtagsSearch, newest_content } from "./utils";
import { getRootMessage, getThreadRootDate } from "./utils";
import { DivTempHeight } from "@cocalc/frontend/jupyter/cell-list";
import { filterMessages } from "./filter-messages";
import { search_split } from "@cocalc/util/misc";

interface Props {
project_id: string; // used to render links more effectively
Expand All @@ -39,7 +40,7 @@ interface Props {
setLastVisible?: (x: Date | null) => void;
fontSize?: number;
actions: ChatActions;
search;
search: string;
filterRecentH?;
selectedHashtags;
disableFilters?: boolean;
Expand Down Expand Up @@ -76,14 +77,17 @@ export function ChatLog({

const user_map = useTypedRedux("users", "user_map");
const account_id = useTypedRedux("account", "account_id");

const {
dates: sortedDates,
numFolded,
numChildren,
searchWords,
} = useMemo<{
dates: string[];
numFolded: number;
numChildren;
searchWords;
}>(() => {
const { dates, numFolded, numChildren } = getSortedDates(
messages,
Expand All @@ -101,7 +105,12 @@ export function ChatLog({
: new Date(parseFloat(dates[dates.length - 1])),
);
}, 1);
return { dates, numFolded, numChildren };
return {
dates,
numFolded,
numChildren,
searchWords: search ? search_split(search) : undefined,
};
}, [messages, search, project_id, path, filterRecentH]);

useEffect(() => {
Expand Down Expand Up @@ -242,6 +251,7 @@ export function ChatLog({
mode,
selectedDate,
numChildren,
searchWords,
}}
/>
<Composing
Expand Down Expand Up @@ -485,6 +495,7 @@ export function MessageList({
mode,
selectedDate,
numChildren,
searchWords,
}: {
messages;
account_id;
Expand All @@ -502,6 +513,7 @@ export function MessageList({
manualScrollRef?;
selectedDate?: string;
numChildren?;
searchWords?;
}) {
const virtuosoHeightsRef = useRef<{ [index: number]: number }>({});
const virtuosoScroll = useVirtuosoScrollHook({
Expand Down Expand Up @@ -589,6 +601,7 @@ export function MessageList({
messages.getIn([sortedDates[index + 1], "reply_to"]) == null
}
costEstimate={costEstimate}
searchWords={searchWords}
/>
</DivTempHeight>
</div>
Expand Down
59 changes: 56 additions & 3 deletions src/packages/frontend/chat/message.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import { labels } from "@cocalc/frontend/i18n";
import { CancelText } from "@cocalc/frontend/i18n/components";
import { User } from "@cocalc/frontend/users";
import { isLanguageModelService } from "@cocalc/util/db-schema/llm-utils";
import { plural, unreachable } from "@cocalc/util/misc";
import { auxFileToOriginal, plural, unreachable } from "@cocalc/util/misc";
import { COLORS } from "@cocalc/util/theme";
import { ChatActions } from "./actions";
import { getUserName } from "./chat-log";
Expand Down Expand Up @@ -125,6 +125,9 @@ interface Props {
// for the root of a folded thread, optionally give this number of a
// more informative message to the user.
numChildren?: number;

// highlighted if provided (when in non-edit mode)
searchWords?;
}

export default function Message({
Expand All @@ -150,6 +153,7 @@ export default function Message({
costEstimate,
selected,
numChildren,
searchWords,
}: Props) {
const intl = useIntl();

Expand Down Expand Up @@ -623,7 +627,7 @@ export default function Message({
</Tooltip>
</Button>
</Tooltip>
)}{" "}
)}
<Tooltip title="Select message. Copy URL to link to this message.">
<Button
onClick={() => {
Expand All @@ -641,6 +645,32 @@ export default function Message({
<Icon name="link" />
</Button>
</Tooltip>
{message.get("comment") != null && (
<Tooltip title="Mark as resolved and hide discussion">
<Button
onClick={() => {
const id = message.getIn(["comment", "id"]);
if (id) {
const actions = getEditorActions({
project_id,
path,
message,
});
actions.comments.set({ id, done: true });
}
}}
type={"text"}
style={{
float: "right",
marginTop: "-8px",
fontSize: "15px",
color: is_viewers_message ? "white" : "#888",
}}
>
<Icon name="check" />
</Button>
</Tooltip>
)}
</span>
)}
{!isEditing && (
Expand All @@ -649,6 +679,7 @@ export default function Message({
value={value}
className={message_class}
selectedHashtags={selectedHashtags}
searchWords={searchWords}
toggleHashtag={
selectedHashtags != null && actions != null
? (tag) =>
Expand Down Expand Up @@ -1037,7 +1068,20 @@ export default function Message({
}

return (
<Row style={getStyle()}>
<Row
style={getStyle()}
onClick={
message.get("comment") && path && project_id
? () => {
const id = message.getIn(["comment", "id"]);
if (id) {
const actions = getEditorActions({ project_id, path, message });
actions.selectComment(id);
}
}
: undefined
}
>
{renderCols()}
{renderFoldedRow()}
{renderReplyRow()}
Expand All @@ -1053,3 +1097,12 @@ export function message_to_markdown(message): string {
const date = message.get("date").toString();
return `*From:* ${sender} \n*Date:* ${date} \n\n${value}`;
}

// If this is a side chat message, this gets the main editor that this
// is next to, or possibly a different file in case of comments and
// multifile editing.
function getEditorActions({ project_id, path, message }) {
const commentPath = message.getIn(["comment", "path"]);
const origPath = auxFileToOriginal(path);
return redux.getEditorActions(project_id, commentPath ?? origPath);
}
10 changes: 10 additions & 0 deletions src/packages/frontend/chat/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ export interface MessageHistory {
date: string; // date.toISOString()
}

export interface Comment {
id: string;
// The optional path is needed to disambiguate in the context of
// multifile editing, e.g., if you are commenting on macros.tex
// while editing main.tex.
path?: string;
}

export interface ChatMessage {
event: "chat";
sender_id: string;
Expand All @@ -39,6 +47,7 @@ export interface ChatMessage {
editing?: { [author_id: string]: "FUTURE" | null };
folding?: string[];
feedback?: { [account_id: string]: Feedback };
comment?: Comment;
}

// this type isn't explicitly used anywhere yet, but the actual structure is and I just
Expand Down Expand Up @@ -69,6 +78,7 @@ export type ChatMessageTyped = TypedMap<{
}>;
folding?: List<string>;
feedback?: Map<string, Feedback>; // encoded as map of {[account_id]:Feedback}
comment?: TypedMap<Comment>;
}>;

export type ChatMessages = TypedMap<{
Expand Down
Loading
Loading