Skip to content

Private Messges#60

Merged
ValwareIRC merged 6 commits into
mainfrom
feat/PMs
Sep 22, 2025
Merged

Private Messges#60
ValwareIRC merged 6 commits into
mainfrom
feat/PMs

Conversation

@matheusfillipe
Copy link
Copy Markdown
Contributor

@matheusfillipe matheusfillipe commented Sep 20, 2025

Summary by CodeRabbit

  • New Features

    • Private messages: start chats from a user menu or modal, view a Private Messages list, send/receive DMs with unread counts and typing indicators. Member list hides during private chats.
    • User context menu on usernames and member list with “Send Message.” Clickable usernames open actions. Improved username autocomplete.
  • Improvements

    • Contextual chat headers/placeholders and correct routing for channel vs. private messages.
    • Echobot now responds to mentions and direct messages.
  • Bug Fixes

    • Replies now go to the correct target (channel or DM).
  • Chores

    • Added .dockerignore; Docker Compose tweaks and unbuffered bot output.
image image

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Sep 20, 2025

Walkthrough

Adds private messaging across UI, store, and IRC client: new PrivateChat model, user context menus, modal to start PMs, ChatArea routing for PM send/typing, and TAGMSG handling. Infra tweaks include .dockerignore, compose updates, and a more capable echobot. Tests updated to reflect new state and typing API.

Changes

Cohort / File(s) Summary
Docker & Infra
./.dockerignore, compose.yaml, docker/echobot.py
New .dockerignore excluding src-tauri/target. Compose quoting tweaks; echobot commands now unbuffered (-u). Echobot now handles PMs, logs messages, uses mention detection, and replies to correct target.
Layout & Messaging UI
src/components/layout/AppLayout.tsx, src/components/layout/ChannelList.tsx, src/components/layout/ChatArea.tsx, src/components/layout/MemberList.tsx
AppLayout hides member list when a private chat is active. ChannelList adds “Private Messages” section with open/delete and AddPrivateChatModal trigger. ChatArea supports PM context, user context menus, autocomplete, per-target typing, and local echo of outgoing PMs. MemberList adds per-user context menu and PM initiation. Public props extended in MessageItem and UserItem.
New UI Components
src/components/ui/ClickableUsername.tsx, src/components/ui/UserContextMenu.tsx, src/components/ui/AddPrivateChatModal.tsx
Adds ClickableUsername, a UserContextMenu with “Send Message,” and AddPrivateChatModal to search/select users for PMs. Viewport-clamped positioning and global close handlers.
Core Store/Types/Client
src/store/index.ts, src/types/index.ts, src/lib/ircClient.ts
Introduces PrivateChat type and Server.privateChats. Store adds selectedPrivateChatId, actions: selectPrivateChat, openPrivateChat, deletePrivateChat; handles USERMSG and TAGMSG for PMs; routes messages to PM channels. IRCClient adds sendTyping and getAllUsers; initializes privateChats; simplifies TAGMSG target handling.
Tests
tests/components/ChatArea.test.tsx
Mocks extended with sendTyping; server mock includes privateChats; UI state includes selectedPrivateChatId; minor DOM typings for selectionStart.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant CL as ChannelList
  participant M as AddPrivateChatModal
  participant S as Store
  participant UI as ChatArea

  U->>CL: Click "Private Messages" ➜ Plus
  CL->>M: Open modal (serverId)
  U->>M: Select username
  M->>S: openPrivateChat(serverId, username)
  S->>S: Create/find PrivateChat, set selectedPrivateChatId
  S-->>UI: State updated (render PM view)
Loading
sequenceDiagram
  autonumber
  actor U as User
  participant UI as ChatArea
  participant C as IRCClient
  participant IRC as IRC Server
  participant S as Store

  U->>UI: Type message in PM
  UI->>C: sendTyping(serverId, username, true)
  C->>IRC: TAGMSG + typing=active
  UI->>C: PRIVMSG username :text
  Note over UI,S: Locally echo outgoing PM
  IRC-->>C: USERMSG/PRIVMSG from peer
  C-->>S: Dispatch incoming PM
  S->>S: Create/find PrivateChat, add Message, update unread
  UI->>C: sendTyping(serverId, username, false)
  C->>IRC: TAGMSG + typing=done
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • feat/tab completion #59 — Overlapping changes to ChatArea, autocomplete, context menus, and similar infra edits suggest shared implementation scope.

