feat: add image preview functionality with modal and thumbnail compon… #417
Conversation
…ents - Introduced `ImagePreviewModal` for displaying images with zoom and navigation controls. - Added `ImagePreviewThumbnail` for quick access to the first image and indication of additional images. - Integrated image preview components into the `KanbanCard`, allowing users to view images associated with features. - Implemented keyboard navigation and mouse wheel zooming for enhanced user experience.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 |
Shironex
added
Enhancement
Summary of ChangesHello @Shironex, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user interface by introducing robust image preview capabilities for Kanban cards. Users can now easily view attached images in a dedicated modal that offers advanced interaction features like zooming, panning, and seamless navigation between multiple images. This improves the visual context and accessibility of image-related information directly within the Kanban board. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a well-structured image preview functionality with a modal and thumbnail component. The implementation is comprehensive, covering features like zoom, pan, keyboard navigation, and loading/error states.
My review focuses on a few key areas for improvement:
- TypeScript Usage: All new components have disabled TypeScript checking with
// @ts-nocheck. This is a critical issue that should be addressed to leverage the full benefits of TypeScript. - Performance: There's an opportunity to optimize the rendering of thumbnails by memoizing their URLs.
- Code Clarity: A minor refactoring can simplify one of the
useEffecthooks by removing redundant state resets.
Overall, this is a great addition. Addressing these points will make the new components more robust and performant.
| @@ -0,0 +1,435 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
The use of // @ts-nocheck disables all TypeScript type checking for this file. This undermines the benefits of using TypeScript, such as static type safety and improved code quality. Please remove this comment and resolve any TypeScript errors that appear. This is crucial for maintaining a robust and error-free codebase.
| @@ -0,0 +1,154 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
The use of // @ts-nocheck disables all TypeScript type checking for this file. This undermines the benefits of using TypeScript, such as static type safety and improved code quality. Please remove this comment and resolve any TypeScript errors that appear. This is crucial for maintaining a robust and error-free codebase.
| @@ -1,5 +1,5 @@ | |||
| // @ts-nocheck | |||
There was a problem hiding this comment.
The use of // @ts-nocheck disables all TypeScript type checking for this file. This undermines the benefits of using TypeScript, such as static type safety and improved code quality. Please remove this comment and resolve any TypeScript errors that appear. This is crucial for maintaining a robust and error-free codebase.
| useEffect(() => { | ||
| if (open) { | ||
| setCurrentIndex(initialIndex); | ||
| setZoom(1); | ||
| setPosition({ x: 0, y: 0 }); | ||
| setIsLoading(true); | ||
| setImageError(false); | ||
| } | ||
| }, [open, initialIndex]); |
There was a problem hiding this comment.
There's some redundant state-setting logic here. The useEffect hook that runs when currentIndex changes (lines 70-75) already handles resetting zoom, position, isLoading, and imageError. Since opening the modal calls setCurrentIndex(initialIndex), which in turn triggers the other effect, you can simplify this effect to only set the current index. This avoids resetting the state twice and makes the component's logic clearer.
// Reset state when opening modal
useEffect(() => {
if (open) {
setCurrentIndex(initialIndex);
}
}, [open, initialIndex]);
| {imagePaths.map((img, index) => { | ||
| const thumbUrl = currentProject?.path | ||
| ? getAuthenticatedImageUrl(img.path, currentProject.path) | ||
| : null; |
There was a problem hiding this comment.
The thumbnail URLs are being generated inside the map function, which means getAuthenticatedImageUrl is called for every thumbnail on every render of the component. This can be inefficient, especially if there are many images. To improve performance, you can memoize the list of thumbnail URLs using useMemo.
For example, you can add this before the return statement:
const thumbnailUrls = useMemo(() => {
if (!currentProject?.path) {
return imagePaths.map(() => null);
}
return imagePaths.map((img) =>
getAuthenticatedImageUrl(img.path, currentProject.path)
);
}, [imagePaths, currentProject?.path]);And then use it in the map function:
{imagePaths.map((img, index) => {
const thumbUrl = thumbnailUrls[index];
// ... rest of the component
})}This will ensure the URLs are only recalculated when imagePaths or currentProject.path changes.
ImagePreviewModalfor displaying images with zoom and navigation controls.ImagePreviewThumbnailfor quick access to the first image and indication of additional images.KanbanCard, allowing users to view images associated with features.Preview