feat(platform): chat image gallery and zoom-pan viewer#754
Merged
Conversation
…onent Refactor image preview dialog to support multi-image gallery with prev/next navigation via arrow keys and buttons. Extract zoom/pan logic into a reusable ZoomPanViewer component with pinch-to-zoom and mouse wheel support. Consolidate image thumbnails to consistent size-9 with ring borders.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughThis PR introduces a new ZoomPanViewer component and useZoomPan hook for image zooming and panning functionality. The component supports three toolbar layouts (overlay, bottom, inline) and is integrated into the chat image preview system, replacing the previous zoom/pan implementation. Integration includes adding an image gallery mechanism to message bubbles with multi-image navigation, updating file display components with image click handlers, and adding comprehensive Storybook stories and test coverage. Minor styling adjustments are applied to image attachments and preview buttons across the chat UI, with corresponding i18n message updates. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@services/platform/app/components/ui/data-display/zoom-pan-viewer.stories.tsx`:
- Around line 6-7: The SAMPLE_IMAGE constant currently points to an external
Unsplash URL which makes stories flaky; replace it with a bundled fixture or
inline data URI and update references to SAMPLE_IMAGE in this file. Add a local
image asset (e.g., put a small PNG/JPG in the same folder or a fixtures/assets
folder), import it at the top (e.g., import sampleImg from
'./fixtures/sample-image.png') or replace the string with a base64 data URI,
then assign SAMPLE_IMAGE = sampleImg so the ZoomPanViewer stories use the
local/bundled asset instead of the external URL.
In `@services/platform/app/components/ui/data-display/zoom-pan-viewer.test.tsx`:
- Around line 83-98: Tests for toolbar positioning use fragile CSS class
selectors; update the ZoomPanViewer component to add stable data-testid
attributes (e.g., data-testid="toolbar-overlay" and
data-testid="toolbar-inline") on the respective toolbar container elements, then
update the test file zoom-pan-viewer.test.tsx to queryByTestId / getByTestId for
"toolbar-overlay" and "toolbar-inline" instead of using
container.querySelector('.absolute.top-4') and '.justify-end.mb-2'; ensure the
test assertions still call toBeInTheDocument() on the elements returned from
render.
In `@services/platform/app/components/ui/data-display/zoom-pan-viewer.tsx`:
- Around line 82-110: The three raw button elements in the ZoomPanViewer UI (the
buttons wired to the zoomOut, zoomIn, and reset handlers that use buttonClass
and aria-labels like t('imagePreview.zoomOut')/zoomIn/resetZoom') default to
type="submit" and can accidentally submit enclosing forms; update each of those
<button> elements to explicitly include type="button" to prevent form submission
while preserving their onClick, disabled, className and aria-label props.
In
`@services/platform/app/features/chat/components/message-bubble/image-preview-dialog.tsx`:
- Around line 40-41: The dialog uses activeIndex directly even when images
changes, causing invalid counters; clamp activeIndex to the valid range before
using it in currentSrc/currentAlt and the counter. In image-preview-dialog
(variables currentSrc/currentAlt and the counter render), compute a safeIndex =
Math.max(0, Math.min(activeIndex, (images?.length ?? 1) - 1)) (or equivalent)
and use safeIndex everywhere you currently use activeIndex so the viewer and
"activeIndex + 1 / images.length" stay consistent when images updates. Ensure
the same clamped index is used in the viewer logic and the block that renders
the counter.
- Around line 22-25: The gallery mode should only be enabled when navigation is
actually wired; update the logic that computes isGallery (used with images,
activeIndex, onActiveIndexChange) so it returns true only when images?.length >
1 AND onActiveIndexChange is provided (and activeIndex is not undefined), and
hide/disable Prev/Next controls and keyboard ArrowLeft/ArrowRight handlers when
onActiveIndexChange is absent; ensure functions like the prev/next click
handlers and the keyboard handler early-returning behavior is matched by the
isGallery guard so controls are never shown or active unless onActiveIndexChange
(and a valid activeIndex) exist.
In `@services/platform/app/hooks/use-zoom-pan.ts`:
- Around line 94-102: handlePointerMove allows unrestricted panning so the image
can be dragged completely off-screen; clamp pan before calling setPan by
computing visible bounds from the container size and scaled image size and
limiting pan.x/pan.y to min/max values. In the handlePointerMove callback
(referencing dragStart, panStart, isDragging, setPan) calculate imageWidth =
naturalWidth * scale and imageHeight = naturalHeight * scale (or read computed
sizes), compute maxPanX/minPanX and maxPanY/minPanY based on container
dimensions and image dimensions, then clamp the computed newX/newY into those
ranges and pass the clamped values to setPan; keep existing isDragging guard and
dependency list.
In `@services/platform/messages/en.json`:
- Around line 408-411: Move the three zoom translation keys from the
dialogs.imagePreview section into the common.imagePreview namespace so
ZoomPanViewer can find them via t('imagePreview.zoomIn') etc.; specifically
relocate "zoomIn", "zoomOut", and "resetZoom" entries into the
common.imagePreview object in en.json (remove them from dialogs.imagePreview) so
the component using common namespace resolves correctly.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3549b330-250f-4108-9664-f3e9d3086949
📒 Files selected for processing (11)
services/platform/app/components/ui/data-display/file-preview-card.tsxservices/platform/app/components/ui/data-display/zoom-pan-viewer.stories.tsxservices/platform/app/components/ui/data-display/zoom-pan-viewer.test.tsxservices/platform/app/components/ui/data-display/zoom-pan-viewer.tsxservices/platform/app/features/chat/components/chat-input.tsxservices/platform/app/features/chat/components/message-bubble.tsxservices/platform/app/features/chat/components/message-bubble/file-displays.tsxservices/platform/app/features/chat/components/message-bubble/image-preview-dialog.tsxservices/platform/app/features/chat/components/message-bubble/markdown-renderer.tsxservices/platform/app/hooks/use-zoom-pan.tsservices/platform/messages/en.json
Comment on lines
+6
to
+7
| const SAMPLE_IMAGE = | ||
| 'https://images.unsplash.com/photo-1506905925346-21bda4d32df4?w=800&h=600&fit=crop'; |
There was a problem hiding this comment.
Use a local fixture for the story image.
Relying on a third-party URL here makes Storybook and visual-regression runs flaky/offline-hostile. Please switch this to a bundled asset or data URI so the stories stay deterministic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/data-display/zoom-pan-viewer.stories.tsx`
around lines 6 - 7, The SAMPLE_IMAGE constant currently points to an external
Unsplash URL which makes stories flaky; replace it with a bundled fixture or
inline data URI and update references to SAMPLE_IMAGE in this file. Add a local
image asset (e.g., put a small PNG/JPG in the same folder or a fixtures/assets
folder), import it at the top (e.g., import sampleImg from
'./fixtures/sample-image.png') or replace the string with a base64 data URI,
then assign SAMPLE_IMAGE = sampleImg so the ZoomPanViewer stories use the
local/bundled asset instead of the external URL.
Comment on lines
+83
to
+98
| describe('toolbar positions', () => { | ||
| it('renders overlay toolbar by default', () => { | ||
| const { container } = render(<ZoomPanViewer {...defaultProps} />); | ||
|
|
||
| const toolbarWrapper = container.querySelector('.absolute.top-4'); | ||
| expect(toolbarWrapper).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('renders inline toolbar when toolbarPosition is inline', () => { | ||
| const { container } = render( | ||
| <ZoomPanViewer {...defaultProps} toolbarPosition="inline" />, | ||
| ); | ||
|
|
||
| const inlineWrapper = container.querySelector('.justify-end.mb-2'); | ||
| expect(inlineWrapper).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using more stable test selectors.
The toolbar position tests rely on CSS class selectors like .absolute.top-4 and .justify-end.mb-2, which are implementation details that could break if styling changes. Consider adding data-testid attributes to the toolbar containers for more resilient tests.
💡 Example with test IDs
In the component, add:
<div data-testid="toolbar-overlay" className="absolute top-4 ...">Then in tests:
-const toolbarWrapper = container.querySelector('.absolute.top-4');
+const toolbarWrapper = screen.getByTestId('toolbar-overlay');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/data-display/zoom-pan-viewer.test.tsx`
around lines 83 - 98, Tests for toolbar positioning use fragile CSS class
selectors; update the ZoomPanViewer component to add stable data-testid
attributes (e.g., data-testid="toolbar-overlay" and
data-testid="toolbar-inline") on the respective toolbar container elements, then
update the test file zoom-pan-viewer.test.tsx to queryByTestId / getByTestId for
"toolbar-overlay" and "toolbar-inline" instead of using
container.querySelector('.absolute.top-4') and '.justify-end.mb-2'; ensure the
test assertions still call toBeInTheDocument() on the elements returned from
render.
Comment on lines
+82
to
+110
| <button | ||
| onClick={zoomOut} | ||
| disabled={!canZoomOut} | ||
| className={buttonClass} | ||
| aria-label={t('imagePreview.zoomOut')} | ||
| > | ||
| <ZoomOut className="size-4" /> | ||
| </button> | ||
| <Text | ||
| as="span" | ||
| align="center" | ||
| className="min-w-[3rem] text-sm tabular-nums" | ||
| > | ||
| {Math.round(zoom * 100)}% | ||
| </Text> | ||
| <button | ||
| onClick={zoomIn} | ||
| disabled={!canZoomIn} | ||
| className={buttonClass} | ||
| aria-label={t('imagePreview.zoomIn')} | ||
| > | ||
| <ZoomIn className="size-4" /> | ||
| </button> | ||
| <button | ||
| onClick={reset} | ||
| disabled={!isZoomed} | ||
| className={buttonClass} | ||
| aria-label={t('imagePreview.resetZoom')} | ||
| > |
There was a problem hiding this comment.
Add type="button" to the zoom controls.
These raw <button> elements default to submit, so embedding ZoomPanViewer inside a form will submit it when users click zoom/reset.
Proposed fix
<button
+ type="button"
onClick={zoomOut}
disabled={!canZoomOut}
className={buttonClass}
aria-label={t('imagePreview.zoomOut')}
>
@@
<button
+ type="button"
onClick={zoomIn}
disabled={!canZoomIn}
className={buttonClass}
aria-label={t('imagePreview.zoomIn')}
>
@@
<button
+ type="button"
onClick={reset}
disabled={!isZoomed}
className={buttonClass}
aria-label={t('imagePreview.resetZoom')}
>📝 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
| <button | |
| onClick={zoomOut} | |
| disabled={!canZoomOut} | |
| className={buttonClass} | |
| aria-label={t('imagePreview.zoomOut')} | |
| > | |
| <ZoomOut className="size-4" /> | |
| </button> | |
| <Text | |
| as="span" | |
| align="center" | |
| className="min-w-[3rem] text-sm tabular-nums" | |
| > | |
| {Math.round(zoom * 100)}% | |
| </Text> | |
| <button | |
| onClick={zoomIn} | |
| disabled={!canZoomIn} | |
| className={buttonClass} | |
| aria-label={t('imagePreview.zoomIn')} | |
| > | |
| <ZoomIn className="size-4" /> | |
| </button> | |
| <button | |
| onClick={reset} | |
| disabled={!isZoomed} | |
| className={buttonClass} | |
| aria-label={t('imagePreview.resetZoom')} | |
| > | |
| <button | |
| type="button" | |
| onClick={zoomOut} | |
| disabled={!canZoomOut} | |
| className={buttonClass} | |
| aria-label={t('imagePreview.zoomOut')} | |
| > | |
| <ZoomOut className="size-4" /> | |
| </button> | |
| <Text | |
| as="span" | |
| align="center" | |
| className="min-w-[3rem] text-sm tabular-nums" | |
| > | |
| {Math.round(zoom * 100)}% | |
| </Text> | |
| <button | |
| type="button" | |
| onClick={zoomIn} | |
| disabled={!canZoomIn} | |
| className={buttonClass} | |
| aria-label={t('imagePreview.zoomIn')} | |
| > | |
| <ZoomIn className="size-4" /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={reset} | |
| disabled={!isZoomed} | |
| className={buttonClass} | |
| aria-label={t('imagePreview.resetZoom')} | |
| > |
🧰 Tools
🪛 Biome (2.4.6)
[error] 82-87: Provide an explicit type prop for the button element.
(lint/a11y/useButtonType)
[error] 97-102: Provide an explicit type prop for the button element.
(lint/a11y/useButtonType)
[error] 105-110: Provide an explicit type prop for the button element.
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/components/ui/data-display/zoom-pan-viewer.tsx` around
lines 82 - 110, The three raw button elements in the ZoomPanViewer UI (the
buttons wired to the zoomOut, zoomIn, and reset handlers that use buttonClass
and aria-labels like t('imagePreview.zoomOut')/zoomIn/resetZoom') default to
type="submit" and can accidentally submit enclosing forms; update each of those
<button> elements to explicitly include type="button" to prevent form submission
while preserving their onClick, disabled, className and aria-label props.
Comment on lines
+22
to
+25
| /** Gallery images for prev/next navigation. When provided, src/alt are ignored in favor of images[activeIndex]. */ | ||
| images?: GalleryImage[]; | ||
| activeIndex?: number; | ||
| onActiveIndexChange?: (index: number) => void; |
There was a problem hiding this comment.
Don’t enter gallery mode unless navigation is actually wired.
Right now isGallery only checks images.length > 1, but both navigation callbacks return early when onActiveIndexChange is missing. That leaves visible prev/next controls and ArrowLeft/ArrowRight handlers that never change the image.
Proposed fix
- const isGallery = images && images.length > 1;
+ const isGallery = Boolean(images && images.length > 1 && onActiveIndexChange);Also applies to: 39-68, 102-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/chat/components/message-bubble/image-preview-dialog.tsx`
around lines 22 - 25, The gallery mode should only be enabled when navigation is
actually wired; update the logic that computes isGallery (used with images,
activeIndex, onActiveIndexChange) so it returns true only when images?.length >
1 AND onActiveIndexChange is provided (and activeIndex is not undefined), and
hide/disable Prev/Next controls and keyboard ArrowLeft/ArrowRight handlers when
onActiveIndexChange is absent; ensure functions like the prev/next click
handlers and the keyboard handler early-returning behavior is matched by the
isGallery guard so controls are never shown or active unless onActiveIndexChange
(and a valid activeIndex) exist.
Comment on lines
+40
to
+41
| const currentSrc = images ? (images[activeIndex]?.src ?? src) : src; | ||
| const currentAlt = images ? (images[activeIndex]?.alt ?? alt) : alt; |
There was a problem hiding this comment.
Clamp activeIndex before using it in the viewer and counter.
If the parent keeps a stale index after images changes, the dialog falls back to src/alt for rendering but still shows activeIndex + 1 in the counter, so you can end up with invalid states like “4 / 3”.
Proposed fix
- const currentSrc = images ? (images[activeIndex]?.src ?? src) : src;
- const currentAlt = images ? (images[activeIndex]?.alt ?? alt) : alt;
+ const normalizedIndex = images?.length
+ ? Math.min(Math.max(activeIndex, 0), images.length - 1)
+ : 0;
+ const currentSrc = images ? (images[normalizedIndex]?.src ?? src) : src;
+ const currentAlt = images ? (images[normalizedIndex]?.alt ?? alt) : alt;
@@
- current: activeIndex + 1,
+ current: normalizedIndex + 1,Also applies to: 91-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@services/platform/app/features/chat/components/message-bubble/image-preview-dialog.tsx`
around lines 40 - 41, The dialog uses activeIndex directly even when images
changes, causing invalid counters; clamp activeIndex to the valid range before
using it in currentSrc/currentAlt and the counter. In image-preview-dialog
(variables currentSrc/currentAlt and the counter render), compute a safeIndex =
Math.max(0, Math.min(activeIndex, (images?.length ?? 1) - 1)) (or equivalent)
and use safeIndex everywhere you currently use activeIndex so the viewer and
"activeIndex + 1 / images.length" stay consistent when images updates. Ensure
the same clamped index is used in the viewer logic and the block that renders
the counter.
Comment on lines
+94
to
+102
| const handlePointerMove = useCallback( | ||
| (e: React.PointerEvent) => { | ||
| if (!isDragging) return; | ||
| const dx = e.clientX - dragStart.current.x; | ||
| const dy = e.clientY - dragStart.current.y; | ||
| setPan({ x: panStart.current.x + dx, y: panStart.current.y + dy }); | ||
| }, | ||
| [isDragging], | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding pan bounds (optional enhancement).
Currently, users can pan the image completely off-screen when zoomed in. If this becomes a UX concern, consider clamping pan values based on the container and image dimensions. This could be a future enhancement if user feedback warrants it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/app/hooks/use-zoom-pan.ts` around lines 94 - 102,
handlePointerMove allows unrestricted panning so the image can be dragged
completely off-screen; clamp pan before calling setPan by computing visible
bounds from the container size and scaled image size and limiting pan.x/pan.y to
min/max values. In the handlePointerMove callback (referencing dragStart,
panStart, isDragging, setPan) calculate imageWidth = naturalWidth * scale and
imageHeight = naturalHeight * scale (or read computed sizes), compute
maxPanX/minPanX and maxPanY/minPanY based on container dimensions and image
dimensions, then clamp the computed newX/newY into those ranges and pass the
clamped values to setPan; keep existing isDragging guard and dependency list.
Comment on lines
+408
to
+411
| "title": "Image preview", | ||
| "zoomIn": "Zoom in", | ||
| "zoomOut": "Zoom out", | ||
| "resetZoom": "Reset zoom" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how zoom translation keys are accessed in components
rg -n "imagePreview\.(zoomIn|zoomOut|resetZoom)" services/platform/app/Repository: tale-project/tale
Length of output: 785
🏁 Script executed:
#!/bin/bash
# Check the ZoomPanViewer component's useT hook and namespace
sed -n '1,60p' services/platform/app/components/ui/data-display/zoom-pan-viewer.tsxRepository: tale-project/tale
Length of output: 1770
🏁 Script executed:
#!/bin/bash
# Also check if there are other dialogs.imagePreview references in the codebase
rg "dialogs\.imagePreview" services/platform/Repository: tale-project/tale
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Check how the test mocks the translations - what namespace is used
sed -n '1,30p' services/platform/app/components/ui/data-display/zoom-pan-viewer.test.tsxRepository: tale-project/tale
Length of output: 902
Move zoom control translations to the correct namespace.
The ZoomPanViewer component uses the common namespace and accesses keys like t('imagePreview.zoomIn'), but the new translations are placed under dialogs.imagePreview in en.json. These should be under common.imagePreview to match the component's usage.
Translation keys to relocate in en.json
Move lines 408-411 from the dialogs.imagePreview section to the common.imagePreview section:
"zoomIn": "Zoom in",
"zoomOut": "Zoom out",
"resetZoom": "Reset zoom"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@services/platform/messages/en.json` around lines 408 - 411, Move the three
zoom translation keys from the dialogs.imagePreview section into the
common.imagePreview namespace so ZoomPanViewer can find them via
t('imagePreview.zoomIn') etc.; specifically relocate "zoomIn", "zoomOut", and
"resetZoom" entries into the common.imagePreview object in en.json (remove them
from dialogs.imagePreview) so the component using common namespace resolves
correctly.
- Add type="button" to zoom control buttons in ZoomPanViewer - Only enable gallery mode when onActiveIndexChange is provided - Clamp activeIndex to valid range to prevent stale index issues - Move zoom translation keys to common.imagePreview namespace
yannickmonney
pushed a commit
that referenced
this pull request
Apr 8, 2026
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
ZoomPanViewercomponent with mouse wheel zoom, pinch-to-zoom, and drag panningsize-9with consistent ring borders across chat input and message bubblesimageViewerto sharedimagePreviewnamespaceTest plan
ZoomPanViewerunit tests and Storybook storiesSummary by CodeRabbit
New Features
UI/UX Improvements