Suggested reviewers

  • ValwareIRC

Poem

A bunny taps to start a chat,
With whiskered clicks and menus that
Pop by names and softly say,
“Let’s PM-hop this lovely day!”
New lanes burrow through the UI’s map—
Thump-thump! I send a typing tap.
Echoes bounce. We’ve closed the gap. 🐇💬

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning While the private-messaging code changes align with the PR objective, this branch also includes infrastructure and tooling edits that are not clearly required by the private-messaging feature (notably .dockerignore, compose.yaml quoting/command changes, and docker/echobot.py modifications), increasing review scope and suggesting unrelated work in the same PR. If those infra/tool changes were included to support development/testing of PMs (for example an updated test bot), that justification is not documented in the PR description. Either move the infra/tooling edits into a separate PR or add an explicit justification in this PR explaining why each infra change is required for the private-messaging work and how they affect tests/dev tooling, and ensure any dependent test bots or scripts are documented and referenced by the PR.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Linked Issues Check ❓ Inconclusive The changeset implements a comprehensive private-messaging flow (new PrivateChat type, store actions open/select/delete, UI components including a Private Messages section and AddPrivateChatModal, ChatArea/MemberList wiring, IRC client updates like sendTyping and TAGMSG handling, and test updates), so it appears to address the coding work for private messages referenced by issue [#29]. However the linked issue text provided is minimal ("Add private messages under voice channels") and lacks explicit acceptance criteria or UI placement requirements, so I cannot conclusively verify that every expected requirement (for example exact UI placement under voice channels, persistence or server-side contract expectations, permissions, and any acceptance tests) has been satisfied. Please provide the issue's acceptance criteria or a short checklist (UI placement details, required server-side behavior, persistence expectations, permission rules, and any tests) or confirm which of those items this PR is intended to satisfy so I can re-evaluate compliance precisely.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The PR title "Private Messges" is concise and directly related to the primary feature implemented (private messages/PMs) and therefore summarizes the main change in the changeset, but it contains a spelling error and would be clearer if written as an action ("Add private messages") or corrected to "Private Messages".
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/PMs

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@matheusfillipe matheusfillipe changed the base branch from main to feat/tab-completion September 20, 2025 16:30
@github-actions
Copy link
Copy Markdown

Pages Preview
Preview URL: https://feat-pms.obsidianirc.pages.dev

Automated deployment preview for the PR in the Cloudflare Pages.

Base automatically changed from feat/tab-completion to main September 21, 2025 22:21
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (16)
docker/echobot.py (4)

41-41: Make self-check case-insensitive

Avoid self-replies if server normalizes nick case.

-                if sender != NICK:
+                if sender.casefold() != NICK.casefold():

43-45: Broaden mention detection (support @nick, commas, spaces; avoid brittle word boundary)

Current regex misses “EchoBot,” “@echobot hello” etc.

-                    nick_mentioned = re.search(rf"\b{re.escape(NICK)}\b", message, re.IGNORECASE) or re.match(
-                        rf"^{re.escape(NICK)}:\s+", message, re.IGNORECASE
-                    )
+                    nick_mentioned = (
+                        re.search(rf'(^|\W){re.escape(NICK)}(\W|$)', message, re.IGNORECASE)
+                        or re.match(rf'^\s*@?{re.escape(NICK)}\s*[:,\-]?\s+', message, re.IGNORECASE)
+                    )

46-46: Ensure logs flush in Docker stdout

Prevents missing lines in buffered environments.

-                    print(f"Message from {sender} in {target}: {message}")
+                    print(f"Message from {sender} in {target}: {message}", flush=True)

54-58: Mirror gating and delay for channel mentions

Keep TAGMSG optional and reduce/blocking delay similarly.

-                    elif nick_mentioned:
-                        send(f"@+typing=active TAGMSG {target}")
-                        time.sleep(2)  # Simulate typing delay
-                        send(f"PRIVMSG {target} :{sender}: I heard you mention me! You said: {message}")
+                    elif nick_mentioned:
+                        if TYPING_TAGS:
+                            send(f"@+typing=active TAGMSG {target}")
+                        time.sleep(TYPING_DELAY)  # Simulate typing delay
+                        send(f"PRIVMSG {target} :{sender}: I heard you mention me! You said: {message}")
src/components/ui/ClickableUsername.tsx (2)

14-26: Consider accessibility improvements for keyboard navigation.

The component handles mouse clicks but lacks keyboard accessibility support. Users should be able to trigger the context menu using keyboard navigation.

Add keyboard support for better accessibility:

 const handleClick = (e: React.MouseEvent) => {
   e.preventDefault();
   e.stopPropagation();
   if (onContextMenu) {
     onContextMenu(e, username, serverId);
   }
 };
+
+const handleKeyDown = (e: React.KeyboardEvent) => {
+  if (e.key === 'Enter' || e.key === ' ') {
+    e.preventDefault();
+    e.stopPropagation();
+    if (onContextMenu) {
+      // Convert keyboard event to mouse-like coordinates for positioning
+      const rect = e.currentTarget.getBoundingClientRect();
+      const mouseEvent = {
+        clientX: rect.left,
+        clientY: rect.bottom,
+      } as React.MouseEvent;
+      onContextMenu(mouseEvent, username, serverId);
+    }
+  }
+};

28-36: Add keyboard event handler and improve accessibility attributes.

The span element needs keyboard support and proper ARIA attributes for screen reader users.

 return (
   <span
     className={`cursor-pointer ${className}`}
     onClick={handleClick}
+    onKeyDown={handleKeyDown}
     title={`Click to message ${username}`}
+    tabIndex={0}
+    role="button"
+    aria-label={`Message ${username}`}
   >
     {username}
   </span>
 );
src/types/index.ts (1)

48-55: Consider adding message tracking to PrivateChat interface.

The PrivateChat interface tracks metadata but doesn't include a messages array like the Channel interface does. This might require separate message storage/retrieval logic for private chats.

Consider aligning the data model with channels for consistency:

 export interface PrivateChat {
   id: string;
   username: string;
   serverId: string;
   unreadCount: number;
   isMentioned: boolean;
   lastActivity?: Date;
+  messages?: Message[];
+  user?: User; // Reference to the user object for status/avatar
 }

This would provide:

  • Consistent message storage between channels and private chats
  • Direct access to user details (status, avatar) without lookups
  • Simplified message retrieval logic in the UI
src/components/ui/UserContextMenu.tsx (1)

56-58: Consider using a more robust viewport boundary calculation.

The current implementation uses fixed offsets (200px width, 100px height) which might not match the actual menu dimensions, potentially causing clipping issues on smaller screens.

Use the actual menu dimensions for more accurate positioning:

-  // Adjust position to prevent menu from going off-screen
-  const adjustedX = Math.min(x, window.innerWidth - 200);
-  const adjustedY = Math.min(y, window.innerHeight - 100);
+  // Adjust position to prevent menu from going off-screen
+  const menuWidth = 200; // Consider making this configurable
+  const menuHeight = 100; // Consider making this configurable
+  const padding = 8; // Safety margin from viewport edges
+  
+  let adjustedX = x;
+  let adjustedY = y;
+  
+  // Flip horizontally if too close to right edge
+  if (x + menuWidth + padding > window.innerWidth) {
+    adjustedX = Math.max(padding, x - menuWidth);
+  }
+  
+  // Flip vertically if too close to bottom edge
+  if (y + menuHeight + padding > window.innerHeight) {
+    adjustedY = Math.max(padding, y - menuHeight);
+  }
src/lib/ircClient.ts (1)

529-543: Potential performance issue with getAllUsers method.

The current implementation iterates through all channels and users without any caching mechanism. For servers with many channels and users, this could become a performance bottleneck, especially if called frequently (e.g., during user search for private messages).

Consider implementing a cached user list that updates on JOIN/PART/QUIT events:

 export class IRCClient {
   private sockets: Map<string, WebSocket> = new Map();
   private servers: Map<string, Server> = new Map();
   private nicks: Map<string, string> = new Map();
   private currentUser: User | null = null;
+  private serverUsersCache: Map<string, Map<string, User>> = new Map();

   // ... other methods ...

   getAllUsers(serverId: string): User[] {
     const server = this.servers.get(serverId);
     if (!server) return [];
 
-    const allUsers = new Map<string, User>();
-
-    // Collect users from all joined channels
-    for (const channel of server.channels) {
-      for (const user of channel.users) {
-        allUsers.set(user.username, user);
-      }
-    }
-
-    return Array.from(allUsers.values());
+    // Return cached users if available
+    let usersCache = this.serverUsersCache.get(serverId);
+    if (!usersCache) {
+      usersCache = new Map<string, User>();
+      
+      // Build initial cache
+      for (const channel of server.channels) {
+        for (const user of channel.users) {
+          usersCache.set(user.username, user);
+        }
+      }
+      
+      this.serverUsersCache.set(serverId, usersCache);
+    }
+    
+    return Array.from(usersCache.values());
   }
+
+  // Add cache invalidation in JOIN/PART/QUIT/NICK handlers
+  private invalidateUsersCache(serverId: string): void {
+    this.serverUsersCache.delete(serverId);
+  }

Then call invalidateUsersCache in the appropriate event handlers.

src/components/layout/MemberList.tsx (1)

103-120: Consider extracting magic numbers and improving menu positioning logic.

The hardcoded offset (-5) and the positioning logic could be more maintainable.

+const MENU_AVATAR_GAP = 5; // Gap between avatar and menu
+const MENU_OFFSET = 10; // Default offset from cursor

 let x = e.clientX;
 let y = e.clientY;
 
 // If avatar element is provided, position menu relative to it
 if (avatarElement) {
   const rect = avatarElement.getBoundingClientRect();
-  x = rect.left;
-  y = rect.top - 5; // Position above the avatar with small gap
+  // Position menu to the right of avatar if space allows, otherwise to the left
+  const spaceOnRight = window.innerWidth - rect.right;
+  x = spaceOnRight > 200 ? rect.right + MENU_OFFSET : rect.left - 200 - MENU_OFFSET;
+  y = rect.top; // Align with top of avatar
 }
tests/components/ChatArea.test.tsx (1)

268-280: Minor type assertion improvement opportunity.

Consider using a type guard or extracting the type assertion into a helper function for better readability and reusability.

-    const initialCursorPosition = (input as HTMLInputElement).selectionStart;
+    const inputElement = input as HTMLInputElement;
+    const initialCursorPosition = inputElement.selectionStart;

     fireEvent.keyDown(input, { key: "ArrowDown", code: "ArrowDown" });

-    expect((input as HTMLInputElement).selectionStart).toBe(
-      initialCursorPosition,
-    );
+    expect(inputElement.selectionStart).toBe(initialCursorPosition);

     fireEvent.keyDown(input, { key: "Up", code: "ArrowUp" });

-    expect((input as HTMLInputElement).selectionStart).toBe(
-      initialCursorPosition,
-    );
+    expect(inputElement.selectionStart).toBe(initialCursorPosition);
src/components/ui/AddPrivateChatModal.tsx (2)

25-25: Complex type annotation could be simplified.

The type annotation (typeof server.channels)[0]["users"][0] is complex and could be more maintainable with a proper type import.

Import the User type and use it directly:

-    const allUsers = new Map<string, (typeof server.channels)[0]["users"][0]>();
+    const allUsers = new Map<string, User>();

Add the import at the top:

import type { User } from "../../types";

28-32: Consider deduplication by username rather than reference.

The current implementation may not properly deduplicate users if the same user object isn't reused across channels. Consider deduplicating by username to ensure uniqueness.

     // Collect users from all channels
     for (const channel of server.channels) {
       for (const user of channel.users) {
-        allUsers.set(user.username, user);
+        // Use username as key to ensure deduplication
+        if (!allUsers.has(user.username)) {
+          allUsers.set(user.username, user);
+        }
       }
     }
src/components/layout/ChannelList.tsx (1)

261-271: Consider removing the avatar click handler for system messages.

The click handler on Lines 261-271 adds a cursor pointer and click event for system messages, but the handler checks and returns early if it's a system message. Consider skipping the handler setup entirely.

-          <div
-            className={`mr-4 ${message.userId !== "system" && currentUser?.username !== message.userId.split("-")[0] ? "cursor-pointer" : ""}`}
-            onClick={(e) => {
-              if (message.userId !== "system") {
-                onUsernameContextMenu(
-                  e,
-                  message.userId.split("-")[0],
-                  message.serverId,
-                  e.currentTarget,
-                );
-              }
-            }}
-          >
+          {message.userId !== "system" ? (
+            <div
+              className={`mr-4 ${currentUser?.username !== message.userId.split("-")[0] ? "cursor-pointer" : ""}`}
+              onClick={(e) => {
+                onUsernameContextMenu(
+                  e,
+                  message.userId.split("-")[0],
+                  message.serverId,
+                  e.currentTarget,
+                );
+              }}
+            >
+              <div className={`w-8 h-8 rounded-full bg-${theme}-dark-400 flex items-center justify-center text-white`}>
+                {message.userId.charAt(0).toUpperCase()}
+              </div>
+            </div>
+          ) : (
+            <div className="mr-4">
+              <div className={`w-8 h-8 rounded-full bg-${theme}-dark-400 flex items-center justify-center text-white`}>
+                {message.userId.charAt(0).toUpperCase()}
+              </div>
+            </div>
+          )}
src/components/layout/ChatArea.tsx (2)

627-636: Consider extracting common typing notification logic.

The typing notification logic is repeated multiple times. Consider extracting it into a helper function.

+      const sendTypingNotification = (active: boolean) => {
+        const target = selectedChannel?.name ?? selectedPrivateChat?.username;
+        if (target && selectedServerId) {
+          ircClient.sendTyping(selectedServerId, target, active);
+        }
+      };

       // Send typing done notification
-      if (selectedChannel?.name || selectedPrivateChat?.username) {
-        const target = selectedChannel?.name ?? selectedPrivateChat?.username;
-        ircClient.sendTyping(
-          selectedServerId as string,
-          target as string,
-          false,
-        );
-      }
+      sendTypingNotification(false);

Apply similar refactoring to lines 662-674, 887-899, and 892-899.


687-702: Minor optimization opportunity for private chat user list.

The user list construction for private chats creates a new array on every render. Consider memoizing this.

+  const privateChatUsers = useMemo(() => {
+    if (!selectedPrivateChat) return [];
+    return [
+      ...(currentUser ? [currentUser] : []),
+      {
+        id: `${selectedPrivateChat.serverId}-${selectedPrivateChat.username}`,
+        username: selectedPrivateChat.username,
+        isOnline: true,
+      },
+    ];
+  }, [selectedPrivateChat, currentUser]);

   // For channels, use channel users; for private chats, use both participants
   const users =
-    selectedChannel?.users ||
-    (selectedPrivateChat
-      ? [
-          ...(currentUser ? [currentUser] : []),
-          {
-            id: `${selectedPrivateChat.serverId}-${selectedPrivateChat.username}`,
-            username: selectedPrivateChat.username,
-            isOnline: true,
-          },
-        ]
-      : []);
+    selectedChannel?.users || privateChatUsers;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 752e6c0 and 2b4e247.

📒 Files selected for processing (14)
  • .dockerignore (1 hunks)
  • compose.yaml (4 hunks)
  • docker/echobot.py (2 hunks)
  • src/components/layout/AppLayout.tsx (2 hunks)
  • src/components/layout/ChannelList.tsx (3 hunks)
  • src/components/layout/ChatArea.tsx (21 hunks)
  • src/components/layout/MemberList.tsx (5 hunks)
  • src/components/ui/AddPrivateChatModal.tsx (1 hunks)
  • src/components/ui/ClickableUsername.tsx (1 hunks)
  • src/components/ui/UserContextMenu.tsx (1 hunks)
  • src/lib/ircClient.ts (4 hunks)
  • src/store/index.ts (10 hunks)
  • src/types/index.ts (2 hunks)
  • tests/components/ChatArea.test.tsx (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
src/components/layout/ChannelList.tsx (1)
src/components/ui/AddPrivateChatModal.tsx (1)
  • AddPrivateChatModal (12-124)
src/store/index.ts (1)
src/types/index.ts (2)
  • PrivateChat (48-55)
  • User (1-9)
src/components/layout/MemberList.tsx (2)
src/types/index.ts (1)
  • User (1-9)
src/components/ui/UserContextMenu.tsx (1)
  • UserContextMenu (14-95)
src/components/layout/ChatArea.tsx (2)
src/lib/ircClient.ts (1)
  • ircClient (550-550)
src/components/ui/UserContextMenu.tsx (1)
  • UserContextMenu (14-95)
src/lib/ircClient.ts (1)
src/types/index.ts (1)
  • User (1-9)
🔇 Additional comments (19)
.dockerignore (1)

1-1: LGTM! Proper exclusion of build artifacts.

Excluding the Tauri target directory from Docker builds is a good practice to reduce image size and avoid including unnecessary build artifacts in the container.

compose.yaml (2)

8-8: LGTM! Consistent quote style across the file.

The change from double quotes to single quotes for port mappings and healthcheck test maintains consistency throughout the compose file without changing functionality.

Also applies to: 11-11, 23-24


44-44: Good improvement: Adding unbuffered Python output.

The -u flag ensures Python output is unbuffered, which is important for Docker containers to see real-time logs from the echobot services. This is especially valuable for debugging and monitoring bot behavior.

Also applies to: 62-62, 80-80

docker/echobot.py (1)

2-2: Import re: LGTM

Regex is necessary for mention detection; addition is appropriate.

src/components/layout/AppLayout.tsx (1)

29-30: LGTM! Good UX decision to hide member list during private chats.

The logic correctly hides the member list when a private chat is selected, which makes sense since private chats don't have a member list context. The comment clearly explains the intent.

src/lib/ircClient.ts (2)

207-210: LGTM! Clean implementation of typing indicators.

The typing indicator implementation follows IRC standards with proper TAGMSG usage and state management.


363-372: Simplified TAGMSG handling looks good.

The removal of channel-specific branching makes sense since TAGMSG can be used for both channels and private messages. The target parameter correctly identifies the destination.

src/components/layout/MemberList.tsx (2)

98-101: Good defensive check for current user.

The check prevents users from opening a context menu on their own username, which is appropriate UX.


162-170: Props mismatch with UserContextMenu interface.

The component is being passed individual x and y props, but according to the interface definition in UserContextMenu.tsx, these should be direct props, not nested in a position object.

The usage here matches the interface definition. My earlier concern about a position prop appears to be incorrect based on the actual implementation.

src/components/ui/UserContextMenu.tsx (1)

4-12: Incorrect — props interface matches usage. Call sites pass x and y as separate props (e.g. src/components/layout/ChatArea.tsx ~lines 1252–1260 and src/components/layout/MemberList.tsx ~lines 162–170), so there is no position object mismatch.

Likely an incorrect or invalid review comment.

tests/components/ChatArea.test.tsx (3)

12-12: LGTM! Mock properly extended for typing notifications.

The addition of sendTyping: vi.fn() to the mock is correct and aligns with the new private messaging functionality.


52-52: LGTM! Mock server model properly extended.

The addition of privateChats: [] to mockServer aligns with the new PrivateChat type model.


65-65: LGTM! UI state correctly initialized.

The addition of selectedPrivateChatId: null properly extends the UI state to support private chat selection.

src/store/index.ts (4)

358-358: LGTM! Consistent state clearing on navigation.

Good practice to clear selectedPrivateChatId when selecting a server or channel, preventing UI state confusion.

Also applies to: 412-412, 423-423


531-533: Prevent private chats with self.

Good defensive check to prevent users from opening private chats with themselves.


908-912: LGTM! Prevents echo messages in private chats.

The check correctly prevents the server from echoing our own private messages back to us, which would create duplicate messages in the UI.


1310-1340: Efficient handling of typing indicators for both channels and private chats.

The code correctly distinguishes between channel and private chat contexts and creates appropriate user objects for typing indicators.

src/components/layout/ChatArea.tsx (1)

914-916: Prevent context menu for own username.

Good check to prevent users from opening a context menu on their own username, avoiding confusion.

src/components/layout/ChannelList.tsx (1)

300-300: Approve — privateChats handling is safe

selectedServer is checked before this block and the codebase/types initialize or guard privateChats (types/index.ts declares privateChats: PrivateChat[] and lib/ircClient.ts initializes privateChats: []), so using selectedServer.privateChats?.map(...) is safe — no change required.

Comment thread docker/echobot.py
Comment on lines +48 to 53
if target == NICK:
# Private message
send(f"@+typing=active TAGMSG {sender}")
time.sleep(2)
send(f"PRIVMSG {sender} :You said: {message}")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

❓ Verification inconclusive

TAGMSG requires CAP negotiation; gate typing tags and make delay configurable

Unconditional TAGMSG can fail (421/unknown command) on servers without message-tags/typing; 2s sleep blocks PING handling under load.

-                    if target == NICK:
-                        # Private message
-                        send(f"@+typing=active TAGMSG {sender}")
-                        time.sleep(2)
-                        send(f"PRIVMSG {sender} :You said: {message}")
+                    if target == NICK:
+                        # Private message
+                        if TYPING_TAGS:
+                            send(f"@+typing=active TAGMSG {sender}")
+                        time.sleep(TYPING_DELAY)
+                        send(f"PRIVMSG {sender} :You said: {message}")

Add these near the top (outside this hunk) to configure behavior:

# config
TYPING_TAGS = os.environ.get("TYPING_TAGS", "0") == "1"
TYPING_DELAY = float(os.environ.get("TYPING_DELAY", "0.3"))

Optional (only if TYPING_TAGS is enabled): request capabilities after connect and before JOIN:

if TYPING_TAGS:
    send("CAP LS 302")
    send("CAP REQ :message-tags draft/typing")
    send("CAP END")

Please verify the target IRCd supports message-tags/typing when TYPING_TAGS=1.


Gate TAGMSG behind CAP and avoid blocking sleep; make typing delay configurable

File: docker/echobot.py Lines: 48-53

  • TAGMSG requires message-tags/typing CAP — do not send unconditionally; gate behind a config flag and request capabilities when enabled.
  • time.sleep(2) is blocking and can prevent PING handling under load — replace with a non-blocking timer or a configurable short delay.
-                    if target == NICK:
-                        # Private message
-                        send(f"@+typing=active TAGMSG {sender}")
-                        time.sleep(2)
-                        send(f"PRIVMSG {sender} :You said: {message}")
+                    if target == NICK:
+                        # Private message
+                        if TYPING_TAGS:
+                            send(f"@+typing=active TAGMSG {sender}")
+                        time.sleep(TYPING_DELAY)
+                        send(f"PRIVMSG {sender} :You said: {message}")

Add near the top (outside this hunk):

# config
TYPING_TAGS = os.environ.get("TYPING_TAGS", "0") == "1"
TYPING_DELAY = float(os.environ.get("TYPING_DELAY", "0.3"))

Optional (if TYPING_TAGS enabled): request capabilities after connect and before JOIN:

if TYPING_TAGS:
    send("CAP LS 302")
    send("CAP REQ :message-tags draft/typing")
    send("CAP END")

Verify the target IRCd supports message-tags/typing when TYPING_TAGS=1.

🤖 Prompt for AI Agents
docker/echobot.py lines 48-53: TAGMSG is being sent unconditionally and
time.sleep(2) blocks the main loop; add two config flags at module top
(TYPING_TAGS from env "0"/"1" and TYPING_DELAY as float from env, default ~0.3),
request the message-tags/draft/typing capabilities after connecting and before
JOIN only if TYPING_TAGS is true, gate sending the TAGMSG behind TYPING_TAGS so
you don't send tags when unsupported, and replace the blocking time.sleep with a
non-blocking mechanism (e.g., schedule the delayed PRIVMSG with threading.Timer
or an equivalent async timer so PING handling isn't blocked) while using the
configured TYPING_DELAY; also document that TYPING_TAGS must only be enabled if
the IRCd supports message-tags/typing.

Comment on lines 594 to 619
ircClient.sendRaw(
selectedServerId,
`${localReplyTo ? `@+draft/reply=${localReplyTo.id};` : ""} PRIVMSG ${selectedChannel?.name ?? ""} :${formattedText}`,
`${localReplyTo ? `@+draft/reply=${localReplyTo.id};` : ""} PRIVMSG ${target} :${formattedText}`,
);

// For private messages, manually add our own message to the chat
// since the server doesn't echo private messages back to us
if (selectedPrivateChat && currentUser) {
const outgoingMessage = {
id: uuidv4(),
content: messageText,
timestamp: new Date(),
userId: currentUser.username || currentUser.id,
channelId: selectedPrivateChat.id,
serverId: selectedServerId,
type: "message" as const,
reacts: [],
replyMessage: localReplyTo,
mentioned: [],
};

// Add the message to the store
const { addMessage } = useStore.getState();
addMessage(outgoingMessage);
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Critical: Fix inconsistent message ID handling for private messages.

The code doesn't use the formatting (color/style) when adding outgoing private messages to the store, creating inconsistency with channel messages. The formatted text with color codes is sent to the server but not stored locally.

         // For private messages, manually add our own message to the chat
         // since the server doesn't echo private messages back to us
         if (selectedPrivateChat && currentUser) {
           const outgoingMessage = {
             id: uuidv4(),
-            content: messageText,
+            content: formattedText,  // Use formatted text with colors/styles
             timestamp: new Date(),
             userId: currentUser.username || currentUser.id,
             channelId: selectedPrivateChat.id,
             serverId: selectedServerId,
             type: "message" as const,
             reacts: [],
             replyMessage: localReplyTo,
             mentioned: [],
           };

           // Add the message to the store
           const { addMessage } = useStore.getState();
           addMessage(outgoingMessage);
         }
📝 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
ircClient.sendRaw(
selectedServerId,
`${localReplyTo ? `@+draft/reply=${localReplyTo.id};` : ""} PRIVMSG ${selectedChannel?.name ?? ""} :${formattedText}`,
`${localReplyTo ? `@+draft/reply=${localReplyTo.id};` : ""} PRIVMSG ${target} :${formattedText}`,
);
// For private messages, manually add our own message to the chat
// since the server doesn't echo private messages back to us
if (selectedPrivateChat && currentUser) {
const outgoingMessage = {
id: uuidv4(),
content: messageText,
timestamp: new Date(),
userId: currentUser.username || currentUser.id,
channelId: selectedPrivateChat.id,
serverId: selectedServerId,
type: "message" as const,
reacts: [],
replyMessage: localReplyTo,
mentioned: [],
};
// Add the message to the store
const { addMessage } = useStore.getState();
addMessage(outgoingMessage);
}
}
ircClient.sendRaw(
selectedServerId,
`${localReplyTo ? `@+draft/reply=${localReplyTo.id};` : ""} PRIVMSG ${target} :${formattedText}`,
);
// For private messages, manually add our own message to the chat
// since the server doesn't echo private messages back to us
if (selectedPrivateChat && currentUser) {
const outgoingMessage = {
id: uuidv4(),
content: formattedText, // Use formatted text with colors/styles
timestamp: new Date(),
userId: currentUser.username || currentUser.id,
channelId: selectedPrivateChat.id,
serverId: selectedServerId,
type: "message" as const,
reacts: [],
replyMessage: localReplyTo,
mentioned: [],
};
// Add the message to the store
const { addMessage } = useStore.getState();
addMessage(outgoingMessage);
}
}
🤖 Prompt for AI Agents
In src/components/layout/ChatArea.tsx around lines 594 to 619, the outgoing
private message stored locally uses messageText (plain text) while the server
receives formattedText (with color/style codes); update the outgoingMessage
construction to use the same formattedText as the content (and keep the existing
uuidv4 id, timestamp, user/channel/server fields and reply/mention data) so the
local store reflects the exact formatted message sent to the server.

@ValwareIRC
Copy link
Copy Markdown
Contributor

Amazing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add private messages under voice channels

2 participants