feat: add in-chat media gallery with lightbox image viewer Closes #499#536
Open
darshil2032007 wants to merge 1 commit into
Open
feat: add in-chat media gallery with lightbox image viewer Closes #499#536darshil2032007 wants to merge 1 commit into
darshil2032007 wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR implements a media gallery feature with lightbox viewer for browsing shared images in chats, alongside socket-based user activity tracking. The backend adds a media endpoint filtering image messages, while the frontend introduces a lightbox viewer and gallery grid, integrating them into the chat interface with a major refactor of ChatWindow and MessageBubble components. ChangesMedia Gallery Feature with Lightbox
Socket User Activity Tracking
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Author
|
Hey @UTKARSHH20! 👋 I've implemented the In-Chat Media Gallery with Lightbox Image Viewer Could you please review and merge this PR? 🙏 Here's a quick summary of what was done:
This is my contribution under GSSoC'26 for issue #499. Please let me know if any changes are needed! 😊 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/lib/socket.js`:
- Around line 19-24: The cache is being updated before the DB write so on
failure the timestamp remains and blocks retries; in the block around
lastDbUpdateCache.set(userId, now) and User.findByIdAndUpdate, capture the
previous cache value (e.g., prev = lastDbUpdateCache.get(userId)) before calling
set, then if User.findByIdAndUpdate throws restore the previous state (either
lastDbUpdateCache.set(userId, prev) if prev existed or
lastDbUpdateCache.delete(userId) if not) inside the catch, and keep the existing
console.error logging; reference lastDbUpdateCache, userId, now, and
User.findByIdAndUpdate when making the change.
In `@backend/src/routes/message.route.js`:
- Line 29: The current Mongo filter image: { $ne: null, $exists: true } allows
empty strings; update the query that builds the media filter (the object using
the image field in backend/src/routes/message.route.js) to explicitly exclude
empty strings and non-string types — e.g. change to image: { $exists: true, $ne:
"", $type: "string" } (or at minimum add $ne: ""), so only messages with a real
image value are returned.
- Around line 19-24: The GET /:id/media route handler is using req.user._id and
the Message model without importing it; change the handler to read the
authenticated user id from req.userId (provided by protectRoute) instead of
req.user._id, and add the missing import for the Message model (e.g., import or
require Message at the top of the file). Update references inside the
router.get("/:id/media", protectRoute, async (req, res) => { ... }) handler to
use myId = req.userId and ensure Message.find(...) uses the imported Message
model.
In `@frontend/components/chat/ChatWindow.jsx`:
- Around line 421-423: The code creates preview object URLs with
URL.createObjectURL but never revokes them, causing memory growth; update the
logic that calls setImagePreview(URL.createObjectURL(compressedFile)) to first
revoke any existing preview URL (e.g., if imagePreview state is non-null call
URL.revokeObjectURL(imagePreview)), revoke the previous URL before replacing and
also revoke on clear/send actions and in the component cleanup (useEffect
cleanup/unmount). Apply the same pattern wherever setImagePreview is used (the
other occurrences you noted) so every new createObjectURL is paired with a
revoke of the prior URL and revoked when the preview is no longer needed.
- Around line 993-1060: Remove the duplicated controls so there is only one
hidden file input (keep the input that uses fileRef, accept="image/*",
onChange={handleImage} and include disabled={processingImage}), one Image attach
button that triggers fileRef.current?.click() and respects processingImage, one
Emoji toggle button (keep the onClick that calls e.stopPropagation();
setShowEmoji(v => !v) and the conditional class using showEmoji), and a single
Schedule button that opens the schedule modal (keep disabled={!text.trim() &&
!imageBase64 && !audioBase64} and onClick={() => setShowScheduleModal(true)}).
Ensure unique references in these retained elements (fileRef, handleImage,
processingImage, setShowEmoji, showEmoji, setShowScheduleModal, text,
imageBase64, audioBase64) and delete the extra duplicated blocks so refs and
behaviors are consistent.
- Around line 772-801: The gallery currently reads only the loaded slice via
sharedMedia and thus misses paginated conversation media; replace the local
slice usage by wiring in the existing MediaGallery fetching logic (or call the
/messages/:id/media endpoint) so the component shows the full conversation media
set instead of only messages/sharedMedia. Specifically, in the block that
renders when showGallery is true, stop mapping sharedMedia directly and instead
render/use MediaGallery (or fetch via /messages/:id/media) to provide the images
array passed into ImageLightbox and control initialIndex via setLightboxIndex;
ensure ImageLightbox still receives images={...} and onClose={() =>
setLightboxIndex(null)} and that keys/use of setLightboxIndex remain intact.
In `@frontend/components/chat/MediaGallery.jsx`:
- Around line 11-25: When selectedUserId changes the fetchMedia effect can leave
stale images or let out-of-order responses overwrite state; clear images
immediately on user change, use an AbortController (or axios cancel token)
inside fetchMedia and return a cleanup that aborts in-flight requests, and on
catch setImages([]) (or clear) instead of leaving previous images; ensure you
only call setImages/res.setLoading if the request wasn’t aborted (check
controller.signal.aborted or use a local requestId) — update the
useEffect/fetchMedia logic, referencing useEffect, fetchMedia, setImages,
setLoading, axiosInstance.get and selectedUserId to implement these changes.
In `@frontend/components/chat/MessageBubble.jsx`:
- Line 23: The component defines state selectedImage and updates it via
setSelectedImage on image clicks but never uses selectedImage in the render, so
clicks have no visible effect; update the MessageBubble component to consume
selectedImage (e.g., render a Lightbox/Modal when selectedImage is non-null) and
provide a way to close it (clear selectedImage with setSelectedImage(null));
locate the image click handler that calls setSelectedImage and the selectedImage
state declaration to wire the modal/lightbox rendering and the close handler.
- Line 72: The reply preview is hidden when a reply only contains attachments
because the conditional uses msg.replyTo?.message; change the guard in
MessageBubble.jsx to check for the existence of the reply object instead (e.g.,
use msg.replyTo or msg.replyTo != null) so ReplyPreview is rendered for
attachment-only replies; keep ReplyPreview's handling of absent text (the "📎
Attachment" behavior) unchanged and only update the conditional around the JSX
that renders the ReplyPreview.
- Around line 80-86: The MessageBubble component is rendering the "Jump to
parent message" link twice; remove the duplicate element so only one clickable
text remains. Locate the two identical divs in MessageBubble.jsx (the elements
with className "text-[10px] text-primary cursor-pointer hover:underline mb-1")
and delete one of them, ensuring any event handler or props (if attached
elsewhere in the component) remain wired to the single remaining element.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cca54160-e51e-43fa-b263-e470f68dfd3f
📒 Files selected for processing (6)
backend/src/lib/socket.jsbackend/src/routes/message.route.jsfrontend/components/chat/ChatWindow.jsxfrontend/components/chat/ImageLightbox.jsxfrontend/components/chat/MediaGallery.jsxfrontend/components/chat/MessageBubble.jsx
Comment on lines
+19
to
+24
| lastDbUpdateCache.set(userId, now); | ||
| try { | ||
| await User.findByIdAndUpdate(userId, { lastSeen: new Date() }); | ||
| } catch (err) { | ||
| console.error("Error updating lastSeen:", err); | ||
| } |
There was a problem hiding this comment.
Rollback throttle cache when lastSeen DB write fails.
The cache timestamp is set before the DB write, but on error it is never reverted. That suppresses retries for 30s despite no successful update, which can leave lastSeen stale.
Suggested fix
const throttledUpdateLastSeen = async (userId) => {
const now = Date.now();
const lastUpdate = lastDbUpdateCache.get(userId) || 0;
if (now - lastUpdate < 30000) return; // 30 second throttle
lastDbUpdateCache.set(userId, now);
try {
await User.findByIdAndUpdate(userId, { lastSeen: new Date() });
} catch (err) {
+ lastDbUpdateCache.delete(userId); // allow immediate retry after transient failure
console.error("Error updating lastSeen:", err);
}
};📝 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.
Suggested change
| lastDbUpdateCache.set(userId, now); | |
| try { | |
| await User.findByIdAndUpdate(userId, { lastSeen: new Date() }); | |
| } catch (err) { | |
| console.error("Error updating lastSeen:", err); | |
| } | |
| lastDbUpdateCache.set(userId, now); | |
| try { | |
| await User.findByIdAndUpdate(userId, { lastSeen: new Date() }); | |
| } catch (err) { | |
| lastDbUpdateCache.delete(userId); // allow immediate retry after transient failure | |
| console.error("Error updating lastSeen:", err); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/lib/socket.js` around lines 19 - 24, The cache is being updated
before the DB write so on failure the timestamp remains and blocks retries; in
the block around lastDbUpdateCache.set(userId, now) and User.findByIdAndUpdate,
capture the previous cache value (e.g., prev = lastDbUpdateCache.get(userId))
before calling set, then if User.findByIdAndUpdate throws restore the previous
state (either lastDbUpdateCache.set(userId, prev) if prev existed or
lastDbUpdateCache.delete(userId) if not) inside the catch, and keep the existing
console.error logging; reference lastDbUpdateCache, userId, now, and
User.findByIdAndUpdate when making the change.
Comment on lines
+19
to
+24
| router.get("/:id/media", protectRoute, async (req, res) => { | ||
| try { | ||
| const myId = req.user._id; | ||
| const theirId = req.params.id; | ||
|
|
||
| const mediaMessages = await Message.find({ |
There was a problem hiding this comment.
Media endpoint will 500 due auth field mismatch and missing model import.
protectRoute provides req.userId, but this handler reads req.user._id, and Message is used without an import. Either condition can break the route at runtime.
Suggested fix
import express from "express";
import protectRoute from "../middleware/auth.middleware.js";
+import Message from "../models/message.model.js";
@@
- const myId = req.user._id;
+ const myId = req.userId;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/routes/message.route.js` around lines 19 - 24, The GET /:id/media
route handler is using req.user._id and the Message model without importing it;
change the handler to read the authenticated user id from req.userId (provided
by protectRoute) instead of req.user._id, and add the missing import for the
Message model (e.g., import or require Message at the top of the file). Update
references inside the router.get("/:id/media", protectRoute, async (req, res) =>
{ ... }) handler to use myId = req.userId and ensure Message.find(...) uses the
imported Message model.
| { senderId: myId, receiverId: theirId }, | ||
| { senderId: theirId, receiverId: myId }, | ||
| ], | ||
| image: { $ne: null, $exists: true }, |
There was a problem hiding this comment.
Current media filter still includes non-image messages.
image: { $ne: null, $exists: true } matches empty strings, and your schema defaults image to "". That means text-only messages can leak into this endpoint.
Suggested fix
- image: { $ne: null, $exists: true },
+ image: { $nin: [null, ""] },📝 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.
Suggested change
| image: { $ne: null, $exists: true }, | |
| image: { $nin: [null, ""] }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/routes/message.route.js` at line 29, The current Mongo filter
image: { $ne: null, $exists: true } allows empty strings; update the query that
builds the media filter (the object using the image field in
backend/src/routes/message.route.js) to explicitly exclude empty strings and
non-string types — e.g. change to image: { $exists: true, $ne: "", $type:
"string" } (or at minimum add $ne: ""), so only messages with a real image value
are returned.
Comment on lines
+421
to
+423
| setImagePreview(URL.createObjectURL(compressedFile)); | ||
| setImageBase64(reader.result); | ||
| setProcessingImage(false); |
There was a problem hiding this comment.
Preview object URLs should be revoked to avoid memory growth.
URL.createObjectURL is created per selected image but never revoked when replacing/clearing/sending the preview.
Suggested fix
reader.onloadend = () => {
- setImagePreview(URL.createObjectURL(compressedFile));
+ setImagePreview((prev) => {
+ if (prev) URL.revokeObjectURL(prev);
+ return URL.createObjectURL(compressedFile);
+ });
setImageBase64(reader.result);
setProcessingImage(false);
};
@@
onClick={() => {
+ if (imagePreview) URL.revokeObjectURL(imagePreview);
setImagePreview(null);
setImageBase64(null);
}}
@@
setShowEmoji(false);
setText("");
+ if (imagePreview) URL.revokeObjectURL(imagePreview);
setImagePreview(null);
setImageBase64(null);Also applies to: 913-916, 460-463
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/components/chat/ChatWindow.jsx` around lines 421 - 423, The code
creates preview object URLs with URL.createObjectURL but never revokes them,
causing memory growth; update the logic that calls
setImagePreview(URL.createObjectURL(compressedFile)) to first revoke any
existing preview URL (e.g., if imagePreview state is non-null call
URL.revokeObjectURL(imagePreview)), revoke the previous URL before replacing and
also revoke on clear/send actions and in the component cleanup (useEffect
cleanup/unmount). Apply the same pattern wherever setImagePreview is used (the
other occurrences you noted) so every new createObjectURL is paired with a
revoke of the prior URL and revoked when the preview is no longer needed.
Comment on lines
+772
to
801
| {showGallery && ( | ||
| <div className="border-b border-base-200 bg-base-100 p-3"> | ||
| <h3 className="font-semibold text-sm mb-3">Shared Media Gallery</h3> | ||
|
|
||
| <div className="grid grid-cols-3 gap-2 max-h-64 overflow-y-auto"> | ||
| {sharedMedia.map((media, index) => ( | ||
| <img | ||
| key={media._id} | ||
| src={media.image} | ||
| alt="shared media" | ||
| className="rounded-lg object-cover h-24 w-full cursor-pointer hover:scale-105 transition" | ||
| onClick={() => setLightboxIndex(index)} | ||
| /> | ||
| ))} | ||
| </div> | ||
|
|
||
| {sharedMedia.length === 0 && ( | ||
| {sharedMedia.length === 0 && ( | ||
| <p className="text-xs text-base-content/50"> | ||
| No shared media found | ||
| No shared media found | ||
| </p> | ||
| )} | ||
| </div> | ||
| )} | ||
|
|
||
| <div ref={chatContainerRef} onScroll={handleScroll} style={{ overflowAnchor: "none" }} className="flex-1 overflow-y-auto overflow-x-hidden px-4 py-4 space-y-1 overscroll-contain"> | ||
| {showNotes && ( | ||
| <div className="border-b border-base-200 p-3 bg-base-200"> | ||
| <div className="flex justify-between items-center mb-2"> | ||
| <h3 className="font-semibold text-sm"> | ||
| Collaborative Notes | ||
| </h3> | ||
|
|
||
| <span className="badge badge-primary badge-sm"> | ||
| Shared | ||
| </span> | ||
| </div> | ||
|
|
||
| <textarea | ||
| value={sharedNotes} | ||
| onChange={(e) => | ||
| setSharedNotes(e.target.value) | ||
| } | ||
| placeholder="Write shared notes here..." | ||
| className="textarea textarea-bordered w-full h-32" | ||
| /> | ||
| )} | ||
|
|
||
| <div className="text-xs text-base-content/50 mt-2"> | ||
| Changes are visible to all participants | ||
| {lightboxIndex !== null && ( | ||
| <ImageLightbox | ||
| images={sharedMedia.map((m) => m.image)} | ||
| initialIndex={lightboxIndex} | ||
| onClose={() => setLightboxIndex(null)} | ||
| /> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Gallery is scoped to loaded chat slice, not full conversation media.
This block reads from messages/sharedMedia, so with pagination it won’t show all shared images. The PR already introduced MediaGallery + /messages/:id/media; wire that here to meet the feature contract.
Suggested fix
-import ImageLightbox from "./ImageLightbox";
+import MediaGallery from "./MediaGallery";
@@
- {showGallery && (
- <div className="border-b border-base-200 bg-base-100 p-3">
- <h3 className="font-semibold text-sm mb-3">Shared Media Gallery</h3>
-
- <div className="grid grid-cols-3 gap-2 max-h-64 overflow-y-auto">
- {sharedMedia.map((media, index) => (
- <img
- key={media._id}
- src={media.image}
- alt="shared media"
- className="rounded-lg object-cover h-24 w-full cursor-pointer hover:scale-105 transition"
- onClick={() => setLightboxIndex(index)}
- />
- ))}
- </div>
-
- {sharedMedia.length === 0 && (
- <p className="text-xs text-base-content/50">
- No shared media found
- </p>
- )}
-
- {lightboxIndex !== null && (
- <ImageLightbox
- images={sharedMedia.map((m) => m.image)}
- initialIndex={lightboxIndex}
- onClose={() => setLightboxIndex(null)}
- />
- )}
- </div>
- )}
+ {showGallery && (
+ <div className="border-b border-base-200 bg-base-100 p-3">
+ <h3 className="font-semibold text-sm mb-3">Shared Media Gallery</h3>
+ <MediaGallery selectedUserId={selectedUser._id} />
+ </div>
+ )}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/components/chat/ChatWindow.jsx` around lines 772 - 801, The gallery
currently reads only the loaded slice via sharedMedia and thus misses paginated
conversation media; replace the local slice usage by wiring in the existing
MediaGallery fetching logic (or call the /messages/:id/media endpoint) so the
component shows the full conversation media set instead of only
messages/sharedMedia. Specifically, in the block that renders when showGallery
is true, stop mapping sharedMedia directly and instead render/use MediaGallery
(or fetch via /messages/:id/media) to provide the images array passed into
ImageLightbox and control initialIndex via setLightboxIndex; ensure
ImageLightbox still receives images={...} and onClose={() =>
setLightboxIndex(null)} and that keys/use of setLightboxIndex remain intact.
Comment on lines
+993
to
+1060
| <button | ||
| onClick={() => fileRef.current?.click()} | ||
| disabled={processingImage} | ||
| className="btn btn-ghost btn-sm btn-square shrink-0" | ||
| title="Attach image" | ||
| > | ||
| <Image className="w-4 h-4 text-base-content/50" /> | ||
| </button> | ||
| <input | ||
| type="file" | ||
| ref={fileRef} | ||
| accept="image/*" | ||
| className="hidden" | ||
| onChange={handleImage} | ||
| disabled={processingImage} | ||
| /> | ||
| <button | ||
| onClick={() => toast.success("Message scheduling coming soon!")} | ||
| className="btn btn-ghost btn-sm btn-square shrink-0" | ||
| title="Schedule Message" | ||
| > | ||
| <Clock className="w-4 h-4 text-base-content/50" /> | ||
| </button> | ||
|
|
||
| {processingImage && ( | ||
| <div className="flex items-center gap-2 text-xs text-primary"> | ||
| <Loader2 className="w-3 h-3 animate-spin" /> | ||
| Compressing image... | ||
| </div> | ||
| )} | ||
|
|
||
| <ScheduleMessageModal | ||
| isOpen={showScheduleModal} | ||
| onClose={() => setShowScheduleModal(false)} | ||
| receiverId={selectedUser?._id} | ||
| messageContent={{ | ||
| message: text, | ||
| image: imageBase64, | ||
| audio: audioBase64, | ||
| }} | ||
| <button | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| setShowEmoji((v) => !v); | ||
| }} | ||
| className={`btn btn-ghost btn-sm btn-square shrink-0 ${ | ||
| showEmoji ? "text-primary" : "text-base-content/50" | ||
| }`} | ||
| title="Emoji" | ||
| > | ||
| <Smile className="w-4 h-4" /> | ||
| </button> | ||
| <input | ||
| type="file" | ||
| ref={fileRef} | ||
| accept="image/*" | ||
| className="hidden" | ||
| onChange={handleImage} | ||
| /> | ||
| </div> | ||
| ) | ||
| <button | ||
| onClick={() => setShowScheduleModal(true)} | ||
| disabled={!text.trim() && !imageBase64 && !audioBase64} | ||
| className="btn btn-ghost btn-sm btn-square shrink-0" | ||
| title="Schedule Message" | ||
| > | ||
| <Clock className="w-4 h-4 text-base-content/50" /> | ||
| </button> | ||
| <button | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| setShowEmoji((v) => !v); | ||
| }} | ||
| className={`btn btn-ghost btn-sm btn-square shrink-0 ${showEmoji ? "text-primary" : "text-base-content/50"}`} | ||
| title="Emoji" | ||
| > | ||
| <Smile className="w-4 h-4" /> | ||
| </button> |
There was a problem hiding this comment.
Composer controls are duplicated with conflicting behavior.
There are two hidden file inputs bound to the same fileRef, two Emoji buttons, and two Schedule buttons (toast-only + modal). This creates inconsistent UI and unpredictable ref targeting.
Suggested fix
- <button
- onClick={() => toast.success("Message scheduling coming soon!")}
- className="btn btn-ghost btn-sm btn-square shrink-0"
- title="Schedule Message"
- >
- <Clock className="w-4 h-4 text-base-content/50" />
- </button>
@@
- <button
- onClick={(e) => {
- e.stopPropagation();
- setShowEmoji((v) => !v);
- }}
- className={`btn btn-ghost btn-sm btn-square shrink-0 ${
- showEmoji ? "text-primary" : "text-base-content/50"
- }`}
- title="Emoji"
- >
- <Smile className="w-4 h-4" />
- </button>
- <input
- type="file"
- ref={fileRef}
- accept="image/*"
- className="hidden"
- onChange={handleImage}
- />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/components/chat/ChatWindow.jsx` around lines 993 - 1060, Remove the
duplicated controls so there is only one hidden file input (keep the input that
uses fileRef, accept="image/*", onChange={handleImage} and include
disabled={processingImage}), one Image attach button that triggers
fileRef.current?.click() and respects processingImage, one Emoji toggle button
(keep the onClick that calls e.stopPropagation(); setShowEmoji(v => !v) and the
conditional class using showEmoji), and a single Schedule button that opens the
schedule modal (keep disabled={!text.trim() && !imageBase64 && !audioBase64} and
onClick={() => setShowScheduleModal(true)}). Ensure unique references in these
retained elements (fileRef, handleImage, processingImage, setShowEmoji,
showEmoji, setShowScheduleModal, text, imageBase64, audioBase64) and delete the
extra duplicated blocks so refs and behaviors are consistent.
Comment on lines
+11
to
+25
| useEffect(() => { | ||
| const fetchMedia = async () => { | ||
| try { | ||
| setLoading(true); | ||
| const res = await axiosInstance.get(`/messages/${selectedUserId}/media`); | ||
| setImages(res.data); | ||
| } catch (err) { | ||
| console.error("Failed to fetch media:", err); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| if (selectedUserId) fetchMedia(); | ||
| }, [selectedUserId]); |
There was a problem hiding this comment.
Prevent stale media showing from another chat on request race/failure.
This effect can display wrong images when requests resolve out of order, and failed fetches keep old images state. Clear state on user change/error and guard/cancel in-flight requests.
Suggested fix
useEffect(() => {
+ if (!selectedUserId) {
+ setImages([]);
+ setLoading(false);
+ return;
+ }
+
+ const controller = new AbortController();
+ let active = true;
+
const fetchMedia = async () => {
try {
setLoading(true);
- const res = await axiosInstance.get(`/messages/${selectedUserId}/media`);
- setImages(res.data);
+ setImages([]);
+ const res = await axiosInstance.get(`/messages/${selectedUserId}/media`, {
+ signal: controller.signal,
+ });
+ if (active) setImages(res.data);
} catch (err) {
- console.error("Failed to fetch media:", err);
+ if (active) setImages([]);
} finally {
- setLoading(false);
+ if (active) setLoading(false);
}
};
- if (selectedUserId) fetchMedia();
+ fetchMedia();
+ return () => {
+ active = false;
+ controller.abort();
+ };
}, [selectedUserId]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/components/chat/MediaGallery.jsx` around lines 11 - 25, When
selectedUserId changes the fetchMedia effect can leave stale images or let
out-of-order responses overwrite state; clear images immediately on user change,
use an AbortController (or axios cancel token) inside fetchMedia and return a
cleanup that aborts in-flight requests, and on catch setImages([]) (or clear)
instead of leaving previous images; ensure you only call
setImages/res.setLoading if the request wasn’t aborted (check
controller.signal.aborted or use a local requestId) — update the
useEffect/fetchMedia logic, referencing useEffect, fetchMedia, setImages,
setLoading, axiosInstance.get and selectedUserId to implement these changes.
| onReact, | ||
| }) { | ||
| const [showTranslation, setShowTranslation] = useState(false); | ||
| const [selectedImage, setSelectedImage] = useState(null); |
There was a problem hiding this comment.
Image click updates state with no UI effect.
selectedImage is set on click (Line 99), but never consumed in render, so the click action appears broken despite pointer affordance.
Suggested fix (if lightbox is intended here)
+import ImageLightbox from "./ImageLightbox";
...
const [selectedImage, setSelectedImage] = useState(null);
...
{msg.image && (
<img
src={msg.image}
alt="attachment"
className="max-w-full rounded-lg mb-1 cursor-pointer hover:opacity-90 transition"
onClick={() => setSelectedImage(msg.image)}
/>
)}
+ {selectedImage && (
+ <ImageLightbox
+ images={[{ image: selectedImage }]}
+ initialIndex={0}
+ onClose={() => setSelectedImage(null)}
+ />
+ )}Also applies to: 99-99
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/components/chat/MessageBubble.jsx` at line 23, The component defines
state selectedImage and updates it via setSelectedImage on image clicks but
never uses selectedImage in the render, so clicks have no visible effect; update
the MessageBubble component to consume selectedImage (e.g., render a
Lightbox/Modal when selectedImage is non-null) and provide a way to close it
(clear selectedImage with setSelectedImage(null)); locate the image click
handler that calls setSelectedImage and the selectedImage state declaration to
wire the modal/lightbox rendering and the close handler.
| <span>Pinned Message</span> | ||
| </div> | ||
| )} | ||
| {msg.replyTo?.message && ( |
There was a problem hiding this comment.
Reply preview is incorrectly hidden for attachment-only replies.
Line 72 checks msg.replyTo?.message, but ReplyPreview already handles missing text (📎 Attachment). This suppresses valid reply metadata for media/file replies.
Suggested fix
- {msg.replyTo?.message && (
+ {msg.replyTo && (📝 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.
Suggested change
| {msg.replyTo?.message && ( | |
| {msg.replyTo && ( |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/components/chat/MessageBubble.jsx` at line 72, The reply preview is
hidden when a reply only contains attachments because the conditional uses
msg.replyTo?.message; change the guard in MessageBubble.jsx to check for the
existence of the reply object instead (e.g., use msg.replyTo or msg.replyTo !=
null) so ReplyPreview is rendered for attachment-only replies; keep
ReplyPreview's handling of absent text (the "📎 Attachment" behavior) unchanged
and only update the conditional around the JSX that renders the ReplyPreview.
Comment on lines
+80
to
+86
| <div className="text-[10px] text-primary cursor-pointer hover:underline mb-1"> | ||
| Jump to parent message | ||
| </div> | ||
|
|
||
| <div className="text-[10px] text-primary cursor-pointer hover:underline mb-1"> | ||
| Jump to parent message | ||
| </div> | ||
| <div className="text-[10px] text-primary cursor-pointer hover:underline mb-1"> | ||
| Jump to parent message | ||
| </div> |
There was a problem hiding this comment.
Duplicate “Jump to parent message” is rendered twice.
This produces duplicated UI in every reply bubble.
Suggested fix
<div className="text-[10px] text-primary cursor-pointer hover:underline mb-1">
Jump to parent message
</div>
-
- <div className="text-[10px] text-primary cursor-pointer hover:underline mb-1">
- Jump to parent message
- </div>📝 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.
Suggested change
| <div className="text-[10px] text-primary cursor-pointer hover:underline mb-1"> | |
| Jump to parent message | |
| </div> | |
| <div className="text-[10px] text-primary cursor-pointer hover:underline mb-1"> | |
| Jump to parent message | |
| </div> | |
| <div className="text-[10px] text-primary cursor-pointer hover:underline mb-1"> | |
| Jump to parent message | |
| </div> | |
| <div className="text-[10px] text-primary cursor-pointer hover:underline mb-1"> | |
| Jump to parent message | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/components/chat/MessageBubble.jsx` around lines 80 - 86, The
MessageBubble component is rendering the "Jump to parent message" link twice;
remove the duplicate element so only one clickable text remains. Locate the two
identical divs in MessageBubble.jsx (the elements with className "text-[10px]
text-primary cursor-pointer hover:underline mb-1") and delete one of them,
ensuring any event handler or props (if attached elsewhere in the component)
remain wired to the single remaining element.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #499
This PR implements the In-Chat Media Gallery feature with a fullscreen
Lightbox Image Viewer as described in issue #499.
Changes Made
Frontend
MediaGallery.jsx— New component showing all shared images in a responsive 3-column gridImageLightbox.jsx— New fullscreen lightbox viewer with navigation and downloadChatWindow.jsx— Added gallery toggle button and integrated MediaGallery + ImageLightboxBackend
socket.js— Fixed missingthrottledUpdateLastSeenandlastDbUpdateCachefunctionsmessage.route.js— Added/api/messages/:id/mediaendpoint returning only image messagesFeatures Implemented
Screenshots
🖼️ Media Gallery Grid
🔍 Lightbox Fullscreen View
📭 Empty State
Testing Done
GSSoC'26 Contribution 🚀
Summary by CodeRabbit
New Features
Style