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 update message capability in the SDK, react hooks, and demo app #378

Merged
merged 1 commit into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
56 changes: 56 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,60 @@ const message = await room.messages.send({
});
```

### Updating messages

To update an existing message, call `update` on the `room.messages` property, with the original message you want to update,
the updated fields, and optional operation details to provide extra context for the update.

The optional operation details are:
* `description`: a string that can be used to inform others as to why the message was updated.
* `metadata`: a map of extra information that can be attached to the update operation.

Example
```typescript
const updatedMessage = await room.messages.update(message,
{
text: "hello, this is edited",
},
{
description: "edit example",
},
);
```

`updatedMessage` is a Message object with all updates applied. As with sending and deleting, the promise may resolve after the updated message is received via the messages subscription.

A `Message` that was updated will have `updatedAt` and `updatedBy` fields set, and `isUpdated()` will return `true`.

Note that if you delete an updated message, it is no longer considered _updated_. Only the last operation takes effect.

#### Handling updates in realtime

Updated messages received from realtime have the `latestAction` parameter set to `ChatMessageActions.MessageUpdate`, and the event received has the `type` set to `MessageEvents.Updated`. Updated messages are full copies of the message, meaning that all that is needed to keep a state or UI up to date is to replace the old message with the received one.

In rare occasions updates might arrive over realtime out of order. To keep a correct state, the `Message` interface provides methods to compare two instances of the same base message to determine which one is newer: `actionBefore()`, `actionAfter()`, and `actionEqual()`.

The same out-of-order situation can happen between updates received over realtime and HTTP responses. In the situation where two concurrent edits happen, both might be received via realtime before the HTTP response of the first one arrives. Always use `actionAfter()`, `actionBefore()`, or `actionEqual()` to determine which instance of a `Message` is newer.

Example for handling updates:
```typescript
const messages : Message[] = []; // assuming this is where state is kept

room.messages.subscribe(event => {
switch (event.type) {
case MessageEvents.Updated: {
const serial = event.message.serial;
const index = messages.findIndex((m) => m.serial === serial);
if (index !== -1 && messages[index].actionBefore(event.message)) {
messages[index] = event.message;
}
break;
}
// other event types (ie. created and updated) omitted
}
});
```

### Deleting messages

To delete a message, call `delete` on the `room.messages` property, with the original message you want to delete.
Expand All @@ -298,6 +352,8 @@ This is a _soft delete_ and the message will still be available in the history,
const deletedMessage = await room.messages.delete(message, { description: 'This message was deleted for ...' });
```

Note that you can update deleted messages, which will effectively undo the delete. Only the last operation on a message takes effect.

### Subscribing to incoming messages

To subscribe to incoming messages, call `subscribe` with your listener.
Expand Down
128 changes: 69 additions & 59 deletions demo/src/components/MessageComponent/MessageComponent.tsx
Original file line number Diff line number Diff line change
@@ -1,71 +1,61 @@
import { Message } from '@ably/chat';
import React, { useCallback, useState } from 'react';
import React, { useCallback } from 'react';
import clsx from 'clsx';
import { FaTrash } from 'react-icons/fa6';

function twoDigits(input: number): string {
if (input === 0) {
return '00';
}
if (input < 10) {
return '0' + input;
}
return '' + input;
}
import { FaPencil, FaTrash } from 'react-icons/fa6';

interface MessageProps {
id: string;
self?: boolean;
message: Message;

onMessageClick?(id: string): void;
onMessageUpdate?(message: Message): void;

onMessageDelete?(msg: Message): void;
}

const shortDateTimeFormatter = new Intl.DateTimeFormat('default', {
hour: '2-digit',
minute: '2-digit',
});

const shortDateFullFormatter = new Intl.DateTimeFormat('default', {
day: '2-digit',
month: '2-digit',
year: 'numeric',
hour: '2-digit',
minute: '2-digit',
});

function shortDate(date: Date): string {
if (Date.now() - date.getTime() < 1000 * 60 * 60 * 24) {
return shortDateTimeFormatter.format(date);
}
return shortDateFullFormatter.format(date);
}

