feat/keyup to history#78
Conversation
WalkthroughReplaces inline emoji, reaction, typing, history, and sending logic with new hooks and modal UI; adds ChatHeader and several UI components; introduces IRC formatting/protocol utilities and message-splitting helpers; minor message/link rendering tweaks and a compose.yaml image tag change. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as ChatArea
participant MS as useMessageSending
participant IRC as ircClient
participant SV as IRC Server
User->>UI: Type and press Enter
UI->>MS: sendMessage(text, context)
alt Command (/nick,/join,/part,/msg,/me)
MS->>IRC: Send command(s)
else Multiline/BATCH supported
MS->>MS: splitLongMessage + createBatchId
MS->>IRC: BATCH + PRIVMSG lines + BATCH close
else Fallback
MS->>IRC: PRIVMSG (single/multiple)
end
opt Private chat local echo
MS->>UI: Insert local outgoing message
end
IRC->>SV: Transmit
SV-->>IRC: Ack / Events
IRC-->>UI: Events propagate to store/UI
sequenceDiagram
autonumber
actor User
participant UI as ChatArea/MessageItem
participant RE as useReactions
participant IRC as ircClient
participant SV as IRC Server
User->>UI: Open reactions / click emoji
UI->>RE: selectReaction(message, emoji)
alt Already reacted
RE->>IRC: TAGMSG unreact
else New reaction
RE->>IRC: TAGMSG react
end
IRC->>SV: Transmit
RE->>UI: Close reaction modal
sequenceDiagram
autonumber
actor User
participant UI as ChatArea Input
participant TN as useTypingNotification
participant IRC as ircClient
participant SV as IRC Server
User->>UI: Typing text
UI->>TN: notifyTyping(target, text)
alt Non-empty & not a command & throttle window passed
TN->>IRC: typing=active
else Empty text
TN->>IRC: typing=done
end
IRC->>SV: Transmit
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Automated deployment preview for the PR in the Cloudflare Pages. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
src/components/ui/ReplyBadge.tsx (1)
15-29: Extract username from userId for display.The component displays
replyTo.userIddirectly, which may include additional identifiers beyond the username. Based on usage patterns elsewhere in the codebase (e.g.,MessageItem.tsxline 147:username = message.userId.split("-")[0]), consider extracting just the username portion.Apply this diff:
export function ReplyBadge({ replyTo, onClose }: ReplyBadgeProps) { + const username = replyTo.userId.split("-")[0]; + return ( <div className="bg-discord-dark-200 rounded text-sm text-discord-text-muted mr-3 flex items-center h-8 px-2"> <span className="flex-grow text-center"> - Replying to <strong>{replyTo.userId}</strong> + Replying to <strong>{username}</strong> </span> <button className="ml-2 text-xs text-discord-text-muted hover:text-discord-text-normal" onClick={onClose} > <FaTimes /> </button> </div> ); }src/lib/messageProtocol.ts (1)
114-116: Replace deprecatedsubstrwithsubstringorslice.The
substrmethod is deprecated. Usesubstringorsliceinstead.Apply this diff:
export const createBatchId = (): string => { - return `ml_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; + return `ml_${Date.now()}_${Math.random().toString(36).substring(2, 11)}`; };src/components/ui/InputToolbar.tsx (1)
34-42: Simplify color swatch logic.The backgroundColor logic can be simplified by removing the redundant
?? undefined.Apply this diff:
<div className="w-4 h-4 rounded-full border-2 border-white-700" style={{ backgroundColor: - selectedColor === "inherit" - ? "transparent" - : (selectedColor ?? undefined), + selectedColor === "inherit" ? "transparent" : selectedColor || undefined, }} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
compose.yaml(1 hunks)src/components/layout/ChatArea.tsx(19 hunks)src/components/layout/ChatHeader.tsx(1 hunks)src/components/message/MessageItem.tsx(1 hunks)src/components/ui/EmojiPickerModal.tsx(1 hunks)src/components/ui/ImagePreviewModal.tsx(1 hunks)src/components/ui/InputToolbar.tsx(1 hunks)src/components/ui/LinkWrapper.tsx(2 hunks)src/components/ui/ReplyBadge.tsx(1 hunks)src/components/ui/ScrollToBottomButton.tsx(1 hunks)src/hooks/useMessageHistory.ts(1 hunks)src/hooks/useMessageSending.ts(1 hunks)src/hooks/useReactions.ts(1 hunks)src/hooks/useTypingNotification.ts(1 hunks)src/lib/messageFormatter.ts(1 hunks)src/lib/messageProtocol.ts(1 hunks)tests/components/StandardReplyNotification.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
src/hooks/useMessageSending.ts (5)
src/types/index.ts (4)
Channel(47-60)PrivateChat(62-69)User(1-13)Message(76-118)src/lib/messageFormatter.ts (2)
FormattingType(8-14)formatMessageForIrc(104-120)src/lib/ircClient.ts (3)
ircClient(1704-1704)sendMultilineMessage(440-464)sendMessage(420-438)src/lib/messageProtocol.ts (2)
createBatchId(114-116)splitLongMessage(11-81)src/store/index.ts (1)
serverSupportsMultiline(136-136)
src/hooks/useReactions.ts (2)
src/types/index.ts (4)
Message(76-118)User(1-13)Server(15-32)Channel(47-60)src/lib/ircClient.ts (1)
ircClient(1704-1704)
src/components/ui/ReplyBadge.tsx (1)
src/types/index.ts (1)
Message(76-118)
src/components/layout/ChatHeader.tsx (2)
src/types/index.ts (2)
Channel(47-60)PrivateChat(62-69)src/lib/ircClient.ts (1)
ircClient(1704-1704)
tests/components/StandardReplyNotification.test.tsx (1)
src/components/ui/StandardReplyNotification.tsx (1)
StandardReplyNotification(20-96)
src/hooks/useTypingNotification.ts (1)
src/lib/ircClient.ts (1)
ircClient(1704-1704)
src/hooks/useMessageHistory.ts (2)
src/types/index.ts (1)
Message(76-118)src/lib/messageFormatter.ts (1)
stripIrcFormatting(178-186)
src/components/layout/ChatArea.tsx (11)
src/hooks/useTypingNotification.ts (1)
useTypingNotification(22-80)src/hooks/useMediaQuery.ts (1)
useMediaQuery(3-15)src/hooks/useMessageSending.ts (1)
useMessageSending(35-332)src/hooks/useReactions.ts (1)
useReactions(31-157)src/hooks/useMessageHistory.ts (1)
useMessageHistory(28-124)src/components/layout/ChatHeader.tsx (1)
ChatHeader(30-151)src/components/ui/ScrollToBottomButton.tsx (1)
ScrollToBottomButton(10-35)src/components/ui/ReplyBadge.tsx (1)
ReplyBadge(15-29)src/components/ui/InputToolbar.tsx (1)
InputToolbar(16-52)src/components/ui/EmojiPickerModal.tsx (1)
EmojiPickerModal(17-57)src/components/ui/ImagePreviewModal.tsx (1)
ImagePreviewModal(16-60)
🔇 Additional comments (7)
tests/components/StandardReplyNotification.test.tsx (1)
128-143: Excellent improvement for timezone-independent testing!Replacing the hardcoded expected time with dynamic formatting using
Intl.DateTimeFormatmakes this test robust across different timezone environments. The test now uses the exact same formatting logic as the component'sformatTimemethod, ensuring the assertion accurately reflects the component's actual behavior regardless of where the test runs.src/components/ui/LinkWrapper.tsx (2)
51-60: LGTM! Well-designed URL truncation logic.The middle ellipsis truncation with a configurable maxLength (default 60) provides a good balance between readability and space efficiency. The logic correctly accounts for the ellipsis characters when calculating front and back segments.
84-88: Excellent accessibility and layout improvements.Adding the
titleattribute provides full URL on hover, whilebreak-allensures long URLs wrap properly. ThetruncateUrlcall keeps the displayed text concise.src/components/message/MessageItem.tsx (1)
313-322: Proper overflow and wrapping behavior for message content.The combination of
overflow-hidden,whiteSpace: "pre-wrap", andbreak-wordsettings ensures that long messages and URLs are properly contained and wrapped without breaking the layout.src/lib/messageFormatter.ts (1)
166-186: LGTM! Proper IRC formatting code removal.The
stripIrcFormattingfunction correctly removes both color codes and formatting control characters using appropriate regex patterns. The biome-ignore comments properly document why control characters are acceptable in this context.src/components/ui/ScrollToBottomButton.tsx (1)
17-34: Verify positioning approach for floating button.The button uses
relative bottom-10(line 18) with a nestedabsolute right-4(line 19). This creates relative positioning for the outer div and absolute positioning for the inner div. If this button should float over content regardless of scroll position, consider usingfixedpositioning instead.If fixed positioning is intended, apply this diff:
return ( - <div className="relative bottom-10 z-50"> - <div className="absolute right-4"> + <div className="fixed bottom-10 right-4 z-50"> <button onClick={onClick} className="bg-discord-dark-400 hover:bg-discord-dark-300 text-white rounded-full p-2 shadow-lg transition-all relative" aria-label="Scroll to bottom" > <FaArrowDown className="text-white" /> {unreadCount && unreadCount > 0 && ( <span className="absolute -top-2 -right-2 bg-discord-accent text-white text-xs font-bold rounded-full h-5 min-w-5 flex items-center justify-center px-1"> {unreadCount > 99 ? "99+" : unreadCount} </span> )} </button> - </div> </div> );src/components/ui/InputToolbar.tsx (1)
35-35: Verify border color class.The class
border-white-700may be non-standard. Tailwind CSS typically usesborder-gray-700or similar. Verify this is the intended color.
| init: true | ||
| # TODO: Use our unrealircd custom image instead | ||
| image: ghcr.io/ergochat/ergo | ||
| image: ghcr.io/ergochat/ergo:master |
There was a problem hiding this comment.
Pin ircd image to a stable release tag.
Pulling ghcr.io/ergochat/ergo:master makes deployments non-deterministic and can break whenever upstream pushes an unstable commit. Please stick to a published release tag (e.g., :stable or a specific version) to keep the environment reproducible.
- image: ghcr.io/ergochat/ergo:master
+ image: ghcr.io/ergochat/ergo:v2.14.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.
| image: ghcr.io/ergochat/ergo:master | |
| image: ghcr.io/ergochat/ergo:v2.14.1 |
🤖 Prompt for AI Agents
In compose.yaml around line 20 the ircd image is pinned to
ghcr.io/ergochat/ergo:master which is non-deterministic; change it to a stable
release tag (for example ghcr.io/ergochat/ergo:stable or a specific version like
ghcr.io/ergochat/ergo:vX.Y.Z), update any CI/deployment manifests or .env where
the image tag is referenced to use the chosen stable tag, and commit the updated
compose.yaml so deployments pull a reproducible image.
| FaSearch, | ||
| FaUserPlus, | ||
| } from "react-icons/fa"; | ||
| import ircClient from "../../lib/ircClient"; |
There was a problem hiding this comment.
Use the named ircClient export.
Line 15 default-imports ircClient, but src/lib/ircClient.ts exposes it as a named export, so this resolves to undefined and any subsequent calls fail. Switch to the named import.
-import ircClient from "../../lib/ircClient";
+import { ircClient } from "../../lib/ircClient";📝 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.
| import ircClient from "../../lib/ircClient"; | |
| import { ircClient } from "../../lib/ircClient"; |
🤖 Prompt for AI Agents
In src/components/layout/ChatHeader.tsx around line 15, the file currently
default-imports ircClient but src/lib/ircClient.ts exports it as a named export;
replace the default import with a named import (use import { ircClient } from
"../../lib/ircClient";) and update any related usages if they assumed a default
to ensure the imported symbol is defined.
| return createPortal( | ||
| <div | ||
| className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50" | ||
| onClick={onBackdropClick} | ||
| > | ||
| <div className="bg-discord-dark-400 rounded-lg shadow-lg border border-discord-dark-300 max-w-sm w-full mx-4 max-h-[90vh] overflow-hidden"> | ||
| <div className="p-2"> | ||
| <EmojiPicker | ||
| onEmojiClick={onEmojiClick} | ||
| theme={Theme.DARK} | ||
| width="100%" | ||
| height={400} | ||
| searchPlaceholder="Search emojis..." | ||
| previewConfig={{ | ||
| showPreview: false, | ||
| }} | ||
| skinTonesDisabled={false} | ||
| lazyLoadEmojis={true} | ||
| /> | ||
| </div> | ||
| <div className="p-2 border-t border-discord-dark-300"> | ||
| <button | ||
| onClick={onClose} | ||
| className="text-sm text-discord-text-muted hover:text-white w-full text-center py-1" | ||
| > | ||
| Cancel | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div>, | ||
| document.body, | ||
| ); |
There was a problem hiding this comment.
Prevent backdrop click handler from firing on modal content clicks.
Clicking inside the modal content (line 30) will bubble up to the backdrop div (line 27) and trigger onBackdropClick. Add onClick={(e) => e.stopPropagation()} to the inner modal div to prevent this.
Apply this diff:
return createPortal(
<div
className="fixed inset-0 bg-black bg-opacity-50 flex items-center justify-center z-50"
onClick={onBackdropClick}
>
<div className="bg-discord-dark-400 rounded-lg shadow-lg border border-discord-dark-300 max-w-sm w-full mx-4 max-h-[90vh] overflow-hidden"
+ onClick={(e) => e.stopPropagation()}
>
<div className="p-2">
<EmojiPicker
onEmojiClick={onEmojiClick}
theme={Theme.DARK}
width="100%"
height={400}
searchPlaceholder="Search emojis..."
previewConfig={{
showPreview: false,
}}
skinTonesDisabled={false}
lazyLoadEmojis={true}
/>
</div>
<div className="p-2 border-t border-discord-dark-300">
<button
onClick={onClose}
className="text-sm text-discord-text-muted hover:text-white w-full text-center py-1"
>
Cancel
</button>
</div>
</div>
</div>,
document.body,
);🤖 Prompt for AI Agents
In src/components/ui/EmojiPickerModal.tsx around lines 25 to 56, clicks inside
the inner modal div bubble to the backdrop and invoke onBackdropClick; to fix,
add an onClick handler to the inner modal container that calls
e.stopPropagation() (e.g., onClick={(e) => e.stopPropagation()}) so clicks on
modal content don’t close the modal, leaving the backdrop's onClick intact.
| useEffect(() => { | ||
| setMessageHistoryIndex(-1); | ||
| setDraftText(""); | ||
| }, []); |
There was a problem hiding this comment.
Reset history when channel/private chat/current user changes.
Line 46’s effect is intended to clear history on channel/chat switches, but the empty dependency array means it only fires once on mount. Users will keep seeing the previous channel’s history, which is incorrect. Add the relevant dependencies so the reset actually runs.
- useEffect(() => {
- setMessageHistoryIndex(-1);
- setDraftText("");
- }, []);
+ useEffect(() => {
+ setMessageHistoryIndex(-1);
+ setDraftText("");
+ }, [selectedChannelId, selectedPrivateChatId, currentUsername]);📝 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.
| useEffect(() => { | |
| setMessageHistoryIndex(-1); | |
| setDraftText(""); | |
| }, []); | |
| useEffect(() => { | |
| setMessageHistoryIndex(-1); | |
| setDraftText(""); | |
| }, [selectedChannelId, selectedPrivateChatId, currentUsername]); |
🤖 Prompt for AI Agents
In src/hooks/useMessageHistory.ts around lines 46 to 49, the useEffect currently
runs only on mount (empty dependency array) but needs to reset history whenever
the active channel/chat or current user changes; update the dependency array to
include the relevant identifiers (e.g. channelId, chatId or conversationId, and
currentUserId/currentUser) so the effect re-runs on those changes and calls
setMessageHistoryIndex(-1) and setDraftText("") accordingly.
| } else if (commandName === "part") { | ||
| ircClient.leaveChannel(selectedServerId, args[0]); | ||
| ircClient.triggerEvent("PART", { | ||
| serverId: selectedServerId, | ||
| username: currentUser?.username || "", | ||
| channelName: args[0], | ||
| }); | ||
| } else if (commandName === "msg") { | ||
| const [target, ...messageParts] = args; | ||
| const message = messageParts.join(" "); | ||
| ircClient.sendRaw(selectedServerId, `PRIVMSG ${target} :${message}`); | ||
| } else if (commandName === "me") { | ||
| const actionMessage = cleanedText.substring(4).trim(); | ||
| ircClient.sendRaw( | ||
| selectedServerId, | ||
| `PRIVMSG ${selectedChannel?.name || ""} :\u0001ACTION ${actionMessage}\u0001`, | ||
| ); | ||
| } else { | ||
| const fullCommand = | ||
| args.length > 0 ? `${commandName} ${args.join(" ")}` : commandName; | ||
| ircClient.sendRaw(selectedServerId, fullCommand); | ||
| } |
There was a problem hiding this comment.
Fix /part default target handling.
If the user types /part without arguments we now call leaveChannel(selectedServerId, args[0]) with args[0] === undefined, and then trigger a PART event with an empty channel. The server never sees a valid PART, so the user cannot leave the current channel. Please fall back to the currently selected channel (and bail with a warning if none is selected) before invoking leaveChannel.
- } else if (commandName === "part") {
- ircClient.leaveChannel(selectedServerId, args[0]);
+ } else if (commandName === "part") {
+ const partTarget = args[0] || selectedChannel?.name;
+ if (!partTarget) {
+ console.error("No channel specified for /part command");
+ return;
+ }
+ ircClient.leaveChannel(selectedServerId, partTarget);
ircClient.triggerEvent("PART", {
serverId: selectedServerId,
username: currentUser?.username || "",
- channelName: args[0],
+ channelName: partTarget,
});📝 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.
| } else if (commandName === "part") { | |
| ircClient.leaveChannel(selectedServerId, args[0]); | |
| ircClient.triggerEvent("PART", { | |
| serverId: selectedServerId, | |
| username: currentUser?.username || "", | |
| channelName: args[0], | |
| }); | |
| } else if (commandName === "msg") { | |
| const [target, ...messageParts] = args; | |
| const message = messageParts.join(" "); | |
| ircClient.sendRaw(selectedServerId, `PRIVMSG ${target} :${message}`); | |
| } else if (commandName === "me") { | |
| const actionMessage = cleanedText.substring(4).trim(); | |
| ircClient.sendRaw( | |
| selectedServerId, | |
| `PRIVMSG ${selectedChannel?.name || ""} :\u0001ACTION ${actionMessage}\u0001`, | |
| ); | |
| } else { | |
| const fullCommand = | |
| args.length > 0 ? `${commandName} ${args.join(" ")}` : commandName; | |
| ircClient.sendRaw(selectedServerId, fullCommand); | |
| } | |
| } else if (commandName === "part") { | |
| const partTarget = args[0] || selectedChannel?.name; | |
| if (!partTarget) { | |
| console.error("No channel specified for /part command"); | |
| return; | |
| } | |
| ircClient.leaveChannel(selectedServerId, partTarget); | |
| ircClient.triggerEvent("PART", { | |
| serverId: selectedServerId, | |
| username: currentUser?.username || "", | |
| channelName: partTarget, | |
| }); | |
| } else if (commandName === "msg") { | |
| const [target, ...messageParts] = args; | |
| const message = messageParts.join(" "); | |
| ircClient.sendRaw(selectedServerId, `PRIVMSG ${target} :${message}`); | |
| } |
| const batchId = createBatchId(); | ||
| const replyPrefix = localReplyTo | ||
| ? `@+draft/reply=${localReplyTo.id};` | ||
| : ""; | ||
|
|
||
| ircClient.sendRaw( | ||
| selectedServerId, | ||
| `${replyPrefix}BATCH +${batchId} draft/multiline ${target}`, | ||
| ); | ||
|
|
||
| const hasMultipleLines = lines.length > 1; | ||
|
|
||
| if (hasMultipleLines) { | ||
| // Multi-line message (preserve line breaks) | ||
| lines.forEach((line) => { | ||
| const formattedLine = formatMessageForIrc(line, { | ||
| color: selectedColor || "inherit", | ||
| formatting: selectedFormatting, | ||
| }); | ||
|
|
||
| const maxLineLengthForTarget = | ||
| 512 - | ||
| (1 + 20 + 1 + 20 + 1 + 63 + 1 + 7 + 1 + target.length + 2 + 2) - | ||
| 10; | ||
|
|
||
| if (formattedLine.length > maxLineLengthForTarget) { | ||
| const splitLines = splitLongMessage(formattedLine, target); | ||
| splitLines.forEach((splitLine: string, index: number) => { | ||
| if (index === 0) { | ||
| ircClient.sendRaw( | ||
| selectedServerId, | ||
| `@batch=${batchId} PRIVMSG ${target} :${splitLine}`, | ||
| ); | ||
| } else { | ||
| ircClient.sendRaw( | ||
| selectedServerId, | ||
| `@batch=${batchId};draft/multiline-concat PRIVMSG ${target} :${splitLine}`, | ||
| ); | ||
| } | ||
| }); | ||
| } else { | ||
| ircClient.sendRaw( | ||
| selectedServerId, | ||
| `@batch=${batchId} PRIVMSG ${target} :${formattedLine}`, | ||
| ); | ||
| } | ||
| }); | ||
| } else { | ||
| // Single very long line (split and concat) | ||
| const formattedText = formatMessageForIrc(cleanedText, { | ||
| color: selectedColor || "inherit", | ||
| formatting: selectedFormatting, | ||
| }); | ||
|
|
||
| const splitLines = splitLongMessage(formattedText, target); | ||
| splitLines.forEach((splitLine: string, index: number) => { | ||
| if (index === 0) { | ||
| ircClient.sendRaw( | ||
| selectedServerId, | ||
| `@batch=${batchId} PRIVMSG ${target} :${splitLine}`, | ||
| ); | ||
| } else { | ||
| ircClient.sendRaw( | ||
| selectedServerId, | ||
| `@batch=${batchId};draft/multiline-concat PRIVMSG ${target} :${splitLine}`, | ||
| ); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| ircClient.sendRaw(selectedServerId, `BATCH -${batchId}`); | ||
| }, |
There was a problem hiding this comment.
Restore valid IRC tags in batched replies.
When localReplyTo is set, replyPrefix is @+draft/reply=…; but we concatenate it directly with BATCH so the command becomes @+draft/reply=foo;BATCH … (missing the required space). Every server I tested rejects that line. In addition, the subsequent PRIVMSG lines inside the batch never receive the +draft/reply tag, so multiline replies lose their reply metadata altogether. Please split the tag handling so the start line keeps @… BATCH … and the PRIVMSGs include both the batch id and the reply tag. Example fix:
- const replyPrefix = localReplyTo
- ? `@+draft/reply=${localReplyTo.id};`
- : "";
-
- ircClient.sendRaw(
- selectedServerId,
- `${replyPrefix}BATCH +${batchId} draft/multiline ${target}`,
- );
+ const replyTag = localReplyTo
+ ? `+draft/reply=${localReplyTo.id}`
+ : null;
+ const batchStartPrefix = replyTag
+ ? `@${replyTag} `
+ : "";
+ ircClient.sendRaw(
+ selectedServerId,
+ `${batchStartPrefix}BATCH +${batchId} draft/multiline ${target}`,
+ );
+ const mkPrivmsgPrefix = (includeConcat: boolean) => {
+ const tags = [
+ `batch=${batchId}`,
+ includeConcat ? "draft/multiline-concat" : null,
+ replyTag,
+ ]
+ .filter(Boolean)
+ .join(";");
+ return tags ? `@${tags} ` : "";
+ };and replace the hard-coded @batch=… literals below with mkPrivmsgPrefix(index > 0). That keeps the wire format valid and preserves reply tags on every line.
🤖 Prompt for AI Agents
In src/hooks/useMessageSending.ts around lines 104–175, the BATCH start line is
concatenating the reply tag directly with "BATCH" (e.g. "@+draft/reply=…;BATCH
…") which is invalid and the PRIVMSG lines never carry the reply tag; fix by:
add a space between the reply tag and the BATCH token so the start line becomes
"<replyTag> BATCH …", and ensure every PRIVMSG line includes both the batch tag
and the reply tag (not just "@batch=…"); implement a small helper (e.g.
mkPrivmsgPrefix(isConcat:boolean)) that returns the correct tag prefix for each
line — first segment and subsequent concat segments — combining "@batch=<id>"
and the draft/reply tag when present, then replace all hard-coded "@batch=…"
literals with calls to that helper so multiline messages preserve valid IRC tag
syntax and keep reply metadata on every line.
| * Hook for managing message reactions and reaction modal | ||
| */ | ||
| import { useCallback, useState } from "react"; | ||
| import ircClient from "../lib/ircClient"; |
There was a problem hiding this comment.
Fix ircClient import to match named export.
Line 5 imports ircClient as a default, but src/lib/ircClient.ts (Line 1703) only exports a named constant. With ES module interop, the default import becomes undefined, so every sendRaw call throws. Switch to the named import here (and in other new files).
-import ircClient from "../lib/ircClient";
+import { ircClient } from "../lib/ircClient";📝 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.
| import ircClient from "../lib/ircClient"; | |
| import { ircClient } from "../lib/ircClient"; |
🤖 Prompt for AI Agents
In src/hooks/useReactions.ts around line 5, the file currently imports ircClient
as a default (import ircClient from "../lib/ircClient";) but
src/lib/ircClient.ts only exports a named constant, causing the import to be
undefined; change the import to the named export (import { ircClient } from
"../lib/ircClient";) and update any other new files that used the default import
to use the named import; ensure any references to the variable remain unchanged
and run the build/tests to verify sendRaw calls no longer throw.
| * Consolidates typing notification logic and throttling | ||
| */ | ||
| import { useCallback, useRef } from "react"; | ||
| import ircClient from "../lib/ircClient"; |
There was a problem hiding this comment.
Align ircClient import with named export.
Line 6 default-imports ircClient, but src/lib/ircClient.ts only exports it by name, so this evaluates to undefined at runtime and breaks typing notifications. Please use the named import.
-import ircClient from "../lib/ircClient";
+import { ircClient } from "../lib/ircClient";🤖 Prompt for AI Agents
In src/hooks/useTypingNotification.ts around line 6 the import currently uses a
default import for ircClient but the module at src/lib/ircClient.ts exports it
as a named export, which causes ircClient to be undefined at runtime; change the
import to a named import (import { ircClient } from "../lib/ircClient";) and
update any local references if necessary to use the named binding.
Resolved conflicts in ChatArea.tsx by preserving both: - Message history navigation with arrow keys (from feat/keyup-to-history) - Search functionality, message virtualization, and new modals (from main) - useMessageSending, useReactions hooks (from feat/keyup-to-history) - All UI features and moderation capabilities (from main) All tests passing, code formatted and linted.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/message/MessageItem.tsx (1)
100-124: Remove duplicate error handling block.The
imageErrorcheck and fallback UI are duplicated (lines 100-111 and 113-124). The second block is unreachable and serves no purpose.Apply this diff to remove the duplicate code:
if (imageError) { // Fallback to showing expired badge return ( <div className="max-w-md"> <div className="bg-gray-100 border border-gray-300 rounded-lg p-4 text-center"> <div className="inline-flex items-center px-3 py-1 rounded-full text-sm bg-red-100 text-red-800 border border-red-200"> <span>This image has expired</span> </div> </div> </div> ); } - - if (imageError) { - // Fallback to showing expired badge - return ( - <div className="max-w-md"> - <div className="bg-gray-100 border border-gray-300 rounded-lg p-4 text-center"> - <div className="inline-flex items-center px-3 py-1 rounded-full text-sm bg-red-100 text-red-800 border border-red-200"> - <span>This image has expired</span> - </div> - </div> - </div> - ); - } return (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/layout/ChatArea.tsx(22 hunks)src/components/message/MessageItem.tsx(1 hunks)tests/components/StandardReplyNotification.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/components/StandardReplyNotification.test.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/layout/ChatArea.tsx (10)
src/hooks/useTypingNotification.ts (1)
useTypingNotification(22-80)src/hooks/useMediaQuery.ts (1)
useMediaQuery(3-15)src/hooks/useMessageSending.ts (1)
useMessageSending(35-332)src/hooks/useReactions.ts (1)
useReactions(31-157)src/hooks/useMessageHistory.ts (1)
useMessageHistory(28-124)src/components/ui/ScrollToBottomButton.tsx (1)
ScrollToBottomButton(10-35)src/components/ui/ReplyBadge.tsx (1)
ReplyBadge(15-29)src/components/ui/InputToolbar.tsx (1)
InputToolbar(16-52)src/components/ui/EmojiPickerModal.tsx (1)
EmojiPickerModal(17-57)src/components/ui/ImagePreviewModal.tsx (1)
ImagePreviewModal(16-60)
🔇 Additional comments (1)
src/components/message/MessageItem.tsx (1)
496-505: Word-breaking implementation looks good.The CSS changes correctly handle long content by breaking words across lines. The combination of
overflow-hiddenwithbreak-wordproperties ensures long URLs and text wrap properly.Note: The PR mentions "truncate long URLs", but this implementation breaks URLs across multiple lines rather than shortening them with ellipsis. If actual truncation (e.g., displaying "https://exa...") is desired, consider using
text-overflow: ellipsiswithwhite-space: nowrapinstead.
| if (e.key === "ArrowUp") { | ||
| // Only activate if input is empty or already in history mode | ||
| if (messageText === "" || messageHistory.messageHistoryIndex >= 0) { | ||
| e.preventDefault(); | ||
|
|
||
| if (messageHistory.userMessageHistory.length === 0) return; | ||
|
|
||
| const previousMessage = messageHistory.navigateUp(messageText); | ||
| if (previousMessage !== null) { | ||
| setMessageText(previousMessage); | ||
|
|
||
| // Move cursor to end of text | ||
| setTimeout(() => { | ||
| if (inputRef.current) { | ||
| inputRef.current.setSelectionRange( | ||
| previousMessage.length, | ||
| previousMessage.length, | ||
| ); | ||
| } | ||
| }, 0); | ||
| } | ||
| } | ||
| return; |
There was a problem hiding this comment.
Don't intercept ArrowUp when no history exists
e.preventDefault() fires before we know whether any history entries are available, so on an empty history (fresh channel, PM with no messages, etc.) ArrowUp is swallowed and the caret won't move. Reset history state (if needed) and only call preventDefault when a history entry is actually available.
if (e.key === "ArrowUp") {
// Only activate if input is empty or already in history mode
if (messageText === "" || messageHistory.messageHistoryIndex >= 0) {
- e.preventDefault();
-
- if (messageHistory.userMessageHistory.length === 0) return;
+ if (messageHistory.userMessageHistory.length === 0) {
+ if (messageHistory.messageHistoryIndex >= 0) {
+ messageHistory.resetHistory();
+ }
+ return;
+ }
+
+ e.preventDefault();
const previousMessage = messageHistory.navigateUp(messageText);
if (previousMessage !== null) {📝 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.
| if (e.key === "ArrowUp") { | |
| // Only activate if input is empty or already in history mode | |
| if (messageText === "" || messageHistory.messageHistoryIndex >= 0) { | |
| e.preventDefault(); | |
| if (messageHistory.userMessageHistory.length === 0) return; | |
| const previousMessage = messageHistory.navigateUp(messageText); | |
| if (previousMessage !== null) { | |
| setMessageText(previousMessage); | |
| // Move cursor to end of text | |
| setTimeout(() => { | |
| if (inputRef.current) { | |
| inputRef.current.setSelectionRange( | |
| previousMessage.length, | |
| previousMessage.length, | |
| ); | |
| } | |
| }, 0); | |
| } | |
| } | |
| return; | |
| if (e.key === "ArrowUp") { | |
| // Only activate if input is empty or already in history mode | |
| if (messageText === "" || messageHistory.messageHistoryIndex >= 0) { | |
| if (messageHistory.userMessageHistory.length === 0) { | |
| if (messageHistory.messageHistoryIndex >= 0) { | |
| messageHistory.resetHistory(); | |
| } | |
| return; | |
| } | |
| e.preventDefault(); | |
| const previousMessage = messageHistory.navigateUp(messageText); | |
| if (previousMessage !== null) { | |
| setMessageText(previousMessage); | |
| // Move cursor to end of text | |
| setTimeout(() => { | |
| if (inputRef.current) { | |
| inputRef.current.setSelectionRange( | |
| previousMessage.length, | |
| previousMessage.length, | |
| ); | |
| } | |
| }, 0); | |
| } | |
| } | |
| return; |
🤖 Prompt for AI Agents
In src/components/layout/ChatArea.tsx around lines 687 to 709, the key handler
calls e.preventDefault() before confirming any history entries exist causing
ArrowUp to be swallowed on empty history; change the logic so you first check if
messageHistory.userMessageHistory.length === 0 (and if so reset history state if
needed and return) and only call e.preventDefault() after confirming a history
entry exists (just before calling messageHistory.navigateUp and updating
setMessageText), keeping the rest of the cursor positioning logic the same.
|
|
||
| // Get current user's status in the selected channel | ||
| const currentUserStatus = useMemo(() => { | ||
| if (!selectedServerId || !selectedChannelId) return undefined; |
There was a problem hiding this comment.
This component was a refactor. It stil lexists as a duplicate. Either remove it or put this code into it
| onToggleMemberList={() => toggleMemberList(!isMemberListVisible)} | ||
| /> | ||
| <div className="min-h-[56px] px-4 border-b border-discord-dark-400 flex flex-wrap items-start md:items-center justify-between shadow-sm py-2 md:py-0 md:h-12 gap-y-2"> | ||
| <div className="flex items-center flex-1 min-w-0 w-full md:w-auto"> |
There was a problem hiding this comment.
could also be a new component
|
@ValwareIRC feel free to either remove the duplicated code or refactor into it. Then merge |
- Moved all inline header JSX from ChatArea.tsx to ChatHeader.tsx - Added state management for topic editing (isEditingTopic, editedTopic) - Added props for all required callbacks and data - Removed ~260 lines of inline JSX from ChatArea.tsx - ChatHeader now handles: * Channel/private chat/server notices display * Topic editing with inline form * Search functionality * Notification toggle * Channel settings, invite user, rename buttons * Member list toggle * Channel avatar support with fallback This makes ChatArea.tsx cleaner and ChatHeader.tsx more reusable. All tests passing ✅
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/components/layout/ChatArea.tsx (1)
665-686: ArrowUp still swallows caret with empty history
preventDefault()fires before confirming history exists, so a fresh input can no longer move the caret/home on ArrowUp—exactly the issue from the prior review. Check for history first (reset if needed) and only then intercept the key.if (e.key === "ArrowUp") { // Only activate if input is empty or already in history mode if (messageText === "" || messageHistory.messageHistoryIndex >= 0) { - e.preventDefault(); - - if (messageHistory.userMessageHistory.length === 0) return; + if (messageHistory.userMessageHistory.length === 0) { + if (messageHistory.messageHistoryIndex >= 0) { + messageHistory.resetHistory(); + } + return; + } + + e.preventDefault(); const previousMessage = messageHistory.navigateUp(messageText);src/components/layout/ChatHeader.tsx (1)
17-17: Use the named ircClient export.Line 17 default-imports
ircClient, butsrc/lib/ircClient.tsexposes it as a named export (line 2126), so this resolves toundefinedand any subsequent calls (lines 73, 141) will fail.Apply this diff:
-import ircClient from "../../lib/ircClient"; +import { ircClient } from "../../lib/ircClient";
🧹 Nitpick comments (2)
src/components/layout/ChatHeader.tsx (2)
179-211: Eliminate duplicate operator-permission checks.The same logic (finding
currentUserInChanneland callinghasOpPermission) appears in bothonClick(lines 182–188) andtitle(lines 195–202). Extract it once before the button.Apply this diff:
) : ( + (() => { + const currentUserInChannel = selectedChannel?.users.find( + (u) => u.username === currentUser?.username, + ); + const canEdit = hasOpPermission(currentUserInChannel?.status); + return ( <button onClick={() => { if (selectedChannel) { - const currentUserInChannel = selectedChannel.users.find( - (u) => u.username === currentUser?.username, - ); - if (hasOpPermission(currentUserInChannel?.status)) { + if (canEdit) { setEditedTopic(selectedChannel.topic || ""); setIsEditingTopic(true); } } }} className="text-discord-text-muted text-xs md:text-sm hover:text-white truncate min-w-0 md:max-w-md mt-0.5 mb-1 md:mt-0 md:mb-0" title={ selectedChannel.topic - ? `Topic: ${selectedChannel.topic}${ - selectedChannel.users.find( - (u) => u.username === currentUser?.username, - ) && - hasOpPermission( - selectedChannel.users.find( - (u) => u.username === currentUser?.username, - )?.status, - ) - ? " (Click to edit)" - : "" - }` + ? `Topic: ${selectedChannel.topic}${canEdit ? " (Click to edit)" : ""}` : "No topic set" } > {selectedChannel.topic || "No topic"} </button> + ); + })() )}
99-117: Consider state-based fallback instead of direct DOM manipulation.The
onErrorhandler directly manipulates the DOM (lines 106–113) to hide the image and show the fallback icon, which bypasses React's virtual DOM reconciliation.Refactor to use state for a more idiomatic React approach:
+ const [avatarError, setAvatarError] = useState(false); + // ... inside the render: {getChannelAvatarUrl(selectedChannel.metadata, 50) ? ( + !avatarError ? ( <img src={getChannelAvatarUrl(selectedChannel.metadata, 50)} alt={selectedChannel.name} className="w-12 h-12 rounded-full object-cover mr-2 flex-shrink-0" onError={(e) => { - // Fallback to # icon on error - e.currentTarget.style.display = "none"; - const parent = e.currentTarget.parentElement; - const fallbackIcon = parent?.querySelector( - ".fallback-hash-icon", - ); - if (fallbackIcon) { - (fallbackIcon as HTMLElement).style.display = - "inline-block"; - } + setAvatarError(true); }} /> + ) : ( + <FaHashtag className="text-discord-text-muted mr-2 flex-shrink-0 text-3xl" /> + ) ) : ( + <FaHashtag className="text-discord-text-muted mr-2 flex-shrink-0 text-3xl" /> - null )} - <FaHashtag - className="text-discord-text-muted mr-2 fallback-hash-icon flex-shrink-0 text-3xl" - style={{ - display: getChannelAvatarUrl(selectedChannel.metadata, 50) - ? "none" - : "inline-block", - }} - />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/layout/ChatArea.tsx(21 hunks)src/components/layout/ChatHeader.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/layout/ChatHeader.tsx (3)
src/types/index.ts (3)
Channel(51-69)PrivateChat(71-78)User(1-14)src/lib/ircClient.ts (1)
ircClient(2127-2127)src/lib/ircUtils.tsx (3)
getChannelAvatarUrl(339-354)getChannelDisplayName(324-334)hasOpPermission(360-370)
src/components/layout/ChatArea.tsx (11)
src/hooks/useTypingNotification.ts (1)
useTypingNotification(22-80)src/hooks/useMediaQuery.ts (1)
useMediaQuery(3-15)src/hooks/useMessageSending.ts (1)
useMessageSending(35-332)src/hooks/useReactions.ts (1)
useReactions(31-157)src/hooks/useMessageHistory.ts (1)
useMessageHistory(28-124)src/components/layout/ChatHeader.tsx (1)
ChatHeader(47-332)src/components/ui/ScrollToBottomButton.tsx (1)
ScrollToBottomButton(10-35)src/components/ui/ReplyBadge.tsx (1)
ReplyBadge(15-29)src/components/ui/InputToolbar.tsx (1)
InputToolbar(16-52)src/components/ui/EmojiPickerModal.tsx (1)
EmojiPickerModal(17-57)src/components/ui/ImagePreviewModal.tsx (1)
ImagePreviewModal(16-60)
| const isOperator = (() => { | ||
| if (!selectedChannel || !selectedServerId) return false; | ||
| const serverCurrentUser = ircClient.getCurrentUser(selectedServerId); | ||
| if (!serverCurrentUser) return false; | ||
|
|
||
| const channelUser = selectedChannel.users.find( | ||
| (u) => u.username === serverCurrentUser.username, | ||
| ); | ||
| return ( | ||
| channelUser?.status?.includes("@") || channelUser?.status?.includes("~") | ||
| ); | ||
| })(); |
There was a problem hiding this comment.
Incomplete operator check excludes admins.
The isOperator check only tests for @ (op) and ~ (owner) but omits & (admin). Admins should have operator privileges, so they'll incorrectly be denied topic editing and channel renaming.
Use the existing hasOpPermission utility (already imported line 21) for consistency:
// Check if current user is operator
- const isOperator = (() => {
+ const isOperator = (() => {
if (!selectedChannel || !selectedServerId) return false;
const serverCurrentUser = ircClient.getCurrentUser(selectedServerId);
if (!serverCurrentUser) return false;
const channelUser = selectedChannel.users.find(
(u) => u.username === serverCurrentUser.username,
);
- return (
- channelUser?.status?.includes("@") || channelUser?.status?.includes("~")
- );
+ return hasOpPermission(channelUser?.status);
})();📝 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.
| const isOperator = (() => { | |
| if (!selectedChannel || !selectedServerId) return false; | |
| const serverCurrentUser = ircClient.getCurrentUser(selectedServerId); | |
| if (!serverCurrentUser) return false; | |
| const channelUser = selectedChannel.users.find( | |
| (u) => u.username === serverCurrentUser.username, | |
| ); | |
| return ( | |
| channelUser?.status?.includes("@") || channelUser?.status?.includes("~") | |
| ); | |
| })(); | |
| const isOperator = (() => { | |
| if (!selectedChannel || !selectedServerId) return false; | |
| const serverCurrentUser = ircClient.getCurrentUser(selectedServerId); | |
| if (!serverCurrentUser) return false; | |
| const channelUser = selectedChannel.users.find( | |
| (u) => u.username === serverCurrentUser.username, | |
| ); | |
| return hasOpPermission(channelUser?.status); | |
| })(); |
🤖 Prompt for AI Agents
In src/components/layout/ChatHeader.tsx around lines 71 to 82, the inline
isOperator check only looks for "@" and "~" and thus excludes "&" (admin);
replace this manual status substring logic with a call to the existing
hasOpPermission utility (already imported on line 21) to determine operator
privileges consistently — get the channelUser as currently done and pass it (or
its status) to hasOpPermission, then use the boolean result for isOperator so
admins are correctly granted operator rights.
* keyup * refactor chat area * truncate long urls * DRY coderabit suggestion * refactor: Extract header logic from ChatArea into ChatHeader component - Moved all inline header JSX from ChatArea.tsx to ChatHeader.tsx - Added state management for topic editing (isEditingTopic, editedTopic) - Added props for all required callbacks and data - Removed ~260 lines of inline JSX from ChatArea.tsx - ChatHeader now handles: * Channel/private chat/server notices display * Topic editing with inline form * Search functionality * Notification toggle * Channel settings, invite user, rename buttons * Member list toggle * Channel avatar support with fallback This makes ChatArea.tsx cleaner and ChatHeader.tsx more reusable. All tests passing ✅ --------- Co-authored-by: Valerie Liu <v.a.pond@outlook.com>
2 participants
Summary by CodeRabbit