-
Notifications
You must be signed in to change notification settings - Fork 3
feat: whispers #110
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: whispers #110
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a whispers (direct messaging) feature for the application, allowing users to send and receive private messages. The implementation includes UI components for viewing whisper conversations, managing message threads, and displaying unread message counts.
Key changes include:
- New routes for viewing whisper lists and individual whisper conversations
- Whisper model with message management and API integration
- Global handler architecture to support whispers alongside channel-specific events
- UI updates to the sidebar with unread whisper notifications
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/routes/(main)/whispers/[id]/+page.ts |
Load function for individual whisper conversation (has data access issue) |
src/routes/(main)/whispers/[id]/+page.svelte |
Chat UI for whisper conversations with message list and input |
src/routes/(main)/whispers/+page.ts |
Load function for whisper list page |
src/routes/(main)/whispers/+page.svelte |
UI for displaying whisper threads with relative timestamps |
src/lib/models/whisper.svelte.ts |
New Whisper class for managing messages and sending whispers |
src/lib/models/user.svelte.ts |
Added whispers map to User model |
src/lib/models/index.ts |
Export whisper model |
src/lib/handlers/irc/whisper.ts |
Handler for incoming whisper messages |
src/lib/handlers/helper.ts |
Extended handler types to support global handlers |
src/lib/twitch/irc.ts |
Added WhisperMessage type and BaseMessage interface |
src/lib/managers/user-manager.ts |
Updated error message to use ErrorMessage helper |
src/lib/app.svelte.ts |
Updated handler routing to support global vs channel handlers |
src/routes/(main)/channels/[username]/+page.svelte |
Filter out global handlers from channel message processing |
src/lib/components/Sidebar.svelte |
Added whispers button with unread count badge |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await this.client.post("/whispers", { | ||
| params: { | ||
| from_user_id: app.user.id, | ||
| to_user_id: this.id, | ||
| }, | ||
| body: { | ||
| message, | ||
| }, | ||
| }); | ||
|
|
||
| this.messages.push({ | ||
| id: crypto.randomUUID(), | ||
| createdAt: new Date(), | ||
| badges: [], | ||
| user: app.user, | ||
| text: message, | ||
| }); |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The send method doesn't handle API errors. If the client.post call fails (network error, authorization failure, etc.), the message is still added to the local messages array, creating a discrepancy between local state and the server. Consider wrapping the API call in a try-catch block and only adding the message after a successful response, or implement proper error handling and rollback.
| await this.client.post("/whispers", { | |
| params: { | |
| from_user_id: app.user.id, | |
| to_user_id: this.id, | |
| }, | |
| body: { | |
| message, | |
| }, | |
| }); | |
| this.messages.push({ | |
| id: crypto.randomUUID(), | |
| createdAt: new Date(), | |
| badges: [], | |
| user: app.user, | |
| text: message, | |
| }); | |
| try { | |
| await this.client.post("/whispers", { | |
| params: { | |
| from_user_id: app.user.id, | |
| to_user_id: this.id, | |
| }, | |
| body: { | |
| message, | |
| }, | |
| }); | |
| this.messages.push({ | |
| id: crypto.randomUUID(), | |
| createdAt: new Date(), | |
| badges: [], | |
| user: app.user, | |
| text: message, | |
| }); | |
| } catch (error) { | |
| // Optionally handle the error (e.g., log, notify user) | |
| console.error("Failed to send whisper:", error); | |
| } |
| if (handler.global === undefined) { | ||
| handler.global = false; | ||
| } | ||
|
|
||
| return handler; |
Copilot
AI
Nov 13, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mutating handler.global by setting a default value modifies the input parameter, which is an anti-pattern. This could cause unexpected behavior if the same handler object is passed multiple times. Instead, use a default parameter or handle the undefined case in the return value without mutation: return { ...handler, global: handler.global ?? false };.
| if (handler.global === undefined) { | |
| handler.global = false; | |
| } | |
| return handler; | |
| return { ...handler, global: handler.global ?? false }; |
Closes #63