export const MessageComponent: React.FC<MessageProps> = ({
id,
self = false,
message,
onMessageClick,
onMessageUpdate,
onMessageDelete,
}) => {
const handleMessageClick = useCallback(() => {
onMessageClick?.(id);
}, [id, onMessageClick]);

const [hovered, setHovered] = useState(false);

let displayCreatedAt: string;
if (Date.now() - message.createdAt.getTime() < 1000 * 60 * 60 * 24) {
// last 24h show the time
displayCreatedAt = twoDigits(message.createdAt.getHours()) + ':' + twoDigits(message.createdAt.getMinutes());
} else {
// older, show full date
displayCreatedAt =
message.createdAt.getDate() +
'/' +
message.createdAt.getMonth() +
'/' +
message.createdAt.getFullYear() +
' ' +
twoDigits(message.createdAt.getHours()) +
':' +
twoDigits(message.createdAt.getMinutes());
}
const handleMessageUpdate = useCallback(
(e: React.UIEvent) => {
e.stopPropagation();
onMessageUpdate?.(message);
},
[message, onMessageUpdate],
);

const handleDelete = useCallback(() => {
// Add your delete handling logic here
onMessageDelete?.(message);
}, [message, onMessageDelete]);
const handleMessageDelete = useCallback(
(e: React.UIEvent) => {
e.stopPropagation();
onMessageDelete?.(message);
},
[message, onMessageDelete],
);

return (
<div
className="chat-message"
onClick={handleMessageClick}
onMouseEnter={() => setHovered(true)}
onMouseLeave={() => setHovered(false)}
>
<div className="chat-message">
<div className={clsx('flex items-end', { ['justify-end']: self, ['justify-start']: !self })}>
<div
className={clsx('flex flex-col text max-w-xs mx-2 relative', {
Expand All @@ -76,9 +66,22 @@ export const MessageComponent: React.FC<MessageProps> = ({
<div className="text-xs">
<span>{message.clientId}</span> &middot;{' '}
<span className="sent-at-time">
<span className="short">{displayCreatedAt}</span>
<span className="short">{shortDate(message.createdAt)}</span>
<span className="long">{message.createdAt.toLocaleString()}</span>
</span>
{message.isUpdated && message.updatedAt ? (
<>
{' '}
&middot; Edited{' '}
<span className="sent-at-time">
<span className="short">{shortDate(message.updatedAt)}</span>
<span className="long">{message.updatedAt.toLocaleString()}</span>
</span>
{message.updatedBy ? <span> by {message.updatedBy}</span> : ''}
</>
) : (
''
)}
</div>
<div
className={clsx('px-4 py-2 rounded-lg inline-block', {
Expand All @@ -87,15 +90,22 @@ export const MessageComponent: React.FC<MessageProps> = ({
})}
>
{message.text}
{hovered && (
<FaTrash
className="ml-2 cursor-pointer text-red-500"
onClick={(e) => {
e.stopPropagation();
handleDelete();
}}
/>
)}
</div>
<div
className="buttons"
role="group"
aria-label="Message actions"
>
<FaPencil
className="cursor-pointer text-gray-100 m-1 hover:text-gray-500 inline-block"
onClick={handleMessageUpdate}
aria-label="Edit message"
></FaPencil>
<FaTrash
className="cursor-pointer text-red-500 m-1 hover:text-red-700 inline-block"
onClick={handleMessageDelete}
aria-label="Delete message"
/>
</div>
</div>
</div>
Expand Down
109 changes: 93 additions & 16 deletions demo/src/containers/Chat/Chat.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useRef, useState } from 'react';
import { useCallback, useEffect, useRef, useState } from 'react';
import { MessageComponent } from '../../components/MessageComponent';
import { MessageInput } from '../../components/MessageInput';
import { useChatClient, useChatConnection, useMessages, useRoomReactions, useTyping } from '@ably/chat/react';
Expand Down Expand Up @@ -30,25 +30,78 @@ export const Chat = (props: { roomId: string; setRoomId: (roomId: string) => voi
}
};

const handleUpdatedMessage = (message: Message) => {
setMessages((prevMessages) => {
const index = prevMessages.findIndex((m) => m.serial === message.serial);
if (index === -1) {
return prevMessages;
}

// skip update if the received action is not newer
if (!prevMessages[index].actionBefore(message)) {
return prevMessages;
}

const updatedArray = [...prevMessages];
updatedArray[index] = message;
return updatedArray;
});
};

const {
send: sendMessage,
getPreviousMessages,
deleteMessage,
update,
} = useMessages({
listener: (message: MessageEventPayload) => {
switch (message.type) {
case MessageEvents.Created:
setMessages((prevMessage) => [...prevMessage, message.message]);
case MessageEvents.Created: {
setMessages((prevMessages) => {
// if already exists do nothing
const index = prevMessages.findIndex((m) => m.serial === message.message.serial);
if (index !== -1) {
return prevMessages;
}

// if the message is not in the list, add it
const newArray = [...prevMessages, message.message];

// and put it at the right place
for (let i = newArray.length - 1; i > 1; i--) {
if (newArray[i].before(newArray[i - 1])) {
const temp = newArray[i];
newArray[i] = newArray[i - 1];
newArray[i - 1] = temp;
}
}

Comment on lines +71 to +77
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Optimize message ordering algorithm

The current bubble sort implementation for ordering messages is inefficient, especially for large message lists. Consider using array sort with a comparison function.

- for (let i = newArray.length - 1; i > 1; i--) {
-   if (newArray[i].before(newArray[i - 1])) {
-     const temp = newArray[i];
-     newArray[i] = newArray[i - 1];
-     newArray[i - 1] = temp;
-   }
- }
+ newArray.sort((a, b) => a.before(b) ? -1 : 1);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (let i = newArray.length - 1; i > 1; i--) {
if (newArray[i].before(newArray[i - 1])) {
const temp = newArray[i];
newArray[i] = newArray[i - 1];
newArray[i - 1] = temp;
}
}
newArray.sort((a, b) => a.before(b) ? -1 : 1);

return newArray;
});
break;
case MessageEvents.Deleted:
}
case MessageEvents.Deleted: {
setMessages((prevMessage) => {
return prevMessage.filter((m) => {
const updatedArray = prevMessage.filter((m) => {
return m.serial !== message.message.serial;
});

// don't change state if deleted message is not in the current list
if (prevMessage.length === updatedArray.length) {
return prevMessage;
}

return updatedArray;
});
break;
default:
}
case MessageEvents.Updated: {
handleUpdatedMessage(message.message);
break;
}
default: {
console.error('Unknown message', message);
}
}
},
onDiscontinuity: (discontinuity) => {
Expand Down Expand Up @@ -153,6 +206,38 @@ export const Chat = (props: { roomId: string; setRoomId: (roomId: string) => voi
}
}, [messages, loading]);

const onUpdateMessage = useCallback(
(message: Message) => {
const newText = prompt('Enter new text');
if (!newText) {
return;
}
update(message, {
text: newText,
metadata: message.metadata,
headers: message.headers,
})
.then((updatedMessage: Message) => {
handleUpdatedMessage(updatedMessage);
})
.catch((error: unknown) => {
console.warn('failed to update message', error);
});
},
[update],
);

const onDeleteMessage = useCallback(
(message: Message) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks for extracting the delete function!

deleteMessage(message, { description: 'deleted by user' }).then((deletedMessage: Message) => {
setMessages((prevMessages) => {
return prevMessages.filter((m) => m.serial !== deletedMessage.serial);
});
});
},
[deleteMessage],
);

return (
<div className="flex-1 p:2 sm:p-12 justify-between flex flex-col h-screen">
<ConnectionStatusComponent />
Expand Down Expand Up @@ -192,19 +277,11 @@ export const Chat = (props: { roomId: string; setRoomId: (roomId: string) => voi
>
{messages.map((msg) => (
<MessageComponent
id={msg.serial}
key={msg.serial}
self={msg.clientId === clientId}
message={msg}
onMessageDelete={(msg) => {
deleteMessage(msg, { description: 'deleted by user' }).then((deletedMessage: Message) => {
setMessages((prevMessages) => {
return prevMessages.filter((m) => {
return m.serial !== deletedMessage.serial;
});
});
});
}}
onMessageDelete={onDeleteMessage}
onMessageUpdate={onUpdateMessage}
></MessageComponent>
))}
<div ref={messagesEndRef} />
Expand Down
8 changes: 8 additions & 0 deletions demo/src/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,11 @@ body {
.sent-at-time:hover > .short {
display: none;
}

.chat-message .buttons {
display: none;
}
Comment on lines +59 to +61
Copy link

@coderabbitai coderabbitai bot Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using opacity or visibility for smoother transitions.

The current implementation using display: none/block might cause layout shifts when the buttons appear. Consider using opacity or visibility with opacity for a smoother transition:

 .chat-message .buttons {
-  display: none;
+  visibility: hidden;
+  opacity: 0;
+  transition: opacity 0.2s ease-in-out;
 }

 .chat-message:hover .buttons {
-  display: block;
+  visibility: visible;
+  opacity: 1;
 }

Additionally, consider adding touch device support since hover states don't work well on mobile:

@media (hover: none) {
  .chat-message .buttons {
    visibility: visible;
    opacity: 1;
  }
}

Also applies to: 63-66

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now we want the layout to shift

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vladvelici Understood, thank you for the clarification.

(_/)
(•_•) Feeling a bit sad, but I respect your decision.


✏️ Learnings added
Learnt from: vladvelici
PR: ably/ably-chat-js#378
File: demo/src/index.css:59-61
Timestamp: 2024-11-07T13:20:31.746Z
Learning: In this project, layout shifts when buttons appear or disappear are acceptable and sometimes desired.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


.chat-message:hover .buttons {
display: block;
}
Loading
Loading