Cropped Image-fix in addImagesCropDialog - #1466
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe album image grid preserves image aspect ratios, uses theme-aware placeholders, lazy-loads thumbnails, adds hover scaling, provides filename-based alt text, and applies conditional selection borders. ChangesAlbum thumbnail UI
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
frontend/src/components/Albums/AddImagesToAlbumDialog.tsx (1)
152-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the typed image target.
Because the handler is attached to the
<img>,e.currentTargetprovides the typedHTMLImageElement. Replacee.target as HTMLImageElementwithe.currentTargetto remove the unnecessary assertion.Proposed fix
- const img = e.target as HTMLImageElement; + const img = e.currentTarget;As per coding guidelines:
frontend/src/**/*.{ts,tsx}says not to useasto silence genuine type errors.🤖 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/src/components/Albums/AddImagesToAlbumDialog.tsx` around lines 152 - 154, Update the onError handler on the image element to assign e.currentTarget directly to img instead of casting e.target to HTMLImageElement, preserving the existing img.onerror behavior without using a type assertion.Source: Coding guidelines
🤖 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 `@frontend/src/components/Albums/AddImagesToAlbumDialog.tsx`:
- Around line 138-143: Update the thumbnail element in AddImagesToAlbumDialog to
remove the inline borderColor style and conditionally apply border-primary or
border-transparent through the existing cn() utility based on
selectedImages.has(image.id), preserving hover:border-primary for unselected
thumbnails.
- Around line 155-160: Update the fallback placeholder selection in the
image-loading logic of AddImagesToAlbumDialog to use the application theme from
useTheme(), matching AlbumCard, instead of
window.matchMedia('prefers-color-scheme'). Reuse the existing theme-derived
value or shared placeholder helper and preserve the dark/light placeholder
mapping.
- Line 138: Remove the hover scale styling from the preview image container in
AddImagesToAlbumDialog, specifically the group-hover:scale-105 behavior, while
preserving the aspect-square layout and other existing thumbnail styles.
---
Nitpick comments:
In `@frontend/src/components/Albums/AddImagesToAlbumDialog.tsx`:
- Around line 152-154: Update the onError handler on the image element to assign
e.currentTarget directly to img instead of casting e.target to HTMLImageElement,
preserving the existing img.onerror behavior without using a type assertion.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b6f575c7-543e-49ca-bb8d-888a6abf57fe
📒 Files selected for processing (1)
frontend/src/components/Albums/AddImagesToAlbumDialog.tsx
|
@rohan-pandeyy can you please review this PR. |
1 participant
Addressed Issues:
Fixes #1459 : Images appear cropped in the "Add Image" window when adding images to an album.
Screenshots/Recordings:
PictoPy Before:
631599087-26e43e8f-f2c5-45fd-95b6-70db8b850195.mov
PictoPy After:
Portraits:
Landscapes:
Additional Notes:
File Changed: 1
frontend/src/components/Albums/addImagesToAlbumDialog:
A little bit of css changes:
Fixes:
One Flaw:
The landscape pictures have a little bit of a blank space on the up and down side of the photo box, but if i fix it , it will end up in the crop. It is possible but it will cut a little bit part from landscape images, currently it shows the whole image.
(if you want me to fix it i can, but i kind of prefer the current version)
AI Usage Disclosure:
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact. AI slop is strongly discouraged and may lead to banning and blocking. Do not spam our repos with AI slop.
Check one of the checkboxes below:
I have used the following AI models and tools: Claude Sonnet - 5
Checklist
Summary by CodeRabbit