Add IRCv3 METADATA, including avatars, nick colors, display name and status message#64
Conversation
- Add comprehensive IRC metadata protocol support according to draft/metadata-2 specification - Implement capability negotiation for all metadata variants (draft/metadata-2, draft/metadata-notify-2, draft/metadata=maxsub=10) - Add metadata state management with subscriptions and batch processing - Add conditional metadata UI in User Settings (avatar, display name, homepage, status, color) - Display metadata in member lists and user context menus - Add proper error handling and null-safe operations - Update tests to handle metadata functionality - Fix capability detection to support all metadata variants
- Add logging to metadataSet calls in store and IRC client - Add logging for outgoing METADATA commands in sendRaw - Add logging for incoming METADATA responses and FAIL messages - Add logging for METADATA_KEYNOTSET responses - Log all metadata operations to help debug permission issues
- Update MemberList to show metadata avatar instead of first initial - Update ChatArea message avatars to use metadata avatar - Update ChannelList user avatar to use metadata avatar - Add fallback to first initial if avatar image fails to load - Add proper error handling for broken avatar URLs - All avatar displays now use user.metadata.avatar.value
Replace non-null assertions with proper null checks in avatar image onError handlers across MemberList, ChatArea, and ChannelList components. This resolves lint warnings while maintaining the same fallback behavior.
The METADATA_KEYVALUE event handler was missing current user metadata updates, causing avatars to not display for the current user even when successfully set on the server. Added current user metadata update logic similar to the METADATA handler, and added logging for debugging.
- Add 'avatar' to default metadata subscription keys so avatar updates are received automatically - Request metadata for users when they join channels (JOIN event) - Request metadata for all users listed in NAMES replies - This ensures avatars are displayed for users who join after avatars have been set, and vice versa
- Add color metadata support: usernames colored in member list and chat - Add status metadata with lightbulb icon on avatars that expands on hover - Add display name metadata: replaces nick in messages with username badge - Subscribe to color, displayName, and status metadata keys - Add getColorStyle utility for handling color values
The IRC metadata standard uses 'display-name' (with hyphen) not 'displayName' (camelCase). Updated the code to use the correct key.
WalkthroughAdds end-to-end client-side IRC metadata support and UI integration: protocol CAP/METADATA handling and events, store persistence and metadata APIs, types for metadata/capabilities, UI surfaces (avatars, display-name, color, status, website) across ChannelList/ChatArea/MemberList/ContextMenu/UserSettings, tests, and App auto-reconnect. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as Client UI
participant Store
participant IRC as IRCClient
participant Server as IRCd
rect rgba(220,235,255,0.45)
note over IRC,Server: CAP negotiation for metadata capability
IRC->>Server: CAP REQ :draft/metadata-2
Server-->>IRC: CAP ACK :draft/metadata-2
IRC->>Store: emit CAP_ACKNOWLEDGED(capabilities)
end
rect rgba(220,255,220,0.45)
note over Store,Server: Subscribe / GET metadata
Store->>IRC: metadataSub/metadataGet
IRC->>Server: METADATA SUB / GET ...
Server-->>IRC: 760..774 / SUBOK / KEYVALUE
IRC->>Store: emit METADATA / METADATA_KEYVALUE / SUBOK
Store-->>UI: update entities with metadata
end
rect rgba(255,245,200,0.45)
note over UI,Store: Render with metadata
UI->>Store: select user
Store-->>UI: user { metadata: { avatar, display-name, color, status, url } }
UI->>UI: compute getColorStyle(color)
UI->>UI: render avatar img (onError -> show initial), name, badge, status, website
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
|
@matheusfillipe Please merge if approved |
|
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ui/UserSettings.tsx (1)
202-215: Confusing duplicate "Status" labels for different features.There are two fields labeled "Status":
- Line 146-148: IRCv3 metadata status (free text like "Working from home")
- Line 203-205: Client presence status (online/idle/dnd/invisible)
This creates a confusing UX. Consider renaming one of them:
- Rename the metadata field to "Status Message" or "Status Text"
- Rename the dropdown to "Presence" or "Availability"
- <label className="block text-discord-text-muted text-sm font-medium mb-1"> - Status - </label> + <label className="block text-discord-text-muted text-sm font-medium mb-1"> + Status Message + </label>And for the dropdown:
- <label className="block text-discord-text-muted text-sm font-medium mb-1"> - Status - </label> + <label className="block text-discord-text-muted text-sm font-medium mb-1"> + Presence + </label>
🧹 Nitpick comments (7)
src/components/ui/UserSettings.tsx (1)
35-44: Consider removing unusedhandleSaveMetadatafunction.This helper function appears to be unused as
handleSaveAllhandles all metadata updates directly. Consider removing it to reduce code clutter.src/lib/ircClient.ts (3)
195-199: Good selective logging approach.The selective logging for METADATA commands is a good security practice. Consider extending this pattern to other non-sensitive debug commands.
Consider creating a whitelist of commands safe to log:
const SAFE_TO_LOG = ['METADATA', 'CAP', 'JOIN', 'PART', 'NICK']; if (SAFE_TO_LOG.some(cmd => command.startsWith(cmd))) { console.log(`[IRC] Sending: ${command}`); }
262-274: Remove redundant logging inmetadataSet.Line 272 logs the METADATA SET command, but this is redundant since
sendRawalready logs METADATA commands (Line 196-198).- console.log(`[IRC] Sending metadata SET command: ${command}`); this.sendRaw(serverId, command);
576-602: Good defensive handling of malformed server responses.The code handles a server bug that sends duplicate targets. Consider adding a comment specifying which IRC server implementation exhibits this behavior for future reference.
- // If target is duplicated (server bug), skip the duplicate + // If target is duplicated (server bug in [specify server]), skip the duplicatesrc/components/layout/ChatArea.tsx (2)
268-300: Consider improving avatar error handling and reducing duplication.The current error handling modifies the parent element directly, which isn't ideal in React. Also, the avatar rendering logic is duplicated between ACTION messages and regular messages.
Consider extracting the avatar rendering into a reusable component and using React state for error handling:
+const AvatarWithFallback: React.FC<{ + avatarUrl?: string; + username: string; + userStatus?: string; + className?: string; +}> = ({ avatarUrl, username, userStatus, className }) => { + const [imageError, setImageError] = useState(false); + const initial = username.charAt(0).toUpperCase(); + + return ( + <div className={`relative ${className || ''}`}> + {avatarUrl && !imageError ? ( + <img + src={avatarUrl} + alt={username} + className="w-8 h-8 rounded-full object-cover" + onError={() => setImageError(true)} + /> + ) : ( + <div className="w-8 h-8 rounded-full bg-discord-dark-400 flex items-center justify-center text-white"> + {initial} + </div> + )} + {userStatus && ( + <div className="absolute -bottom-1 -left-1 bg-discord-dark-600 rounded-full p-1 group"> + <div className="w-3 h-3 bg-yellow-400 rounded-full flex items-center justify-center"> + <span className="text-xs">💡</span> + </div> + <div className="absolute bottom-full left-0 mb-2 hidden group-hover:block"> + <div className="bg-discord-dark-600 text-white text-xs px-2 py-1 rounded whitespace-nowrap"> + {userStatus} + </div> + </div> + </div> + )} + </div> + ); +};Then use it in both places:
<AvatarWithFallback avatarUrl={avatarUrl} username={message.userId.split("-")[0]} userStatus={userStatus} className="w-8 h-8" />Also applies to: 349-379
311-316: Unify display name rendering between message types.The display name rendering differs between ACTION messages (concatenation) and regular messages (badge style). Consider using consistent formatting.
For ACTION messages (line 311-313), consider using the same badge style as regular messages:
- : (displayName || message.userId.split("-")[0]) + - (displayName ? ` (${message.userId.split("-")[0]})` : "") + + : displayName || message.userId.split("-")[0]} + {displayName && ( + <span className="ml-2 text-xs bg-discord-dark-600 px-1 py-0.5 rounded"> + {message.userId.split("-")[0]} + </span> + )} + <span className="italic"> + { message.content.substring(7, message.content.length - 1)} + </span>Also applies to: 409-416
src/store/index.ts (1)
1623-1625: Consider making debug logging configurable.The console.log statements for metadata events should be controllable via a debug flag or logging level.
+const DEBUG_METADATA = process.env.NODE_ENV === 'development'; + ircClient.on("METADATA", ({ serverId, target, key, visibility, value }) => { - console.log( - `[METADATA] Received metadata: server=${serverId}, target=${target}, key=${key}, value=${value}, visibility=${visibility}`, - ); + if (DEBUG_METADATA) { + console.log( + `[METADATA] Received metadata: server=${serverId}, target=${target}, key=${key}, value=${value}, visibility=${visibility}`, + ); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/components/layout/ChannelList.tsx(1 hunks)src/components/layout/ChatArea.tsx(9 hunks)src/components/layout/MemberList.tsx(3 hunks)src/components/ui/UserContextMenu.tsx(3 hunks)src/components/ui/UserSettings.tsx(3 hunks)src/lib/ircClient.ts(5 hunks)src/lib/ircUtils.tsx(1 hunks)src/store/index.ts(8 hunks)src/types/index.ts(3 hunks)tests/App.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/components/ui/UserSettings.tsx (1)
src/lib/ircClient.ts (1)
metadataSet(262-274)
src/components/layout/MemberList.tsx (1)
src/lib/ircUtils.tsx (1)
getColorStyle(282-302)
src/components/layout/ChatArea.tsx (2)
src/types/index.ts (1)
User(1-10)src/lib/ircUtils.tsx (1)
getColorStyle(282-302)
src/store/index.ts (1)
src/lib/ircClient.ts (1)
ircClient(759-759)
🔇 Additional comments (28)
tests/App.test.tsx (1)
139-148: LGTM: Improved test selector for modal-specific Cancel button.The updated approach properly handles multiple Cancel buttons by first trying to find the one within the User Settings modal using data-testid, then falling back to styling/class criteria, and finally using the second button as a fallback. This is a robust solution for targeting the correct Cancel button in the presence of multiple modals.
src/lib/ircUtils.tsx (1)
281-302: LGTM: Well-implemented color utility function.The
getColorStylefunction correctly handles the expected color formats:
- Hex colors (starting with #)
- Named CSS colors (letters only)
- RGB/RGBA values (starting with "rgb")
The fallback behavior is appropriate, returning an empty object for invalid/missing values. This provides a clean interface for components to apply dynamic colors from metadata.
src/components/layout/MemberList.tsx (4)
5-5: LGTM: Proper import of color styling utility.Correctly imports the
getColorStyleutility function for consistent color handling across components.
62-66: LGTM: Clean metadata extraction pattern.The metadata extraction correctly accesses nested properties with proper optional chaining. The fallback from
urltowebsitefor the website value provides good flexibility.
80-109: Well-implemented avatar with comprehensive fallback handling.The avatar implementation includes:
- Proper error handling with
onErrorcallback- Graceful fallback to user initial when image fails to load
- Status indicator with hover tooltip for enhanced UX
- Clean visual hierarchy with proper positioning
The error handling logic correctly hides the broken image and displays the fallback initial in the parent container.
111-132: Enhanced user information display with metadata integration.The layout effectively displays:
- Username with dynamic color styling using
getColorStyle- IRC status badges (channel privileges)
- User status from metadata with proper truncation
- Website links with visual emoji indicator
The information hierarchy is clear and the truncation prevents layout issues.
src/components/ui/UserContextMenu.tsx (3)
3-3: LGTM: Appropriate store import for metadata access.Correctly imports the store to access user metadata information.
34-44: LGTM: Comprehensive user resolution with metadata extraction.The implementation correctly:
- Finds the server by serverId from the store
- Searches for the user across both channel users and server users (good coverage)
- Extracts metadata using the same pattern as other components (url/website fallback)
The user resolution logic handles the case where users might be in different locations within the server data structure.
139-148: LGTM: Consistent metadata display in context menu.The status and website display follows the same patterns established in MemberList:
- Conditional rendering based on metadata presence
- Globe emoji prefix for website URLs
- Proper text styling and layout
This maintains UI consistency across components.
src/components/layout/ChannelList.tsx (1)
367-387: LGTM: Consistent avatar implementation with metadata migration.The avatar implementation correctly:
- Migrates from
currentUser.avatartocurrentUser.metadata.avatar.value- Adds
object-coverclass for proper image scaling- Implements comprehensive error handling with fallback to user initial
- Maintains the same fallback pattern used in MemberList component
The error handling logic properly hides broken images and displays the user's initial as a fallback, ensuring consistent user experience across components.
src/types/index.ts (3)
9-9: LGTM! Clean metadata type addition to User interface.The optional metadata field follows the existing pattern and correctly models the IRCv3 metadata structure.
22-23: LGTM! Appropriate Server interface enhancements.The capabilities array and metadata field are well-structured additions that support IRCv3 CAP negotiation and server-level metadata.
48-48: LGTM! Consistent metadata addition to Channel interface.The metadata field maintains consistency across all entity types.
src/components/ui/UserSettings.tsx (4)
7-13: LGTM! Proper store integration and capability detection.Good use of optional chaining for safe server lookup and appropriate use of
startsWithto handle different draft/metadata versions.
15-33: LGTM! Well-structured metadata state management.Good defensive programming with optional chaining and proper fallback values. Correct use of bracket notation for
display-namekey.
104-192: LGTM! Well-structured metadata UI with proper feature gating.Good UX design with the color picker combo and helpful placeholders. The conditional rendering based on
supportsMetadataprovides clear user feedback.
230-247: LGTM! Excellent dynamic button labeling.The save button provides clear feedback about what will be saved based on the current context. Good accessibility with the disabled state.
src/lib/ircClient.ts (4)
62-97: LGTM! Comprehensive METADATA event type definitions.The event types properly cover all METADATA operations including batch support for metadata-notify-2. Well-structured with appropriate payload types.
547-561: LGTM! Correct METADATA command parsing.The parsing correctly handles the METADATA command format and properly strips the leading colon from values.
634-657: LGTM! Robust FAIL METADATA handling.Excellent handling of the FAIL command with proper RATE_LIMITED support and optional parameter parsing.
681-688: LGTM! Flexible metadata capability negotiation.Good approach using
startsWith("draft/metadata")to support different draft versions of the metadata specification.src/components/layout/ChatArea.tsx (4)
32-32: LGTM!The import of
getColorStyleis properly utilized for applying dynamic color styles to usernames based on metadata.
193-205: LGTM!The addition of the
usersprop to the MessageItem component is well-designed for metadata resolution.
209-216: LGTM!The metadata extraction logic properly uses optional chaining for safe access to user metadata properties.
1440-1440: LGTM!The users prop is properly forwarded with safe fallback to empty array when no channel is selected.
src/store/index.ts (3)
74-87: LGTM!The metadata state structure is well-designed with proper type definitions for managing subscriptions and batches.
Also applies to: 171-172
1171-1178: LGTM!The metadata request for joining users is appropriately implemented with a reasonable delay.
1851-1865: LGTM!The automatic subscription to common metadata keys when the capability is acknowledged provides good default functionality.
| metadataSet: (serverId, target, key, value) => { | ||
| console.log( | ||
| `[METADATA] Setting metadata: server=${serverId}, target=${target}, key=${key}, value=${value}`, | ||
| ); | ||
| ircClient.metadataSet(serverId, target, key, value); | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove or improve debug logging.
The console.log statement should be removed or replaced with a proper logging mechanism that respects log levels.
metadataSet: (serverId, target, key, value) => {
- console.log(
- `[METADATA] Setting metadata: server=${serverId}, target=${target}, key=${key}, value=${value}`,
- );
ircClient.metadataSet(serverId, target, key, value);
},📝 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.
| metadataSet: (serverId, target, key, value) => { | |
| console.log( | |
| `[METADATA] Setting metadata: server=${serverId}, target=${target}, key=${key}, value=${value}`, | |
| ); | |
| ircClient.metadataSet(serverId, target, key, value); | |
| }, | |
| metadataSet: (serverId, target, key, value) => { | |
| ircClient.metadataSet(serverId, target, key, value); | |
| }, |
🤖 Prompt for AI Agents
In src/store/index.ts around lines 874 to 879, remove the console.log debug
statement and replace it with the project's logger at an appropriate level
(e.g., logger.debug or logger.trace) so logging respects configured log levels;
import or use the existing logging utility used elsewhere in the codebase, log
the same contextual message via that logger, and ensure the metadataSet call
remains unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/components/MetadataDisplay.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/components/MetadataDisplay.test.tsx (4)
src/types/index.ts (3)
User(1-10)Channel(37-49)Server(12-24)src/components/layout/MemberList.tsx (1)
MemberList(137-281)src/components/layout/ChatArea.tsx (1)
ChatArea(582-1612)src/lib/ircUtils.tsx (1)
getColorStyle(282-302)
🪛 GitHub Actions: Lint and Tests
tests/components/MetadataDisplay.test.tsx
[error] 1-1: Biome: File content differs from formatting output.
[error] 1-1: The imports and exports are not sorted. (Biome: Organize Imports)
[error] 1-1: Several of these imports are unused. (lint/correctness/noUnusedImports)
[error] 1-1: File content differs from formatting output (format mismatch).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/App.tsx(1 hunks)src/store/index.ts(11 hunks)tests/components/MetadataDisplay.test.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/components/MetadataDisplay.test.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/App.tsx (1)
src/lib/ircClient.ts (1)
joinChannel(205-229)
src/store/index.ts (2)
src/types/index.ts (1)
ServerConfig(25-35)src/lib/ircClient.ts (1)
ircClient(759-759)
🪛 GitHub Actions: Lint and Tests
src/store/index.ts
[error] 16-17: File content differs from formatting output. Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
🔇 Additional comments (2)
src/store/index.ts (2)
943-947: Swap debugconsole.logfor the project loggerPer the earlier review, this console log should either be dropped or routed through the project’s structured logger to respect log levels. Please update it accordingly.
1172-1181: Batch metadata lookups instead of 200 ms per userThis per-user
setTimeout(index * 200)loop was previously flagged; the bug remains. Large channels (e.g., 200 members) still trigger 200 serial requests over 40 s and risk hitting server rate limits. Please replace this with a batched/queued strategy (e.g., process N users per interval or reuse the new metadata batch support) so metadata hydration completes promptly without flooding the server.
…idianirc/obsidianirc into feat/new-feature-development
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/store/index.ts(11 hunks)tests/components/MetadataDisplay.test.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/store/index.ts (2)
src/types/index.ts (1)
ServerConfig(25-35)src/lib/ircClient.ts (1)
ircClient(759-759)
tests/components/MetadataDisplay.test.tsx (4)
src/types/index.ts (3)
User(1-10)Channel(37-49)Server(12-24)src/components/layout/MemberList.tsx (1)
MemberList(137-281)src/components/layout/ChatArea.tsx (1)
ChatArea(582-1612)src/lib/ircUtils.tsx (1)
getColorStyle(282-302)
🪛 GitHub Actions: Lint and Tests
src/store/index.ts
[error] 1174-1178: Expected a semicolon or an implicit semicolon after a statement, but found none
[error] 1176-1177: The file contains an invalid export declaration not at the top level (Illegal use of an export declaration not at the top level)
[error] 1998-2001: Illegal use of an export declaration not at the top level
[error] 2000-2001: Code formatting aborted due to parsing errors. To format code with errors, enable the 'formatter.formatWithErrors' option.
🪛 GitHub Actions: Deploy to Cloudflare Pages
src/store/index.ts
[error] 1176-1176: TypeScript error TS1109: Expression expected.
[error] 1180-1180: TypeScript error TS1109: Expression expected.
[error] 1181-1181: TypeScript error TS1109: Expression expected.
[error] 2001-2001: TypeScript error TS1005: '}' expected.
[error] Build failed due to TypeScript compilation errors.
🔇 Additional comments (3)
src/store/index.ts (3)
943-947: Drop the debugconsole.log.This
console.log(Line 944) reintroduces the Biome failure we discussed earlier; please remove it or route it through the project’s logger instead so CI stays green.- console.log( - `[METADATA] Setting metadata: server=${serverId}, target=${target}, key=${key}, value=${value}`, - ); ircClient.metadataSet(serverId, target, key, value);
1714-1751: Handle channel targets in live METADATA updates.Line 1718 updates users but silently ignores
target === channel.name. When the server broadcastsMETADATA #channel …, the UI never reflects it even though we persist it. Mirror the channel logic you added for list responses so real-time updates land.- const updatedChannels = server.channels.map((channel) => { - const updatedUsers = channel.users.map((user) => { + const updatedChannels = server.channels.map((channel) => { + const updatedUsers = channel.users.map((user) => { if (user.username === target) { const metadata = user.metadata || {}; if (value) { metadata[key] = { value, visibility }; } else { delete metadata[key]; } return { ...user, metadata }; } return user; }); - return { ...channel, users: updatedUsers }; + if (channel.name === target) { + const nextMetadata = { ...(channel.metadata ?? {}) }; + if (value) { + nextMetadata[key] = { value, visibility }; + } else { + delete nextMetadata[key]; + } + return { + ...channel, + users: updatedUsers, + metadata: + Object.keys(nextMetadata).length > 0 ? nextMetadata : undefined, + }; + } + return { ...channel, users: updatedUsers }; });
68-87: Restore channel-level metadata on reconnect.Line 80 only repopulates user metadata. Any channel metadata saved for
#channeltargets is dropped on restart, so display names/avatars vanish until the server resends them. Merge the saved channel entry back into each channel during restoration.- const updatedChannels = server.channels.map((channel) => { - const updatedUsers = channel.users.map((user) => { + const updatedChannels = server.channels.map((channel) => { + const updatedUsers = channel.users.map((user) => { const userMetadata = serverMetadata[user.username]; if (userMetadata) { return { ...user, metadata: { ...user.metadata, ...userMetadata }, }; } return user; }); - return { ...channel, users: updatedUsers }; + const channelMetadata = serverMetadata[channel.name]; + if (channelMetadata) { + return { + ...channel, + users: updatedUsers, + metadata: { + ...(channel.metadata ?? {}), + ...channelMetadata, + }, + }; + } + return { ...channel, users: updatedUsers }; });
| capAck: (serverId, key, capabilities) => { | ||
| ircClient.capAck(serverId, key, capabilities); | ||
| }, | ||
| })); |
There was a problem hiding this comment.
Close the create() call.
The object literal ends on Line 973 with });, but we never close the outer create<AppState>(...) call. TypeScript therefore thinks everything that follows—event handlers and the default export—is inside the store initializer, yielding the “Illegal use of an export declaration” and '} expected` errors in CI. Restore the missing paren so the file parses again.
-});
+}));📝 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.
| capAck: (serverId, key, capabilities) => { | |
| ircClient.capAck(serverId, key, capabilities); | |
| }, | |
| })); | |
| capAck: (serverId, key, capabilities) => { | |
| ircClient.capAck(serverId, key, capabilities); | |
| }, | |
| })); |
🤖 Prompt for AI Agents
In src/store/index.ts around lines 970 to 973, the store initializer ends with
"});" but is missing the closing parenthesis for the outer create<AppState>(...)
call; add the missing ')' so the call becomes create<AppState>( { ... } );
(i.e., insert a closing paren before the semicolon) so the object literal is
closed and the rest of the file (event handlers and default export) is outside
the create call.
| const aliceStatusIndicator = screen | ||
| .getByText("alice") | ||
| .closest("div") | ||
| ?.querySelector(".group"); | ||
|
|
||
| if (aliceStatusIndicator) { | ||
| // Simulate hover | ||
| fireEvent.mouseEnter(aliceStatusIndicator); | ||
|
|
||
| // Status text should be visible | ||
| const statusText = screen.getByText("Working on something cool!"); | ||
| expect(statusText).toBeInTheDocument(); | ||
| } |
There was a problem hiding this comment.
Make the tooltip test fail when the indicator is missing.
Because everything is wrapped in an if, the test succeeds even when the status indicator cannot be found, so any regression slips through unnoticed. Force the presence of the element before continuing and interact with it directly so the test actually exercises the UI.
- if (aliceStatusIndicator) {
- // Simulate hover
- fireEvent.mouseEnter(aliceStatusIndicator);
-
- // Status text should be visible
- const statusText = screen.getByText("Working on something cool!");
- expect(statusText).toBeInTheDocument();
- }
+ expect(aliceStatusIndicator).toBeTruthy();
+ // Simulate hover
+ fireEvent.mouseEnter(aliceStatusIndicator as HTMLElement);
+
+ // Status text should be visible
+ const statusText = screen.getByText("Working on something cool!");
+ expect(statusText).toBeInTheDocument();📝 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 aliceStatusIndicator = screen | |
| .getByText("alice") | |
| .closest("div") | |
| ?.querySelector(".group"); | |
| if (aliceStatusIndicator) { | |
| // Simulate hover | |
| fireEvent.mouseEnter(aliceStatusIndicator); | |
| // Status text should be visible | |
| const statusText = screen.getByText("Working on something cool!"); | |
| expect(statusText).toBeInTheDocument(); | |
| } | |
| const aliceStatusIndicator = screen | |
| .getByText("alice") | |
| .closest("div") | |
| ?.querySelector(".group"); | |
| expect(aliceStatusIndicator).toBeTruthy(); | |
| // Simulate hover | |
| fireEvent.mouseEnter(aliceStatusIndicator as HTMLElement); | |
| // Status text should be visible | |
| const statusText = screen.getByText("Working on something cool!"); | |
| expect(statusText).toBeInTheDocument(); |
🤖 Prompt for AI Agents
In tests/components/MetadataDisplay.test.tsx around lines 189–201, the test
currently wraps the tooltip interaction in an `if` so it silently passes when
the status indicator is missing; replace the silent conditional with a hard
assertion that the indicator exists and then interact with it directly (e.g.,
obtain the element deterministically instead of optional chaining, assert it's
not null/undefined, call fireEvent.mouseEnter on it, then assert the tooltip
text is in the document) so the test fails if the indicator cannot be found.
| it("should show status tooltip on avatar hover", () => { | ||
| render(<ChatArea onToggleChanList={() => {}} isChanListVisible={true} />); | ||
|
|
||
| // Find message avatar containers | ||
| const avatars = screen.getAllByRole("img"); | ||
|
|
||
| if (avatars.length > 0) { | ||
| // Find the container with group class (has hover functionality) | ||
| const avatarContainer = avatars[0].closest(".group"); | ||
|
|
||
| if (avatarContainer) { | ||
| fireEvent.mouseEnter(avatarContainer); | ||
|
|
||
| // Status should be visible | ||
| const statusText = screen.queryByText("Working on something cool!"); | ||
| // Note: This might not work in test environment due to CSS hover simulation | ||
| // but the structure should be correct | ||
| } |
There was a problem hiding this comment.
Add real assertions to the avatar hover test.
Right now the if blocks swallow every failure, so the spec passes even when the tooltip never appears. Make the expectations unconditional so we actually verify the hover behaviour.
- if (avatars.length > 0) {
- // Find the container with group class (has hover functionality)
- const avatarContainer = avatars[0].closest(".group");
-
- if (avatarContainer) {
- fireEvent.mouseEnter(avatarContainer);
-
- // Status should be visible
- const statusText = screen.queryByText("Working on something cool!");
- // Note: This might not work in test environment due to CSS hover simulation
- // but the structure should be correct
- }
- }
+ expect(avatars.length).toBeGreaterThan(0);
+ const avatarContainer = avatars[0].closest(".group");
+ expect(avatarContainer).toBeTruthy();
+
+ fireEvent.mouseEnter(avatarContainer as HTMLElement);
+
+ // Status text should be visible (even if CSS controls the actual hover effect)
+ expect(screen.getByText("Working on something cool!")).toBeInTheDocument();📝 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.
| it("should show status tooltip on avatar hover", () => { | |
| render(<ChatArea onToggleChanList={() => {}} isChanListVisible={true} />); | |
| // Find message avatar containers | |
| const avatars = screen.getAllByRole("img"); | |
| if (avatars.length > 0) { | |
| // Find the container with group class (has hover functionality) | |
| const avatarContainer = avatars[0].closest(".group"); | |
| if (avatarContainer) { | |
| fireEvent.mouseEnter(avatarContainer); | |
| // Status should be visible | |
| const statusText = screen.queryByText("Working on something cool!"); | |
| // Note: This might not work in test environment due to CSS hover simulation | |
| // but the structure should be correct | |
| } | |
| it("should show status tooltip on avatar hover", () => { | |
| render(<ChatArea onToggleChanList={() => {}} isChanListVisible={true} />); | |
| // Find message avatar containers | |
| const avatars = screen.getAllByRole("img"); | |
| expect(avatars.length).toBeGreaterThan(0); | |
| const avatarContainer = avatars[0].closest(".group"); | |
| expect(avatarContainer).toBeTruthy(); | |
| fireEvent.mouseEnter(avatarContainer as HTMLElement); | |
| // Status text should be visible (even if CSS controls the actual hover effect) | |
| expect(screen.getByText("Working on something cool!")).toBeInTheDocument(); | |
| }); |
🤖 Prompt for AI Agents
In tests/components/MetadataDisplay.test.tsx around lines 274 to 291, the avatar
hover test currently wraps lookups and assertions in conditional if blocks which
suppress failures; make the assertions unconditional and fail the test when
elements are missing. Replace the optional checks by directly selecting the
avatar container (use getAllByRole/getByRole or
expect(screen.getAllByRole("img").length).toBeGreaterThan(0)), assert the
container exists, fireEvent.mouseEnter on the container, then assert the tooltip
text appears (use getByText or await findByText and an explicit
expect(...).toBeInTheDocument()). Remove all if branches so missing elements or
absent tooltip cause test failures.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/ui/AddPrivateChatModal.tsx(1 hunks)src/components/ui/AddServerModal.tsx(1 hunks)src/components/ui/ReactionModal.tsx(1 hunks)src/components/ui/UserSettings.tsx(3 hunks)src/store/index.ts(11 hunks)tests/components/MetadataDisplay.test.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/ui/AddServerModal.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/ui/UserSettings.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/store/index.ts (2)
src/types/index.ts (1)
ServerConfig(25-35)src/lib/ircClient.ts (1)
ircClient(759-759)
tests/components/MetadataDisplay.test.tsx (4)
src/types/index.ts (3)
User(1-10)Channel(37-49)Server(12-24)src/components/layout/MemberList.tsx (1)
MemberList(137-281)src/components/layout/ChatArea.tsx (1)
ChatArea(582-1612)src/lib/ircUtils.tsx (1)
getColorStyle(282-302)
🔇 Additional comments (4)
src/components/ui/AddPrivateChatModal.tsx (1)
58-58: Modal height guard looks goodClamping the modal container to 90vh while enabling
overflow-y-autogives it a predictable max size and only introduces a scrollbar when the content actually overflows, which is the recommended Tailwind pattern for scrollable panels.(v3.tailwindcss.com)tests/components/MetadataDisplay.test.tsx (3)
158-159: Restore Bob’s color assertion to the literal value the component renders.
getColorStylefeeds React the string"blue", so JSDOM keeps the style attribute ascolor: blue. Expectingrgb(0, 0, 255)still fails under Vitest/Testing Library. Please assert{ color: "blue" }so the spec matches the rendered inline style.
189-201: Make the tooltip test fail when the indicator is missing.Wrapping the hover logic in an
ifturns every failure into a silent pass. Grab the element with aget*query (or assert it’s truthy) and runfireEvent.mouseEnterso the test genuinely checks the tooltip.
274-291: Remove the guards so avatar hover actually asserts the status text.The nested
ifblocks let the test succeed even when no avatar or tooltip renders. Assert the array has items, require the container, trigger hover, then expect the status text.
| const currentState = useStore.getState(); | ||
| const currentUser = currentState.currentUser; | ||
| users.forEach((user, index) => { | ||
| if (currentUser && user.username !== currentUser.username) { | ||
| // Stagger requests to avoid overwhelming the server | ||
| setTimeout(() => { | ||
| useStore.getState().metadataList(serverId, user.username); | ||
| }, index * 200); // 200ms delay between requests | ||
| } | ||
| }); | ||
| const usersToFetch = users.filter( | ||
| (u) => u.username !== currentUser?.username, | ||
| ); | ||
|
|
||
| // Process in batches with shorter delays | ||
| const batchSize = 10; | ||
| const batchDelay = 500; // 500ms between batches | ||
|
|
||
| for (let i = 0; i < usersToFetch.length; i += batchSize) { | ||
| const batch = usersToFetch.slice(i, i + batchSize); | ||
| setTimeout( | ||
| () => { | ||
| batch.forEach((user, idx) => { | ||
| setTimeout(() => { | ||
| useStore.getState().metadataList(serverId, user.username); | ||
| }, idx * 50); // 50ms between requests in a batch | ||
| }); | ||
| }, | ||
| Math.floor(i / batchSize) * batchDelay, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Don’t fire metadataList twice per user.
We still run the original users.forEach(... metadataList ...) and then the new batched loop, so every user gets two LIST requests. That doubles network load and defeats the throttling you just added. Please drop one of the loops (keep the batched version).
🤖 Prompt for AI Agents
In src/store/index.ts around lines 1193 to 1223, the code currently calls
metadataList twice (first in the initial users.forEach staggered loop and again
in the new batched loop); remove the original users.forEach(...) block so only
the batched processing runs. Ensure the batched loop operates on usersToFetch
(already filtered to exclude currentUser) and retains the per-batch and
per-request delays as implemented, so each user triggers exactly one
metadataList call.
| const channelMetadata = channel.metadata || {}; | ||
| if (target === channel.name || target.startsWith("#")) { | ||
| if (value) { | ||
| channelMetadata[key] = { value, visibility }; | ||
| } else { | ||
| delete channelMetadata[key]; | ||
| } | ||
| } | ||
|
|
||
| return { ...channel, users: updatedUsers, metadata: channelMetadata }; |
There was a problem hiding this comment.
Only mutate the channel that actually matches the METADATA target.
target.startsWith("#") is true for every channel, so we now copy one channel’s metadata onto all others (and KEYNOTSET wipes them all). Compare target to channel.name before updating, and early-return the existing object for other channels. Remember to mirror the fix in METADATA_KEYVALUE and METADATA_KEYNOTSET.
…elopment Add IRCv3 `METADATA`, including avatars, nick colors, display name and status message
2 participants
Summary by CodeRabbit