Conversation
motirebuma
commented
Apr 20, 2026
- The post detail page has been implemented successfully.
- Post interactions, including voting (support or disagree), reporting posts, and bookmarking, have also been completed.
- The discussion feature for the post detail page has been implemented.
- UI refine to the post detail page.
- some minor fix on the displaying number of interactions on the Explore page for each posts
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
Implements and refines the post detail experience in quotevote-frontend, including a unified “Open Discussion” feed (votes/comments/quotes/messages), updated sharing/navigation URL handling, and broader avatar-type support.
Changes:
- Added a unified post interaction feed on the post detail page, with updated PostActions UI and chat send behavior.
- Introduced
toAppPostUrlplus a Next.js redirect to normalize legacy/post/...URLs to/dashboard/post/.... - Expanded avatar typing to support Avataaars-style data objects and updated multiple components/tests accordingly.
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| quotevote-frontend/src/types/post.ts | Adds AvataaarsData and broadens creator avatar typing. |
| quotevote-frontend/src/types/chat.ts | Broadens chat participant avatar typing for non-URL avatar data. |
| quotevote-frontend/src/lib/validation/submitPostSchema.ts | Adjusts post body validation (removes max length constraint). |
| quotevote-frontend/src/lib/utils/sanitizeUrl.ts | Adds toAppPostUrl helper for route normalization. |
| quotevote-frontend/src/lib/apollo/apollo-client.ts | Refines Apollo error handling and simplifies cache initialization. |
| quotevote-frontend/src/components/UserPosts/PostCard.tsx | Avoids passing non-string avatar values into avatar rendering. |
| quotevote-frontend/src/components/PostChat/PostChatSend.tsx | Updates optimistic message ID generation + refreshed input UI/behavior. |
| quotevote-frontend/src/components/PostActions/PostActionList.tsx | Updates action list rendering and empty/loading states. |
| quotevote-frontend/src/components/PostActions/PostActionCard.tsx | Major UI/interaction refresh for action cards, including copy/delete UX. |
| quotevote-frontend/src/components/Post/PostCard.tsx | Uses toAppPostUrl for share + navigation; guards avatar string type. |
| quotevote-frontend/src/components/Post/Post.tsx | Refines post detail layout/actions; adds Avataaars avatar rendering and voting toggle. |
| quotevote-frontend/src/components/Chat/UserSearchResults.tsx | Guards avatar rendering for non-string avatar values. |
| quotevote-frontend/src/components/Chat/QuoteHeaderMessage.tsx | Guards avatar rendering for non-string avatar values. |
| quotevote-frontend/src/components/Chat/MessageItemList.tsx | Guards avatar mapping for non-string avatar values. |
| quotevote-frontend/src/components/Chat/AddBuddyDialog.tsx | Guards avatar rendering for non-string avatar values. |
| quotevote-frontend/src/app/dashboard/post/[group]/[title]/[postId]/page.tsx | Reworks post detail page layout and creates a unified interaction feed. |
| quotevote-frontend/src/app/dashboard/layout.tsx | Adjusts dashboard container width/padding for post detail routes. |
| quotevote-frontend/src/app/dashboard/explore/ExploreContent.tsx | Moves explore filters to URL-driven state and updates toggles. |
| quotevote-frontend/src/tests/components/PostChat/PostChatSend.test.tsx | Updates tests for new chat send placeholder/behavior. |
| quotevote-frontend/src/tests/components/PostActions/PostActionList.test.tsx | Updates tests for new action list empty state and DOM structure. |
| quotevote-frontend/src/tests/components/PostActions/PostActionCard.test.tsx | Updates tests for revised PostActionCard UI and toast behavior. |
| quotevote-frontend/pnpm-lock.yaml | Adds lock entries for avataaars. |
| quotevote-frontend/package.json | Adds avataaars dependency. |
| quotevote-frontend/next.config.ts | Adds redirect from /post/... to /dashboard/post/.... |
Files not reviewed (1)
- quotevote-frontend/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
quotevote-frontend/src/lib/validation/submitPostSchema.ts:21
- The post body validation no longer enforces any maximum length. If the backend (or database) has a size limit, this will shift failures to runtime (mutation errors) and can also hurt client performance when rendering very large posts. Consider reinstating a
.max(...)consistent with the server constraint (or documenting why unlimited length is acceptable).
text: z
.string()
.min(1, 'Post content is required')
.refine(
(value) => !containsUrl(value),
{
message: 'Links are not allowed in the post body. Use the Citation URL field instead.',
}
),
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {!isEmpty(sortedActions) ? ( | ||
| <ul className="space-y-4"> | ||
| <div className="divide-y divide-border/60"> | ||
| {sortedActions.map((action) => ( | ||
| <li | ||
| key={action._id} | ||
| id={action._id} | ||
| className="w-full" | ||
| > | ||
| <div key={action._id} id={action._id}> | ||
| <PostActionCard | ||
| postAction={action} | ||
| postUrl={postUrl} | ||
| selected={`#${action._id}` === hash} | ||
| refetchPost={refetchPost} | ||
| /> | ||
| </li> | ||
| </div> | ||
| ))} | ||
| </ul> | ||
| </div> |
There was a problem hiding this comment.
PostActionList was changed from a semantic list (<ul>/<li>) to plain <div> wrappers. This removes list semantics for screen readers and can reduce navigability. Consider keeping <ul>/<li> (you can still use divide-y on a <ul>) or adding appropriate ARIA roles (e.g. role="list" / role="listitem").
| role="button" | ||
| tabIndex={0} | ||
| onClick={handleClick} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); handleClick() } }} |
There was a problem hiding this comment.
The outer container uses role="button" with an onKeyDown handler that triggers on Enter/Space, but the event will also bubble from child controls (e.g. Copy/Delete buttons). This can cause keyboard users to activate the card selection when they intended to activate a child button. Consider guarding with if (e.currentTarget !== e.target) return; (or moving keyboard handling to a real <button> element / using onKeyDownCapture with proper checks).
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); handleClick() } }} | |
| onKeyDown={(e) => { | |
| if (e.currentTarget !== e.target) return | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault() | |
| handleClick() | |
| } | |
| }} |
| <div className="flex items-center justify-between mt-2.5 -ml-1"> | ||
| <CommentReactions actionId={_id} reactions={actionReactions} /> | ||
| <div className="flex items-center gap-0.5 opacity-0 group-hover:opacity-100 transition-opacity duration-150"> | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={(e) => { | ||
| e.stopPropagation() | ||
| handleCopy() | ||
| }} | ||
| className="h-8 w-8" | ||
| onClick={(e) => { e.stopPropagation(); handleCopy() }} | ||
| className="h-7 w-7 text-muted-foreground/50 hover:text-muted-foreground hover:bg-muted/50" | ||
| aria-label="Copy link" | ||
| > | ||
| <Link2 className="h-4 w-4" /> | ||
| <Link2 className="h-3.5 w-3.5" /> | ||
| </Button> | ||
| {isOwner && ( | ||
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| onClick={(e) => { | ||
| e.stopPropagation() | ||
| handleDelete() | ||
| }} | ||
| className="h-8 w-8 text-red-500 hover:text-red-600" | ||
| onClick={(e) => { e.stopPropagation(); handleDelete() }} | ||
| className="h-7 w-7 text-muted-foreground/50 hover:text-destructive hover:bg-destructive/10" | ||
| aria-label="Delete" | ||
| > | ||
| <Trash2 className="h-4 w-4" /> | ||
| <Trash2 className="h-3.5 w-3.5" /> | ||
| </Button> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Copy/Delete controls are rendered with opacity-0 group-hover:opacity-100, which makes them effectively invisible on touch devices (no hover) and also invisible when focused via keyboard (focus ring on an invisible button). Consider also revealing them on group-focus-within/focus-visible, or keeping them visible at least on small screens so the actions remain discoverable and usable.
| * // '/dashboard/post/general/some-title/abc123' | ||
| */ | ||
| export const toAppPostUrl = (url: string): string => { | ||
| const clean = url.replace(/\?/g, '') |
There was a problem hiding this comment.
toAppPostUrl removes all ? characters (url.replace(/\?/g, '')). If the backend URL ever includes a real query string (e.g. /post/...?...), this will produce an invalid path (/post/...x=1) rather than stripping the query. Consider normalizing by removing the query string via split('?')[0] or new URL(...).pathname when the input is absolute, and leave other ? characters intact.
| const clean = url.replace(/\?/g, '') | |
| let clean: string | |
| try { | |
| clean = new URL(url).pathname | |
| } catch { | |
| clean = url.split('?')[0] | |
| } |
| /** Optional avatar URL or avataaars data object */ | ||
| avatar?: string | Record<string, unknown> | ||
| /** Optional contributor badge or role */ |
There was a problem hiding this comment.
avatar?: string | Record<string, unknown> loses type-safety and diverges from the AvataaarsData type introduced in types/post.ts. Consider reusing/importing AvataaarsData here (and in ExtendedChatMessage.user.avatar) so the shape is explicit and consistent across the app.
| const dateSubmitted = new Date() | ||
| const tempId = Array.from({ length: 24 }, () => Math.floor(Math.random() * 16).toString(16)).join('') | ||
|
|
||
| await createMessage({ | ||
| variables: { message }, | ||
| optimisticResponse: { | ||
| createMessage: { | ||
| __typename: 'Message' as const, | ||
| _id: dateSubmitted.toISOString(), | ||
| _id: tempId, | ||
| messageRoomId: messageRoomId || '', |
There was a problem hiding this comment.
The optimistic _id for a new message is generated with Math.random(), which is not collision-resistant and can produce duplicate IDs under rapid sends (breaking list rendering/cache updates). Prefer a deterministic client ID like crypto.randomUUID() (and/or a nanoid) and, if you need 24-hex Mongo-style IDs, derive it from crypto.getRandomValues.
| "@testing-library/jest-dom": "^6.9.1", | ||
| "@testing-library/react": "^16.3.0", | ||
| "@testing-library/user-event": "^14.6.1", | ||
| "avataaars": "^2.0.0", | ||
| "axios": "^1.13.2", |
There was a problem hiding this comment.
avataaars@2.0.0 declares a peer dependency on react: ^17.0.0 (see lockfile), but this app is on React 19. That mismatch can lead to pnpm peer warnings and potential runtime issues. Consider switching to an avatar package that supports React 19, or using a maintained fork/version with compatible peer deps.