Test Suite Fixes and UI Enhancements#70
Conversation
- Add break-words class to message content containers - Ensures long messages and URLs wrap properly - Applied to regular messages, system messages, action messages, and reply messages
- Add break-words class to link and span elements in EnhancedLinkWrapper - Prevents long URLs from forcing page width expansion
- URLs longer than 60 characters are now truncated with '...' - Full URL remains functional in the href attribute - Prevents page width issues while maintaining link usability
- Add IRCv3 capability negotiation with SASL 3.2, message tags, metadata, and extended features - Implement channel listing modal with sorting (alphabetical/user count), filtering, and one-click joining - Add operator-only channel renaming functionality with modal - Move channel list button to top bar for better UX - Fix server duplication issues on reconnect - Prevent duplicate channels in listing with concurrency control - Enhance UI with proper modal management and operator permissions - Add SETNAME support for realname changes in user settings - Implement server-time message tags and echo-message handling - Add comprehensive IRCv3 capability support (cap-notify, account tracking, etc.)
…test coverage - Add IRCv3 capability negotiation (CAP LS 302, CAP REQ/ACK) - Implement SASL 3.2 authentication (PLAIN mechanism) - Add message tags parsing (server-time, account, echo-message) - Support channel LIST command with sorting and filtering - Add RENAME command for channel operators - Implement SETNAME for realname changes - Add duplicate server/channel prevention mechanisms - Move channel list and rename buttons to top bar (operator-only) - Create ChannelListModal with auto-loading, sorting, filtering, joining - Create ChannelRenameModal with operator permissions and validation - Add comprehensive test suite: - IRC client tests for all new commands and responses - Component tests for ChannelListModal and ChannelRenameModal - Store tests for state management functionality - Fix UI layout issues and modal rendering problems - Ensure all tests pass before PR submission
- Fixed ircClient.getCurrentUser mock in ChatArea and MetadataDisplay tests - Recreated UserSettings.test.tsx with proper store mocking and navigation tests - Enhanced message event grouping with CollapsedEventMessage component - Added notification sounds functionality with tests - Improved UI components and layout consistency - Updated type definitions and configuration files - All 181 tests now passing (1 skipped)
- Add proper Window interface extension for __HIDE_SERVER_LIST__ - Replace (window as any) with properly typed window property - Resolves lint warning: Unexpected any. Specify a different type. - All tests still passing (181 passed, 1 skipped)
- Better formatting for multiline function parameters in store/index.ts - Improved object literal formatting in notificationSounds.test.ts - Consistent code style following project formatting standards
|
Caution Review failedThe pull request is closed. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughPer-server current-user tracking, IRCv3 metadata and batching (SASL/CAP/multiline), event grouping UI, new message/event components and avatar/status handling, ignore-list utilities and notification-sound helpers, keyboard-resize hook, a large UserSettings refactor, type updates, and many tests and minor UI tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant ChatArea
participant Store
participant IRCClient
participant Server
rect rgba(224,247,250,0.4)
note over User,ChatArea: Multiline send flow (server supports draft/multiline)
User->>ChatArea: Submit message
ChatArea->>Store: Read global settings (multiline/typing)
alt serverSupportsMultiline
ChatArea->>IRCClient: Emit BATCH_START (draft/multiline)
loop fragments/lines
ChatArea->>IRCClient: PRIVMSG (with draft/multiline-concat as needed)
end
IRCClient-->>ChatArea: MULTILINE_MESSAGE (aggregated)
ChatArea->>IRCClient: Emit BATCH_END
else
ChatArea->>IRCClient: PRIVMSG (split per-line)
end
ChatArea->>Store: typing-done / UI update
IRCClient->>Server: Wire-level messages
end
sequenceDiagram
autonumber
actor Renderer
participant ChatArea
participant EventGrouping
participant CollapsedEventMessage
participant MessageItem
note over ChatArea: Message rendering with grouped events
ChatArea->>EventGrouping: groupConsecutiveEvents(messages)
alt item.type == eventGroup
EventGrouping-->>ChatArea: eventGroup
ChatArea->>CollapsedEventMessage: Render collapsed summary + avatars
else
EventGrouping-->>ChatArea: single message
ChatArea->>MessageItem: Render single message (EventMessage or normal)
end
sequenceDiagram
autonumber
actor User
participant MessageItem
participant UserContextMenu
participant Store
User->>MessageItem: Click avatar / username
MessageItem->>UserContextMenu: Open menu (username)
alt selects "Ignore"
UserContextMenu->>Store: addToIgnoreList(createIgnorePattern(...))
else selects "Unignore"
UserContextMenu->>Store: removeFromIgnoreList(matching patterns)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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 |
There was a problem hiding this comment.
Pull Request Overview
This PR addresses critical test failures and introduces comprehensive UI/UX enhancements, transforming the codebase from failing tests to 181 passing tests with new notification features, enhanced message handling, and improved component reliability.
Key changes include:
- Complete test suite restoration with proper mocking patterns and React environment handling
- New notification system with customizable sounds and smart notification logic
- Enhanced message event handling with intelligent grouping and collapsible events
- Major UI improvements including redesigned UserSettings with tabbed navigation
Reviewed Changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vite.config.ts | Added testing library type reference for test environment setup |
| tests/lib/notificationSounds.test.ts | New comprehensive test suite for notification sound functionality |
| tests/components/UserSettings.test.tsx | Recreated test file with proper mocking and React DOM handling |
| tests/components/MetadataDisplay.test.tsx | Fixed IRC client mock to include getCurrentUser method |
| tests/components/ChatArea.test.tsx | Fixed IRC client mock to include getCurrentUser method |
| src/types/index.ts | Enhanced type definitions for new features and better batch handling |
| src/store/index.ts | Major store enhancements with notification system integration and improved event handling |
| src/lib/notificationSounds.ts | New notification sound system with audio file support and cross-browser compatibility |
| src/lib/ircUtils.tsx | Added user verification utility for IRCv3 account-tag support |
| src/lib/ircClient.ts | Enhanced IRC client with batch processing, notification handling, and per-server user management |
| src/lib/eventGrouping.ts | New event grouping logic for cleaner chat experience |
| src/components/ui/UserSettings.tsx | Complete redesign with tabbed interface and comprehensive settings management |
| src/components/ui/AddPrivateChatModal.tsx | Updated to use per-server current user context |
| src/components/message/index.ts | Added exports for new event message components |
| src/components/message/MessageItem.tsx | Enhanced with user verification display and improved bot detection |
| src/components/message/MessageHeader.tsx | Added verification badge display for authenticated users |
| src/components/message/MessageAvatar.tsx | Improved error handling with React state management |
| src/components/message/EventMessage.tsx | New component for individual event message display |
| src/components/message/CollapsedEventMessage.tsx | New component for grouped event message display |
| src/components/layout/MemberList.tsx | Enhanced with better avatar handling and bot information tooltips |
| src/components/layout/ChatArea.tsx | Integrated event grouping and improved typing notification handling |
| src/components/layout/ChannelList.tsx | Updated avatar handling and per-server user context |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| {/* Small event avatar that expands on individual hover */} | ||
| <div className="flex-shrink-0 mr-3"> | ||
| <div | ||
| className="w-3 h-3 rounded-full bg-black flex items-center justify-center text-white text-xs cursor-pointer hover:opacity-80 transform transition-all duration-200 hover:scale-250 hover:w-8 hover:h-8 hover:text-base relative z-10 hover:z-20" |
There was a problem hiding this comment.
This className string is extremely long and complex. Consider extracting the styling logic into CSS classes or using a styling utility function for better maintainability.
| if (!isUserMessage || isSystemMessage) { | ||
| return false; // Don't trigger sounds for system messages |
There was a problem hiding this comment.
The logic is incorrect. A message cannot be both a user message (message.type === 'message') and a system message (join, part, etc.) simultaneously. The condition !isUserMessage || isSystemMessage will always return false for system messages since isUserMessage is false. This should be if (!isUserMessage) { return false; }.
| if (!isUserMessage || isSystemMessage) { | |
| return false; // Don't trigger sounds for system messages | |
| if (!isUserMessage) { | |
| return false; // Don't trigger sounds for non-user messages |
| console.log("[USER_SETTINGS] handleSaveAll called"); | ||
| console.log("[USER_SETTINGS] originalValues:", originalValues); | ||
| console.log("[USER_SETTINGS] current values:", { | ||
| displayName, | ||
| color, | ||
| avatar, | ||
| status, | ||
| homepage, | ||
| bot, | ||
| }); | ||
| console.log("[USER_SETTINGS] supportsMetadata:", supportsMetadata); | ||
|
|
||
| if (!originalValues) { | ||
| console.log("[USER_SETTINGS] No original values, skipping save"); |
There was a problem hiding this comment.
Debug console.log statements should be removed from production code. Consider using a proper logging library or removing these statements before deployment.
| console.log("[USER_SETTINGS] handleSaveAll called"); | |
| console.log("[USER_SETTINGS] originalValues:", originalValues); | |
| console.log("[USER_SETTINGS] current values:", { | |
| displayName, | |
| color, | |
| avatar, | |
| status, | |
| homepage, | |
| bot, | |
| }); | |
| console.log("[USER_SETTINGS] supportsMetadata:", supportsMetadata); | |
| if (!originalValues) { | |
| console.log("[USER_SETTINGS] No original values, skipping save"); | |
| if (!originalValues) { |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/store/index.ts (2)
1967-2074: Guard USERNOTICE private chat access.This is the same critical issue previously flagged:
server.privateChatsmay be undefined, causing.find(line 1994) and the spread operation (line 2011) to throw a runtime TypeError.Apply the defensive pattern from the previous review:
- let privateChat = server.privateChats.find( - (pc) => pc.username === response.sender, - ); + let privateChat = + server.privateChats?.find((pc) => pc.username === response.sender); if (!privateChat) { const newPrivateChat: PrivateChat = { @@ useStore.setState((state) => { const updatedServers = state.servers.map((s) => { if (s.id === server.id) { - return { ...s, privateChats: [...s.privateChats, newPrivateChat] }; + return { + ...s, + privateChats: [...(s.privateChats ?? []), newPrivateChat], + }; } return s; });
3809-3904: Address formatting issue.The CI pipeline flagged a formatting error at lines 3891-3895. Run the formatter to fix code style.
#!/bin/bash # Run the formatter on this file npx prettier --write src/store/index.ts
🧹 Nitpick comments (2)
src/store/index.ts (2)
1490-1505: Consider passing user/host for richer ignore patterns.The ignore checks currently only pass
sender(nick) withundefinedfor user/host parameters. WhileisUserIgnoredhandles this gracefully, you could enable more precise ignore patterns (e.g.,nick!user@*ornick!*@host) by extracting and passing these from message tags when available.For example, in CHANMSG handler:
const user = mtags?.['user'] || response.user; const host = mtags?.['host'] || response.host; if ( isUserIgnored( response.sender, user, host, globalSettings.ignoreList, ) ) { // ... }Based on learnings from
src/lib/ignoreUtils.ts, the utility supports hostmask patterns likenick!user@host,nick!user@*, andnick!*@hostfor more granular control.Also applies to: 1605-1613, 1777-1786, 1909-1924, 1970-1985
4043-4115: Consider batching state updates in netsplit processing.The netsplit handler updates state separately for each quit event (lines 4085-4108). For large netsplits affecting many users, this triggers multiple re-renders. Consider collecting all user removals first, then applying them in a single
setStatecall.Example approach:
// Collect all updates first const userRemovals = new Map<string, Set<string>>(); // channelId -> usernames quitEvents.forEach((event) => { // ... determine affected channels and users }); // Apply all updates at once useStore.setState((state) => { const updatedServers = state.servers.map((server) => { if (server.id === serverId) { const updatedChannels = server.channels.map((channel) => { const toRemove = userRemovals.get(channel.id) || new Set(); return { ...channel, users: channel.users.filter(u => !toRemove.has(u.username)) }; }); return { ...server, channels: updatedChannels }; } return server; }); return { servers: updatedServers }; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/layout/AppLayout.tsx(2 hunks)src/components/layout/ChannelList.tsx(6 hunks)src/store/index.ts(43 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/layout/ChannelList.tsx (1)
src/lib/ircClient.ts (1)
ircClient(1792-1792)
src/store/index.ts (3)
src/lib/ignoreUtils.ts (1)
isUserIgnored(46-83)src/lib/notificationSounds.ts (2)
shouldPlayNotificationSound(62-110)playNotificationSound(6-59)src/types/index.ts (4)
Message(69-106)PrivateChat(55-62)Channel(40-53)User(1-11)
🔇 Additional comments (4)
src/store/index.ts (4)
3809-3904: WHO_REPLY handler looks good.The implementation correctly:
- Parses WHO data into proper User objects
- Extracts channel status from flags (line 3833-3840)
- Handles bot detection when botMode is enabled (line 3854-3864)
- Updates or adds users to channel lists
1570-1582: Notification sound integration looks solid.The implementation consistently:
- Retrieves per-server current user via
getCurrentUser(serverId)- Checks if sounds should play based on user settings and message content
- Prevents sounds for the user's own messages
- Applies highlight-based filtering when enabled
The pattern is correctly applied across CHANMSG, MULTILINE_MESSAGE, USERMSG, CHANNNOTICE, and USERNOTICE handlers.
Also applies to: 1679-1690, 1853-1864, 1952-1963, 2038-2049
3048-3136: NICK_ERROR handler with auto-retry is well implemented.The handler provides good UX by:
- Automatically retrying nickname registration with an appended underscore on 433 errors
- Providing clear system messages about the retry
- Adding global notifications for other error codes
- Using early return to avoid duplicate notifications on auto-retry
3342-3351: Metadata target resolution correctly handles wildcards.Both METADATA and METADATA_KEYVALUE handlers properly resolve the "*" wildcard to the current user for the specific server, with appropriate fallback to
getNick()if the current user isn't yet set. The verbose logging aids debugging of metadata flows.Also applies to: 3479-3487
| // Reset avatar load failed state when user or server changes | ||
| // biome-ignore lint/correctness/useExhaustiveDependencies: We want to reset when user/server changes | ||
| useEffect(() => { | ||
| setAvatarLoadFailed(false); | ||
| }, [currentUser?.username, selectedServerId]); | ||
|
|
There was a problem hiding this comment.
Reset avatar failure when avatar URL changes.
Once avatarLoadFailed flips to true, we never retry unless the username or server changes. If the same user gets a new avatar (or metadata arrives late), we keep showing the fallback forever. Include the avatar metadata value in the dependencies so the reset runs when the avatar URL updates.
- }, [currentUser?.username, selectedServerId]);
+ }, [
+ currentUser?.username,
+ currentUser?.metadata?.avatar?.value,
+ selectedServerId,
+ ]);📝 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.
| // Reset avatar load failed state when user or server changes | |
| // biome-ignore lint/correctness/useExhaustiveDependencies: We want to reset when user/server changes | |
| useEffect(() => { | |
| setAvatarLoadFailed(false); | |
| }, [currentUser?.username, selectedServerId]); | |
| // Reset avatar load failed state when user or server changes | |
| // biome-ignore lint/correctness/useExhaustiveDependencies: We want to reset when user/server changes | |
| useEffect(() => { | |
| setAvatarLoadFailed(false); | |
| }, [ | |
| currentUser?.username, | |
| currentUser?.metadata?.avatar?.value, | |
| selectedServerId, | |
| ]); |
🤖 Prompt for AI Agents
In src/components/layout/ChannelList.tsx around lines 83 to 88, the useEffect
resets avatarLoadFailed only when username or selectedServerId changes, so if
the user's avatar URL/metadata updates we never retry; update the dependency
array to include the avatar metadata (e.g., currentUser?.avatarUrl or
currentUser?.avatar) so setAvatarLoadFailed(false) runs when the avatar URL
changes, and keep or adjust the biome-ignore lint comment accordingly to allow
this dependency addition.
| } else if (!channelName) { | ||
| // Handle multiline private messages | ||
| // Similar logic to USERMSG but for multiline content | ||
| const currentUser = ircClient.getCurrentUser(response.serverId); | ||
| if (currentUser && sender === currentUser.username) { | ||
| return; // Don't create private chats with ourselves | ||
| } | ||
|
|
||
| // Create or find private chat | ||
| let privateChat = server.privateChats.find( | ||
| (chat) => chat.username === sender, | ||
| ); | ||
| if (!privateChat) { | ||
| const newPrivateChat = { | ||
| id: uuidv4(), | ||
| username: sender, | ||
| serverId: server.id, | ||
| unreadCount: 0, | ||
| isMentioned: false, | ||
| }; | ||
| privateChat = newPrivateChat; | ||
| useStore.setState((state) => ({ | ||
| servers: state.servers.map((s) => | ||
| s.id === server.id | ||
| ? { ...s, privateChats: [...s.privateChats, newPrivateChat] } | ||
| : s, | ||
| ), | ||
| })); | ||
| } |
There was a problem hiding this comment.
Guard multiline private message privateChats access.
Same critical issue as USERNOTICE: when handling multiline private messages (lines 1712, 1727), the code assumes privateChats exists. The first multiline PM on a server without prior private chats will throw a TypeError.
Apply this diff:
- let privateChat = server.privateChats.find(
- (chat) => chat.username === sender,
- );
+ let privateChat = server.privateChats?.find(
+ (chat) => chat.username === sender,
+ );
if (!privateChat) {
@@
useStore.setState((state) => ({
servers: state.servers.map((s) =>
s.id === server.id
- ? { ...s, privateChats: [...s.privateChats, newPrivateChat] }
+ ? { ...s, privateChats: [...(s.privateChats ?? []), newPrivateChat] }
: s,
),
}));📝 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 (!channelName) { | |
| // Handle multiline private messages | |
| // Similar logic to USERMSG but for multiline content | |
| const currentUser = ircClient.getCurrentUser(response.serverId); | |
| if (currentUser && sender === currentUser.username) { | |
| return; // Don't create private chats with ourselves | |
| } | |
| // Create or find private chat | |
| let privateChat = server.privateChats.find( | |
| (chat) => chat.username === sender, | |
| ); | |
| if (!privateChat) { | |
| const newPrivateChat = { | |
| id: uuidv4(), | |
| username: sender, | |
| serverId: server.id, | |
| unreadCount: 0, | |
| isMentioned: false, | |
| }; | |
| privateChat = newPrivateChat; | |
| useStore.setState((state) => ({ | |
| servers: state.servers.map((s) => | |
| s.id === server.id | |
| ? { ...s, privateChats: [...s.privateChats, newPrivateChat] } | |
| : s, | |
| ), | |
| })); | |
| } | |
| } else if (!channelName) { | |
| // Handle multiline private messages | |
| // Similar logic to USERMSG but for multiline content | |
| const currentUser = ircClient.getCurrentUser(response.serverId); | |
| if (currentUser && sender === currentUser.username) { | |
| return; // Don't create private chats with ourselves | |
| } | |
| // Create or find private chat | |
| - let privateChat = server.privateChats.find( | |
| - (chat) => chat.username === sender, | |
| let privateChat = server.privateChats?.find( | |
| (chat) => chat.username === sender, | |
| ); | |
| if (!privateChat) { | |
| const newPrivateChat = { | |
| id: uuidv4(), | |
| username: sender, | |
| serverId: server.id, | |
| unreadCount: 0, | |
| isMentioned: false, | |
| }; | |
| privateChat = newPrivateChat; | |
| useStore.setState((state) => ({ | |
| servers: state.servers.map((s) => | |
| s.id === server.id | |
| ? { ...s, privateChats: [...(s.privateChats ?? []), newPrivateChat] } | |
| : s, | |
| ), | |
| })); | |
| } |
🤖 Prompt for AI Agents
In src/store/index.ts around lines 1703 to 1731, accessing server.privateChats
can throw if it's undefined when the first multiline private message arrives;
update the code to guard that access by treating privateChats as an array (e.g.,
use server.privateChats || []) when calling find and when appending a
newPrivateChat, and ensure when updating state you spread into
(server.privateChats || []) so you never call .find or spread on undefined;
alternatively initialize server.privateChats = [] when loading/creating servers
so the same guards are in place.
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 (5)
src/components/message/MessageItem.tsx (1)
59-115: Fix current-user detection and verification checks.
message.userIdcarries extra suffix data, so comparing it directly to the nick means we never identify our own messages, blocking redaction and header styling. The same composite value is passed toisUserVerified, preventing verified badges from ever showing. Normalize the nick once and reuse it:- const ircCurrentUser = ircClient.getCurrentUser(message.serverId); - const isCurrentUser = ircCurrentUser?.username === message.userId; + const ircCurrentUser = ircClient.getCurrentUser(message.serverId); + const messageNick = message.userId.split("-")[0]; + const isCurrentUser = ircCurrentUser?.username === messageNick; @@ - const messageUser = users.find( - (user) => user.username === message.userId.split("-")[0], - ); + const messageUser = users.find((user) => user.username === messageNick); @@ - const isVerified = isUserVerified(message.userId, message.tags); + const isVerified = isUserVerified(messageNick, message.tags); @@ - const username = message.userId.split("-")[0]; + const username = messageNick;This restores both self-message checks and verified indicators.
src/store/index.ts (4)
73-80: Fix reply resolution to use msgid and keep unique local ids.Replies reference IRC msgid, not our local id. Using replyId as local id can collide.
Apply:
export const findChannelMessageById = ( @@ - return messages.find((message) => message.id === messageId); + // Match by IRC message id (msgid) + return messages.find((message) => message.msgid === messageId);- const newMessage = { - id: replyId ? replyId : uuidv4(), + const newMessage = { + id: uuidv4(), msgid: mtags?.msgid, content: message,Also applies to: 1586-1599
766-768: Remove unused variable assignment in sendMessage.Prevents linter warning.
- sendMessage: (serverId, channelId, content) => { - const message = ircClient.sendMessage(serverId, channelId, content); - }, + sendMessage: (serverId, channelId, content) => { + ircClient.sendMessage(serverId, channelId, content); + },
2615-2623: selectedChannelId set to channel name instead of id on ready.Breaks selection; must resolve the channel by name and store its id.
- useStore.setState((state) => ({ - ui: { - ...state.ui, - selectedServerId: serverId, - selectedChannelId: savedServer.channels[0] || null, - }, - })); + useStore.setState((state) => { + const server = state.servers.find((s) => s.id === serverId); + const firstName = savedServer.channels[0]; + const firstChannelId = + server?.channels.find((c) => c.name === firstName)?.id || null; + return { + ui: { + ...state.ui, + selectedServerId: serverId, + selectedChannelId: firstChannelId, + }, + }; + });
2718-2743: Duplicate SASL AUTH flow: both ircClient and store send AUTHENTICATE. Choose one.This risks double AUTHENTICATE/CAP END and protocol errors. Prefer letting ircClient handle SASL and remove this handler.
-ircClient.on("AUTHENTICATE", ({ serverId, param }) => { - console.log(param); - if (param !== "+") return; - - let user: string | undefined; - let pass: string | undefined; - const servers = loadSavedServers(); - for (const serv of servers) { - if (serv.id !== serverId) continue; - - if (!serv.saslEnabled) return; - - user = serv.saslAccountName?.length ? serv.saslAccountName : serv.nickname; - pass = serv.saslPassword ? atob(serv.saslPassword) : undefined; - } - if (!user || !pass) - // wtf happened lol - return; - - ircClient.sendRaw( - serverId, - `AUTHENTICATE ${btoa(`${user}\x00${user}\x00${pass}`)}`, - ); - ircClient.sendRaw(serverId, "CAP END"); - ircClient.userOnConnect(serverId); -}); +// SASL is handled by ircClient; avoid duplicate AUTHENTICATE here.
♻️ Duplicate comments (8)
src/components/layout/MemberList.tsx (1)
72-75: Keep avatar retryable when the URL updates.The effect is meant to reset
avatarLoadFailedwhenavatarUrlchanges, but the empty dependency array means it only fires once on mount. If a user updates their avatar (or metadata arrives later) the fallback never clears. Wire the effect to the URL instead:- useEffect(() => { - setAvatarLoadFailed(false); - }, []); + useEffect(() => { + setAvatarLoadFailed(false); + }, [avatarUrl]);src/components/message/MessageAvatar.tsx (1)
28-31: Ensure avatar load failures reset on prop changes.The effect still runs only once, so a new
avatarUrl(or a user switch) keeps the fallback avatar forever. Depend on the relevant props:- useEffect(() => { - setImageLoadFailed(false); - }, []); + useEffect(() => { + setImageLoadFailed(false); + }, [avatarUrl, userId]);src/components/layout/ChannelList.tsx (1)
83-88: Reset the avatar fallback when metadata updates.If the current user’s avatar URL changes,
avatarLoadFailedstays true and the UI never retries. Add the avatar metadata to the dependencies:- useEffect(() => { - setAvatarLoadFailed(false); - }, [currentUser?.username, selectedServerId]); + useEffect(() => { + setAvatarLoadFailed(false); + }, [ + currentUser?.username, + currentUser?.metadata?.avatar?.value, + selectedServerId, + ]);src/lib/ircClient.ts (3)
933-959: Align NOTICE message extraction with PRIVMSG parsing.Use the already-parsed trailing parameter via parv for consistency.
- // The message content is now properly parsed as the trailing parameter - const message = trailing || parv.slice(1).join(" "); + // Message content mirrors PRIVMSG parsing + const message = parv.slice(1).join(" ");
280-284: Stop logging sensitive SASL details and clear credentials on success/fail/disconnect.Avoid logging account names; wipe creds after use to reduce exposure. Clear on socket close and after 900–903/904–907 responses.
Apply these diffs:
- console.log(`[SASL] SASL account name: ${_saslAccountName}`); - console.log(`[SASL] SASL password provided: ${!!_saslPassword}`); + // Avoid logging SASL account or password for security + console.log(`[SASL] SASL credentials provided: ${Boolean( + _saslAccountName && _saslPassword, + )}`);socket.onclose = () => { console.log(`Disconnected from server ${host}`); this.sockets.delete(server.id); server.isConnected = false; this.pendingConnections.delete(connectionKey); + // Clear any sensitive credentials on disconnect + this.saslCredentials.delete(server.id); };} else if ( command === "900" || command === "901" || command === "902" || command === "903" ) { // SASL authentication successful const message = parv.slice(2).join(" "); console.log( `SASL authentication successful for ${serverId}: ${message}`, ); + // Clear credentials after successful auth + this.saslCredentials.delete(serverId); // Finish capability negotiation this.sendRaw(serverId, "CAP END"); } else if ( command === "904" || command === "905" || command === "906" || command === "907" ) { // SASL authentication failed const message = parv.slice(2).join(" "); console.log(`SASL authentication failed for ${serverId}: ${message}`); + // Clear credentials on failure + this.saslCredentials.delete(serverId); // Still finish capability negotiation even if SASL failed this.sendRaw(serverId, "CAP END"); }Also applies to: 321-327, 1437-1444, 1450-1455
1067-1072: CAP NAK should clear pending CAP state to avoid duplicate CAP END.Without cleanup, the timeout in onCapLs may also fire and send CAP END again.
} else if (subcommand === "NAK") { console.log( `[CAP NAK] Server rejected capabilities for ${serverId}: ${caps}`, ); + // Clean up pending CAP state since we're ending negotiation + this.pendingCapReqs.delete(serverId); // Server rejected some capabilities, but we should still end CAP negotiation this.sendRaw(serverId, "CAP END"); }src/store/index.ts (2)
1773-1792: Guard privateChats access for multiline PMs.First PM can throw when privateChats is undefined.
- let privateChat = server.privateChats.find( + let privateChat = server.privateChats?.find( (chat) => chat.username === sender, ); @@ - useStore.setState((state) => ({ - servers: state.servers.map((s) => - s.id === server.id - ? { ...s, privateChats: [...s.privateChats, newPrivateChat] } - : s, - ), - })); + useStore.setState((state) => ({ + servers: state.servers.map((s) => + s.id === server.id + ? { + ...s, + privateChats: [...(s.privateChats ?? []), newPrivateChat], + } + : s, + ), + }));
2055-2078: Guard privateChats access for USERNOTICE private chats.Same crash risk as above.
- let privateChat = server.privateChats.find( + let privateChat = server.privateChats?.find( (pc) => pc.username === response.sender, ); @@ - return { ...s, privateChats: [...s.privateChats, newPrivateChat] }; + return { + ...s, + privateChats: [...(s.privateChats ?? []), newPrivateChat], + };
🧹 Nitpick comments (5)
src/lib/ircClient.ts (3)
457-457: Replace deprecated substr with slice in batchId generation.Avoid deprecated API usage.
- const batchId = `ml_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`; + const batchId = `ml_${Date.now()}_${Math.random().toString(36).slice(2, 11)}`;
442-449: Guard multiline send against servers without draft/multiline.Currently sends BATCH even if the server didn’t ACK multiline. Add a capability check or fallback to single-line here to be robust.
Would you like me to wire a per-server capability Map in IRCClient and gate this call?
656-747: Gate verbose debug logging behind a flag.Large, unconditional console logs will flood production consoles and leak content. Wrap debug logs with an environment/config flag.
Also applies to: 720-747, 1020-1061
src/store/index.ts (2)
1484-1566: Ignore-list: OK, but centralize check to reduce duplication.Repeated ignore checks across CHANMSG/USERMSG/MULTILINE/NOTICES can be abstracted into a helper to reduce mistakes.
4175-4275: Batch processing: OK. Ensure unknown types don’t loop endlessly.The fallback re-triggers JOIN/QUIT/PART. Consider bounding or marking re-emitted events to avoid reprocessing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/components/layout/ChannelList.tsx(6 hunks)src/components/layout/MemberList.tsx(4 hunks)src/components/message/ActionMessage.tsx(1 hunks)src/components/message/MessageAvatar.tsx(3 hunks)src/components/message/MessageItem.tsx(8 hunks)src/components/ui/AddServerModal.tsx(2 hunks)src/lib/ircClient.ts(37 hunks)src/store/index.ts(50 hunks)src/types/index.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/components/layout/ChannelList.tsx (1)
src/lib/ircClient.ts (1)
ircClient(1836-1836)
src/lib/ircClient.ts (2)
src/types/index.ts (5)
BaseUserActionEvent(181-183)BaseMessageEvent(174-178)BaseIRCEvent(152-154)User(1-13)Server(15-31)src/lib/ircUtils.tsx (1)
parseMessageTags(34-44)
src/components/message/MessageItem.tsx (6)
src/lib/ircClient.ts (1)
ircClient(1836-1836)src/lib/ircUtils.tsx (1)
isUserVerified(51-61)src/components/message/DateSeparator.tsx (1)
DateSeparator(8-27)src/components/message/EventMessage.tsx (1)
EventMessage(19-121)src/components/ui/LinkWrapper.tsx (1)
EnhancedLinkWrapper(9-85)src/components/message/MessageReactions.tsx (1)
MessageReactions(16-82)
src/store/index.ts (3)
src/lib/ignoreUtils.ts (1)
isUserIgnored(46-83)src/lib/notificationSounds.ts (2)
shouldPlayNotificationSound(62-110)playNotificationSound(6-59)src/types/index.ts (3)
Message(74-111)PrivateChat(60-67)User(1-13)
🔇 Additional comments (4)
src/store/index.ts (3)
138-139: Notification sounds: good integration.Usage respects settings and avoids own messages.
Would you like me to add a silent failure metric counter for debugging (e.g., count of play() rejections)?
Also applies to: 1661-1661, 1967-1967, 2013-2024
351-377: Public API surface changes: connect/changeNick/updateGlobalSettings look coherent.Signatures align with ircClient usage.
1502-1511: Capability propagation to server objects looks consistent.Servers get capabilities stored and are used elsewhere.
Also applies to: 2753-2765, 3790-3803
src/lib/ircClient.ts (1)
578-587: Client METADATA SET commands don’t support visibility
Per the IRCv3 Metadata spec, client-initiatedMETADATA <Target> SET <Key> [:Value]commands do not accept a visibility token; visibility only appears in server notifications. Remove any attempt to includevisibilityin the generated command.Likely an incorrect or invalid review comment.
| const openState = | ||
| isOpen !== undefined ? isOpen : !state.ui.isChannelListVisible; | ||
|
|
||
| // Only change mobileViewActiveColumn if we're not on the serverList view | ||
| // This prevents desktop member list toggles from affecting mobile navigation | ||
| const shouldUpdateMobileColumn = | ||
| state.ui.mobileViewActiveColumn !== "serverList"; | ||
|
|
||
| return { | ||
| ui: { | ||
| ...state.ui, | ||
| isMemberListVisible: | ||
| openState !== undefined ? openState : !state.ui.isMemberListVisible, | ||
| mobileViewActiveColumn: openState ? "memberList" : "chatView", | ||
| mobileViewActiveColumn: shouldUpdateMobileColumn | ||
| ? openState | ||
| ? "memberList" | ||
| : "chatView" | ||
| : state.ui.mobileViewActiveColumn, | ||
| }, | ||
| }; | ||
| }); |
There was a problem hiding this comment.
toggleMemberList toggles wrong state key.
It uses isChannelListVisible instead of isMemberListVisible, causing incorrect behavior.
- const openState =
- isOpen !== undefined ? isOpen : !state.ui.isChannelListVisible;
+ const openState =
+ isOpen !== undefined ? isOpen : !state.ui.isMemberListVisible;📝 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 openState = | |
| isOpen !== undefined ? isOpen : !state.ui.isChannelListVisible; | |
| // Only change mobileViewActiveColumn if we're not on the serverList view | |
| // This prevents desktop member list toggles from affecting mobile navigation | |
| const shouldUpdateMobileColumn = | |
| state.ui.mobileViewActiveColumn !== "serverList"; | |
| return { | |
| ui: { | |
| ...state.ui, | |
| isMemberListVisible: | |
| openState !== undefined ? openState : !state.ui.isMemberListVisible, | |
| mobileViewActiveColumn: openState ? "memberList" : "chatView", | |
| mobileViewActiveColumn: shouldUpdateMobileColumn | |
| ? openState | |
| ? "memberList" | |
| : "chatView" | |
| : state.ui.mobileViewActiveColumn, | |
| }, | |
| }; | |
| }); | |
| const openState = | |
| isOpen !== undefined ? isOpen : !state.ui.isMemberListVisible; | |
| // Only change mobileViewActiveColumn if we're not on the serverList view | |
| // This prevents desktop member list toggles from affecting mobile navigation | |
| const shouldUpdateMobileColumn = | |
| state.ui.mobileViewActiveColumn !== "serverList"; | |
| return { | |
| ui: { | |
| ...state.ui, | |
| isMemberListVisible: | |
| openState !== undefined ? openState : !state.ui.isMemberListVisible, | |
| mobileViewActiveColumn: shouldUpdateMobileColumn | |
| ? openState | |
| ? "memberList" | |
| : "chatView" | |
| : state.ui.mobileViewActiveColumn, | |
| }, | |
| }; | |
| }); |
🤖 Prompt for AI Agents
In src/store/index.ts around lines 1269 to 1289, the toggleMemberList logic
computes openState from the wrong key (isChannelListVisible); change that to use
isMemberListVisible so the initial state and fallback use
state.ui.isMemberListVisible (i.e. replace references to isChannelListVisible
with isMemberListVisible) and ensure the isMemberListVisible update and
mobileViewActiveColumn logic remain consistent with the corrected openState.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/App.test.tsx (1)
104-157: Add assertions for successful connection side effects.The test mocks a successful connection and verifies that
ircClient.connectis called with the correct arguments. However, it doesn't verify that the connection actually succeeded from the user's perspective:
- Is the server added to the store's
serversarray?- Does the UI show the connected server?
- Is the "Add Server" modal closed after successful connection?
Add assertions after line 156 to verify the side effects:
expect(ircClient.connect).toHaveBeenCalledWith( "Test Server", "irc.test.com", 443, "tester", "", "tester", "c3VwZXIgYXdlc29tZSBwYXNzd29yZCBsbWFvIDEyMyAhPyE/IQ==", undefined, ); + + // Verify successful connection side effects + await waitFor(() => { + expect(screen.queryByText(/Add IRC Server/i)).not.toBeInTheDocument(); + }); + + const state = useStore.getState(); + expect(state.servers).toHaveLength(1); + expect(state.servers[0].name).toBe("Test Server"); + expect(state.servers[0].isConnected).toBe(true); });
🧹 Nitpick comments (3)
tests/App.test.tsx (2)
78-78: Clarify the hardcoded ignore pattern.The baseline store state includes
ignoreList: ["HistServ!*@*"]with a specific pattern. If this is test-specific rather than a general default, consider using an empty array for the baseline and setting this value only in tests that require it.Apply this diff if the ignore pattern is not universally needed:
- ignoreList: ["HistServ!*@*"], + ignoreList: [],
212-220: Consider adding test IDs for more reliable button selection.The complex fallback logic for finding the cancel button suggests the DOM structure may be ambiguous. This could lead to brittle tests if the UI structure changes.
Consider adding a
data-testidto the User Settings modal's cancel button to simplify selection:In
src/components/UserSettings.tsx(or wherever the modal is defined):<button data-testid="user-settings-cancel-button" ...> Cancel </button>Then simplify the test:
- const cancelButtons = screen.getAllByRole("button", { name: /cancel/i }); - // Find the cancel button in the User Settings modal (should be the second one) - const userSettingsCancel = - cancelButtons.find( - (button) => - button.closest('[data-testid="user-settings-modal"]') || - (button.textContent === "Cancel" && - button.classList.contains("bg-discord-dark-400")), - ) || cancelButtons[1]; // fallback to second cancel button - await user.click(userSettingsCancel); + await user.click(screen.getByTestId("user-settings-cancel-button"));tests/lib/ircClient.test.ts (1)
151-166: Add assertion forserver.nameon reconnect
Sinceconnectreturns the existing server without updating itsname, addexpect(secondServer.name).toBe("Test Server");to codify this behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/App.test.tsx(7 hunks)tests/lib/ircClient.test.ts(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/App.test.tsx (1)
src/lib/ircClient.ts (1)
ircClient(1836-1836)
🔇 Additional comments (8)
tests/App.test.tsx (4)
1-1: LGTM! Async handling import added.The addition of
waitForis appropriate for handling asynchronous UI behavior in the tests that follow.
172-193: LGTM! Proper async handling for modal and error message.The use of
waitForto handle asynchronous modal opening and error message appearance is correct and follows React Testing Library best practices.
204-204: LGTM! User mock extended with required field.Adding
isOnline: truealigns with the updated type definitions for the current user.
19-19: Verify per-server getCurrentUser behavior in App tests (tests/App.test.tsx:19)
ThegetCurrentUsermock always returns the same user, but the PR adds per-server tracking. Confirm whether any App tests switch servers or callgetCurrentUserwith a server ID; if so, update the mock to return different values based on the server argument.tests/lib/ircClient.test.ts (4)
85-103: LGTM! Consistent parameter update.The test correctly reflects the new
connect()signature withserverNameas the first parameter, and the assertion on line 98 appropriately validatesserver.nameagainst the provided server name.
105-127: Consider re-enabling the skipped connection error test.This test is currently skipped. If the underlying issue causing it to be skipped has been resolved, consider re-enabling it to ensure proper error handling coverage.
Has the issue causing this test to be skipped been resolved? If not, what is blocking it from being re-enabled?
174-179: LGTM! Consistent parameter update.The
connect()call correctly includes the server name as the first parameter, maintaining consistency with the updated signature.
219-224: LGTM! Consistent parameter update in test setup.The
beforeEachsetup correctly includes the server name as the first parameter in theconnect()call, ensuring all channel operation tests use the updated signature.
* feat: make messages wrap instead of widening the page - Add break-words class to message content containers - Ensures long messages and URLs wrap properly - Applied to regular messages, system messages, action messages, and reply messages * fix: ensure long URLs wrap properly - Add break-words class to link and span elements in EnhancedLinkWrapper - Prevents long URLs from forcing page width expansion * fix: truncate long URLs to 60 chars for display - URLs longer than 60 characters are now truncated with '...' - Full URL remains functional in the href attribute - Prevents page width issues while maintaining link usability * style: use template literal for URL truncation * feat: Implement comprehensive IRCv3 client features - Add IRCv3 capability negotiation with SASL 3.2, message tags, metadata, and extended features - Implement channel listing modal with sorting (alphabetical/user count), filtering, and one-click joining - Add operator-only channel renaming functionality with modal - Move channel list button to top bar for better UX - Fix server duplication issues on reconnect - Prevent duplicate channels in listing with concurrency control - Enhance UI with proper modal management and operator permissions - Add SETNAME support for realname changes in user settings - Implement server-time message tags and echo-message handling - Add comprehensive IRCv3 capability support (cap-notify, account tracking, etc.) * feat: Implement comprehensive IRCv3 client features and add complete test coverage - Add IRCv3 capability negotiation (CAP LS 302, CAP REQ/ACK) - Implement SASL 3.2 authentication (PLAIN mechanism) - Add message tags parsing (server-time, account, echo-message) - Support channel LIST command with sorting and filtering - Add RENAME command for channel operators - Implement SETNAME for realname changes - Add duplicate server/channel prevention mechanisms - Move channel list and rename buttons to top bar (operator-only) - Create ChannelListModal with auto-loading, sorting, filtering, joining - Create ChannelRenameModal with operator permissions and validation - Add comprehensive test suite: - IRC client tests for all new commands and responses - Component tests for ChannelListModal and ChannelRenameModal - Store tests for state management functionality - Fix UI layout issues and modal rendering problems - Ensure all tests pass before PR submission * Linting * Protocol fixes * Linting * Persist metadata * Other parsing fixes * update connection test * Biome * Actually fix text * Perfect account-registration * Properly deal with CAPs * More fixes * Put the bin buttons back aaa * Make redaction properly work * =] * Maybe fix metadata not working on some servers * Maybe fix metadata not working on some servers * Fix test suite and enhance components - Fixed ircClient.getCurrentUser mock in ChatArea and MetadataDisplay tests - Recreated UserSettings.test.tsx with proper store mocking and navigation tests - Enhanced message event grouping with CollapsedEventMessage component - Added notification sounds functionality with tests - Improved UI components and layout consistency - Updated type definitions and configuration files - All 181 tests now passing (1 skipped) * fix: Remove explicit any type in UserSettings test - Add proper Window interface extension for __HIDE_SERVER_LIST__ - Replace (window as any) with properly typed window property - Resolves lint warning: Unexpected any. Specify a different type. - All tests still passing (181 passed, 1 skipped) * style: Improve code formatting in store and tests - Better formatting for multiline function parameters in store/index.ts - Improved object literal formatting in notificationSounds.test.ts - Consistent code style following project formatting standards * Fix TypeScript compilation errors and test isolation issues - Add missing interface properties for strict TypeScript compliance - Fix User interface: add required isOnline property - Fix UIState interface: add isChannelListModalOpen and isChannelRenameModalOpen - Fix Message interface: add reactions, replyMessage, mentioned properties - Convert timestamp strings to Date objects in test fixtures - Fix MediaQueryList type in test setup - Add proper test isolation with state cleanup in App.test.tsx - Add waitFor for async connection error handling - All tests now pass (181 passing, 1 skipped) - Build process now completes without TypeScript errors * Use alt nick if nick taken * Rely on 001 to know our nick at connect time * Rely on 001 to know our nick at connect time * Better self-nick tracking * Various improvement * fix broken statusprefix parsing * Fix Android keyboard covering input box - Add android:windowSoftInputMode="adjustResize" to AndroidManifest.xml - Create useKeyboardResize hook for JavaScript keyboard handling - Add native Android keyboard detection in MainActivity.kt - Update viewport meta tag for better mobile handling - Add mobile-optimized CSS for keyboard transitions - Integrate keyboard handling into main App component - Fix IRC message parsing in ircClient.ts * ... * Fix mobile dark grey overlay issue - Remove problematic position: fixed CSS rules for mobile - Make keyboard resize hook work in regular mobile browsers - Add mobile debug indicators to identify layout issues - Ensure server list takes full width on mobile - Fix mobile layout column rendering logic * Fix mobile dark grey overlay issue - Remove problematic position: fixed CSS rules for mobile - Make keyboard resize hook work in regular mobile browsers - Add mobile debug indicators to identify layout issues - Ensure server list takes full width on mobile - Fix mobile layout column rendering logic * Add draft/multline and improve other batching * Try out znc playback * Update src/store/index.ts Co-authored-by: Matheus Fillipe <matheusfillipeag@gmail.com> * Bump biome apparently O.o * Fix mobile display issues * Fix mobile display issues * Bump biome apparently O.o * More fixes regarding metadata syncing sessions over shared sessions like in ergo * Update tests * Modify input placeholder on mobiles * Delete server info when deleting a server --------- Co-authored-by: Matheus Fillipe <matheusfillipeag@gmail.com>
🎯 Overview
This PR addresses critical test failures and introduces comprehensive UI/UX enhancements to improve code quality, user experience, and maintainability. The changes include complete test suite fixes, new notification features, enhanced message event handling, and improved component reliability.
📊 Test Results
🔧 Critical Fixes
1. Test Suite Restoration (Major Fix)
ircClient.getCurrentUsermock issues affecting 21+ testsUserSettings.test.tsxfrom corrupted file with proper:vi.mockpattern2. TypeScript & Code Quality
anytypes with proper interface extensions🚀 New Features & Enhancements
3. Notification System 🔔
New Files:
src/lib/notificationSounds.ts,tests/lib/notificationSounds.test.ts4. Enhanced Message Event Handling 📨
New Files:
src/components/message/CollapsedEventMessage.tsx,src/components/message/EventMessage.tsx,src/lib/eventGrouping.ts5. UI/UX Improvements ✨
Enhanced Components:
📁 Files Changed (23 files)
🆕 New Files (6)
src/components/message/CollapsedEventMessage.tsx- Collapsible event messagessrc/components/message/EventMessage.tsx- Enhanced event message displaysrc/lib/eventGrouping.ts- Smart event grouping logicsrc/lib/notificationSounds.ts- Notification sound systemtests/components/UserSettings.test.tsx- Comprehensive settings teststests/lib/notificationSounds.test.ts- Audio functionality tests🔄 Enhanced Files (17)
🎯 Key Improvements
User Experience
Developer Experience
anyusageCode Quality
🧪 Testing Strategy
Comprehensive Coverage
Quality Assurance
🚀 Migration Notes
Breaking Changes
New Dependencies
Configuration Updates
🔍 Review Checklist
🎉 Impact Summary
This PR transforms the codebase from a state with failing tests and maintenance issues to a robust, well-tested, and feature-rich application. The improvements significantly enhance both user experience and developer productivity while maintaining high code quality standards.
Total Changes: +3,524 insertions, -595 deletions across 23 files
Summary by CodeRabbit
New Features
Improvements
Tests