feat: improve chat, dashboard, landing page, settings, and account completion#57
feat: improve chat, dashboard, landing page, settings, and account completion#570x3EF8 wants to merge 7 commits intohallofcodes:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the chat experience with improved conversation ordering and richer UI (badges/media viewing), and adds database-level cascade deletes to support safe conversation deletion.
Changes:
- Add DM sorting controls (newest/oldest/A–Z/Z–A), message search, and revamped chat layout with left/right sidebars.
- Restore/standardize rank badge styling/animations by moving badge CSS into
app/globals.cssand using class-based badges + icons. - Add a Supabase migration to enforce
ON DELETE CASCADEformessagesandconversation_participantswhen deleting a conversation.
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/migrations/20260327100000_cascade_conversation_delete.sql | Adds cascade FK constraints for conversation cleanup on delete. |
| package.json | Bumps @fortawesome/react-fontawesome to support updated icon usage. |
| package-lock.json | Updates lockfile to reflect dependency bump and npm resolution changes. |
| app/globals.css | Centralizes badge keyframes/styles so they’re shared across UI. |
| app/components/dashboard/Navbar.tsx | Changes default sidebar collapse behavior (desktop now collapsed). |
| app/components/chat/Player.tsx | Adds hint-hiding helper (currently introduces a duplicate declaration). |
| app/components/chat/Messages.tsx | Adds media navigation controls + updated badge rendering with icons. |
| app/components/chat/Conversations.tsx | Updates conversation list item layout/labels/initials. |
| app/components/LeaderboardTable.tsx | Removes inline injected badge CSS now that globals provide styles. |
| app/components/Chat.tsx | Major chat UI refactor: sorting/search, sidebars, media viewer, deletion flow, badge icon mapping. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const hideHint = () => { | ||
| if (hintTimeoutRef.current) window.clearTimeout(hintTimeoutRef.current); | ||
| setActiveHint(null); | ||
| setHintPos(null); | ||
| }; | ||
|
|
There was a problem hiding this comment.
hideHint is declared twice in this component (there is another const hideHint = ... later in the file). This will cause a TypeScript compile error due to redeclaration. Remove one of the declarations and keep a single hideHint implementation.
| const hideHint = () => { | |
| if (hintTimeoutRef.current) window.clearTimeout(hintTimeoutRef.current); | |
| setActiveHint(null); | |
| setHintPos(null); | |
| }; |
| @@ -1,23 +1,49 @@ | |||
| "use client"; | |||
| "use client"; | |||
There was a problem hiding this comment.
There is a leading BOM/zero-width character before the "use client" directive. Next.js requires the directive to be the very first bytes in the file; the BOM can prevent the component from being treated as a client component. Remove the BOM so the file starts exactly with "use client";.
| "use client"; | |
| "use client"; |
| @@ -1,4 +1,4 @@ | |||
| import { User } from "@supabase/supabase-js"; | |||
| import { User } from "@supabase/supabase-js"; | |||
There was a problem hiding this comment.
This file starts with a BOM/zero-width character before the first import. While it may not break runtime behavior, it commonly causes noisy diffs and occasional tooling issues. Please remove the BOM so the file begins with a normal import ....
| import { User } from "@supabase/supabase-js"; | |
| import { User } from "@supabase/supabase-js"; |
| // default to collapsed on desktop too | ||
| setCollapsed(true); | ||
| setMobileHidden(false); |
There was a problem hiding this comment.
This change makes the sidebar default to collapsed on desktop as well as mobile. Since the PR is focused on chat ordering/cascade delete, this looks like an unrelated UX change; if it is intentional, it should be called out in the PR description (or moved to a separate PR) to avoid surprising behavior changes.
| @@ -1,4 +1,4 @@ | |||
| "use client"; | |||
| "use client"; | |||
There was a problem hiding this comment.
There is a leading BOM/zero-width character before the "use client" directive. Next.js requires the directive to be the very first bytes in the file; the BOM can prevent the component from being treated as a client component. Remove the BOM so the file starts exactly with "use client";.
| "use client"; | |
| "use client"; |
app/components/Chat.tsx
Outdated
| const [showModal, setShowModal] = useState(false); | ||
| const [search, setSearch] = useState(""); | ||
| const [allUsers, setAllUsers] = useState<ChatUser[]>([]); | ||
| const [search, setSearch] = useState(""); const [messageSearch, setMessageSearch] = useState(""); const [allUsers, setAllUsers] = useState<ChatUser[]>([]); |
There was a problem hiding this comment.
These state declarations appear to have been accidentally concatenated onto a single line, which hurts readability and makes future diffs harder to review. Please split them into separate const declarations on their own lines.
| const [search, setSearch] = useState(""); const [messageSearch, setMessageSearch] = useState(""); const [allUsers, setAllUsers] = useState<ChatUser[]>([]); | |
| const [search, setSearch] = useState(""); | |
| const [messageSearch, setMessageSearch] = useState(""); | |
| const [allUsers, setAllUsers] = useState<ChatUser[]>([]); |
app/components/Chat.tsx
Outdated
| faEllipsisVertical, | ||
| faFlag, | ||
| faLock, | ||
| faGhost, |
There was a problem hiding this comment.
Several icons are imported here but never referenced in the file (e.g. faEllipsisVertical, faFlag, faLock). This will fail linting in many configurations and adds noise. Remove unused imports or wire them up where intended.
| faEllipsisVertical, | |
| faFlag, | |
| faLock, | |
| faGhost, |
| const allMediaAttachments = messages | ||
| .flatMap((m) => m.attachments || []) | ||
| .filter( | ||
| (a) => a?.mimetype?.startsWith("image/") || a?.mimetype?.startsWith("video/") | ||
| ) | ||
| .reverse(); |
There was a problem hiding this comment.
allMediaAttachments is rebuilt (including a .reverse()) on every render. With long message histories this can become a noticeable cost. Consider memoizing this derived list with useMemo (and potentially sharing the media-viewer logic with Messages to avoid duplicated implementations).
Summary
Note