feat: add FeedbackImage component and integrate it into ImagePreviewButton and SheetDetailTable #2011
+106
−82
Conversation
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
…utton and SheetDetailTable
WalkthroughAdds a new FeedbackImage React component that conditionally fetches presigned image URLs based on channel configuration and router-derived IDs. Refactors image-preview-button and sheet-detail-table to use FeedbackImage. Updates shared UI index to export FeedbackImage. Introduces optional initialIndex prop to image-preview-button. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Component as FeedbackImage
participant Router
participant OAI as OAI Client
participant API as Admin API
User->>Component: Render with url
Component->>Router: Read query (projectId, channelId)
Router-->>Component: IDs (when ready)
rect rgba(230,240,255,0.5)
Component->>OAI: GET /api/admin/projects/{projectId}/channels/{channelId}
OAI->>API: Fetch channel data
API-->>OAI: channelData (imageConfig)
OAI-->>Component: channelData
end
Component->>Component: Compute imageKey from url
alt enablePresignedUrlDownload && imageKey
rect rgba(230,255,230,0.5)
Component->>OAI: GET /api/admin/projects/{projectId}/channels/{channelId}/image-download-url?imageKey=...
OAI->>API: Request presigned URL
API-->>OAI: presignedUrl
OAI-->>Component: presignedUrl
Component-->>User: Render Image src=presignedUrl
end
else
Component-->>User: Render Image src=url
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new FeedbackImage component to handle image rendering with presigned URL support and integrates it across image display components to replace inline image logic.
- Extracts image rendering logic with presigned URL handling into a reusable
FeedbackImagecomponent - Replaces direct
Imageusage and inlinePresignedURLImagecomponent inSheetDetailTableandImagePreviewButton - Exports the new component from the UI module index
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
apps/web/src/shared/ui/feedback-image.tsx |
New component that handles image rendering with presigned URL logic |
apps/web/src/shared/ui/image-preview-button.tsx |
Removes inline PresignedURLImage component and uses FeedbackImage instead |
apps/web/src/shared/ui/sheet-detail-table.ui.tsx |
Replaces direct Image usage with FeedbackImage component |
apps/web/src/shared/ui/index.ts |
Exports the new FeedbackImage component |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
jihun
approved these changes
Aug 29, 2025
![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 (4)
apps/web/src/shared/ui/feedback-image.tsx (1)
66-74: Broaden S3 host detection (support accelerate/region/virtual-host styles).Use a more robust check:
- const host = parsed.hostname; - const parts = key.split('/', 2); - if ( - (host === 's3.amazonaws.com' || host.startsWith('s3.')) && - parts.length >= 2 - ) { + const host = parsed.hostname; + const parts = key.split('/', 2); + const isS3StyleHost = + host === 's3.amazonaws.com' || + host.endsWith('.s3.amazonaws.com') || + /^s3[.-][a-z0-9-]+\.amazonaws\.com$/i.test(host) || + host === 's3-accelerate.amazonaws.com'; + if (isS3StyleHost && parts.length >= 2) { return parts.slice(1).join('/'); }apps/web/src/shared/ui/sheet-detail-table.ui.tsx (2)
198-205: Pass a concrete sizes hint to reduce image payloads for 64px tiles.- <FeedbackImage url={v} /> + <FeedbackImage url={v} sizes="64px" alt="" />
360-367: Same here for edit mode thumbnails.- <FeedbackImage url={v} /> + <FeedbackImage url={v} sizes="64px" alt="" />apps/web/src/shared/ui/image-preview-button.tsx (1)
90-96: Provide tight sizes for thumbnails.- {urls.map((url) => ( + {urls.map((url, idx) => ( - <SwiperSlide - key={url} + <SwiperSlide + key={`${url}-thumb-${idx}`} className="rounded-8 bg-neutral-secondary relative overflow-hidden" > - <FeedbackImage url={url} /> + <FeedbackImage url={url} sizes="64px" alt="" /> </SwiperSlide> ))}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
apps/web/src/shared/ui/feedback-image.tsx(1 hunks)apps/web/src/shared/ui/image-preview-button.tsx(4 hunks)apps/web/src/shared/ui/index.ts(1 hunks)apps/web/src/shared/ui/sheet-detail-table.ui.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: integration-test
🔇 Additional comments (4)
apps/web/src/shared/ui/index.ts (1)
54-55: LGTM — exporting FeedbackImage from shared UI.apps/web/src/shared/ui/sheet-detail-table.ui.tsx (1)
41-41: Import looks good.apps/web/src/shared/ui/image-preview-button.tsx (2)
17-17: OK to add state/effects for thumbs + initial index.
34-35: Good: centralizing image logic via FeedbackImage.
Comment on lines
+22
to
+26
| interface Props { | ||
| url: string; | ||
| } | ||
|
|
||
| const FeedbackImage = ({ url }: Props) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Expose alt/sizes/className to fix a11y and prevent over-fetching with next/image fill.
Add optional props and forward them. Consumers (thumbnails/modals) can pass correct sizes.
-interface Props {
- url: string;
-}
+interface Props {
+ url: string;
+ alt?: string;
+ sizes?: string;
+ className?: string;
+}
-const FeedbackImage = ({ url }: Props) => {
+const FeedbackImage = ({ url, alt, sizes, className }: Props) => {
@@
return (
<Image
- src={presignedUrl ?? url}
- alt={presignedUrl ?? url}
- className="cursor-pointer object-cover"
- fill
+ src={presignedUrl ?? url}
+ alt={alt ?? ''}
+ className={`cursor-pointer object-cover ${className ?? ''}`}
+ fill
+ sizes={sizes}
/>
);Also applies to: 88-93
🤖 Prompt for AI Agents
In apps/web/src/shared/ui/feedback-image.tsx around lines 22-26 (and also apply
same changes at 88-93), the component only accepts url which prevents setting
alt for accessibility and sizes/className for next/image fill behavior; update
the Props type to include optional alt?: string, sizes?: string, and className?:
string, update the component signature to accept and forward those props
(destructure { url, alt, sizes, className }: Props), and pass alt, sizes, and
className through to the underlying Image element so consumers
(thumbnails/modals) can supply appropriate values to fix a11y and avoid
over-fetching. Ensure Typescript types reflect optional props and no other logic
changes are required.
Comment on lines
+42
to
+75
| const imageKey = useMemo(() => { | ||
| if (!channelData?.imageConfig?.enablePresignedUrlDownload) return url; | ||
|
|
||
| let parsed: URL | null = null; | ||
| try { | ||
| parsed = new URL( | ||
| url, | ||
| typeof window !== 'undefined' ? | ||
| window.location.href | ||
| : 'http://localhost', | ||
| ); | ||
| } catch { | ||
| return url; | ||
| } | ||
| if (!/^https?:$/.test(parsed.protocol)) return url; | ||
|
|
||
| const rawPath = parsed.pathname.replace(/^\/+/, ''); | ||
| let key = rawPath; | ||
| try { | ||
| key = decodeURIComponent(rawPath); | ||
| } catch { | ||
| /* keep raw */ | ||
| } | ||
|
|
||
| const host = parsed.hostname; | ||
| const parts = key.split('/', 2); | ||
| if ( | ||
| (host === 's3.amazonaws.com' || host.startsWith('s3.')) && | ||
| parts.length >= 2 | ||
| ) { | ||
| return parts.slice(1).join('/'); | ||
| } | ||
| return key; | ||
| }, [channelData, url]); |
There was a problem hiding this comment.
Don’t hit the presign endpoint when the URL is not a parsable HTTP(S) object.
When parsing fails or protocol isn’t http/https, imageKey currently falls back to the original url, which still enables the presign query and will send an invalid key. Return a falsy value and hard‑gate the query instead.
Apply:
- const imageKey = useMemo(() => {
- if (!channelData?.imageConfig?.enablePresignedUrlDownload) return url;
+ const imageKey = useMemo(() => {
+ if (!channelData?.imageConfig?.enablePresignedUrlDownload) return undefined;
@@
- } catch {
- return url;
+ } catch {
+ return undefined;
}
- if (!/^https?:$/.test(parsed.protocol)) return url;
+ if (!/^https?:$/.test(parsed.protocol)) return undefined;
@@
- const { data: presignedUrl } = useOAIQuery({
+ const { data: presignedUrl } = useOAIQuery({
path: '/api/admin/projects/{projectId}/channels/{channelId}/image-download-url',
variables: { channelId, projectId, imageKey },
queryOptions: {
- enabled: Boolean(
- imageKey && channelData?.imageConfig?.enablePresignedUrlDownload,
- ),
+ enabled: Boolean(
+ imageKey &&
+ channelData?.imageConfig?.enablePresignedUrlDownload &&
+ router.isReady &&
+ Number.isFinite(projectId) &&
+ Number.isFinite(channelId)
+ ),
},
});Also applies to: 77-85
🤖 Prompt for AI Agents
In apps/web/src/shared/ui/feedback-image.tsx around lines 42 to 75 (and also
apply the same change to lines 77-85), imageKey currently returns the original
url when parsing fails or protocol isn't http/https which still triggers the
presign query with an invalid key; change the logic so that when URL parsing
fails or the protocol check fails you return a falsy value (e.g. null) instead
of the original url, and ensure callers/queries hard‑gate on a truthy imageKey
before calling the presign endpoint so presign is only invoked for valid parsed
HTTP(S) keys.
Comment on lines
75
to
78
| {urls.map((url) => ( | ||
| <SwiperSlide key={url} className="relative"> | ||
| <PresignedURLImage url={url} /> | ||
| <FeedbackImage url={url} /> | ||
| </SwiperSlide> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Provide sizes for the main slide to avoid 100vw default and oversized requests. Also mind duplicate keys if urls aren’t unique.
- {urls.map((url) => (
+ {urls.map((url, idx) => (
- <SwiperSlide key={url} className="relative">
- <FeedbackImage url={url} />
+ <SwiperSlide key={`${url}-${idx}`} className="relative">
+ <FeedbackImage url={url} sizes="100vw" alt="" />
</SwiperSlide>
))}If urls are guaranteed unique, keeping key={url} is fine. Otherwise prefer the composite key above. Please confirm.
🏁 Script executed:
#!/usr/bin/env bash
# Locate the ImagePreviewButton file
fd image-preview-button.tsx -t f
# Show the top of the component to inspect props and imports
sed -n '1,200p' apps/web/src/shared/ui/image-preview-button.tsx
# Find where `urls` is declared or referenced
rg -n 'urls' -C3 apps/web/src/shared/ui/image-preview-button.tsx
# Find the import of FeedbackImage
rg -n 'FeedbackImage' -C3 apps/web/src/shared/ui/image-preview-button.tsx
# Locate the FeedbackImage component definition
fd FeedbackImage.tsx -t f
# Display its first 200 lines to inspect accepted props
sed -n '1,200p' "$(fd FeedbackImage.tsx -t f | head -n1)"Length of output: 5667
🏁 Script executed:
#!/usr/bin/env bash
# List files in the shared UI directory
ls apps/web/src/shared/ui
# Identify the exact filename for FeedbackImage
ls apps/web/src/shared/ui | grep -i feedback-image
# Display the component definition to verify accepted props
sed -n '1,200p' apps/web/src/shared/ui/feedback-image.tsxLength of output: 3615
Support responsive image sizing and stable keys
In FeedbackImage (apps/web/src/shared/ui/feedback-image.tsx), extend its Props to accept optional sizes?: string and alt?: string and forward them to Next/Image:
interface Props {
url: string;
alt?: string;
sizes?: string;
}
// …
<Image
src={presignedUrl ?? url}
alt={alt ?? presignedUrl ?? url}
sizes={sizes}
className="cursor-pointer object-cover"
fill
/>Then in ImagePreviewButton (apps/web/src/shared/ui/image-preview-button.tsx) at lines 75–78, update the main slide mapping:
-{urls.map((url) => (
- <SwiperSlide key={url} className="relative">
- <FeedbackImage url={url} />
+{urls.map((url, idx) => (
+ <SwiperSlide key={`${url}-${idx}`} className="relative">
+ <FeedbackImage url={url} sizes="100vw" alt="" />If urls are guaranteed unique, you can keep key={url}; otherwise use the composite key above.
🤖 Prompt for AI Agents
In apps/web/src/shared/ui/image-preview-button.tsx around lines 75 to 78, the
SwiperSlide mapping needs to pass optional alt and sizes through to
FeedbackImage and ensure stable keys; update the map to include alt and sizes
props (e.g., <FeedbackImage url={url} alt={optionalAlt} sizes={optionalSizes}
/>) and, if urls may not be unique, use a composite stable key (for example
`${url}-${index}`) instead of key={url}; also ensure FeedbackImage's Props are
extended to accept sizes?: string and alt?: string and those are forwarded to
the Next/Image component.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 by CodeRabbit