Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions apps/docs/components/ui/action-media.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
'use client'

import { getAssetUrl } from '@/lib/utils'

interface ActionImageProps {
src: string
alt: string
}

interface ActionVideoProps {
src: string
alt: string
}

export function ActionImage({ src, alt }: ActionImageProps) {
const resolvedSrc = getAssetUrl(src.startsWith('/') ? src.slice(1) : src)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getAssetUrl function already handles paths without leading slashes (apps/docs/lib/utils.ts:16-22), so the src.startsWith('/') check and slice(1) logic may be unnecessary duplication.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/docs/components/ui/action-media.tsx
Line: 16:16

Comment:
The `getAssetUrl` function already handles paths without leading slashes (apps/docs/lib/utils.ts:16-22), so the `src.startsWith('/')` check and `slice(1)` logic may be unnecessary duplication.

How can I resolve this? If you propose a fix, please make it concise.


return (
<img
src={resolvedSrc}
alt={alt}
className='inline-block w-full max-w-[200px] rounded border border-neutral-200 dark:border-neutral-700'
/>
)
Comment on lines +19 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing width and height attributes which are important for layout stability and avoiding CLS (Cumulative Layout Shift). The existing Image component (apps/docs/components/ui/image.tsx:13-53) uses Next.js Image component which handles dimensions properly.

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/docs/components/ui/action-media.tsx
Line: 19:24

Comment:
Missing `width` and `height` attributes which are important for layout stability and avoiding CLS (Cumulative Layout Shift). The existing `Image` component (apps/docs/components/ui/image.tsx:13-53) uses Next.js Image component which handles dimensions properly.

How can I resolve this? If you propose a fix, please make it concise.

}
Comment on lines +15 to +25
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider reusing the existing Image component (apps/docs/components/ui/image.tsx:13-53) instead of creating a separate implementation. The existing component uses Next.js Image for better performance and includes lightbox functionality.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/docs/components/ui/action-media.tsx
Line: 15:25

Comment:
Consider reusing the existing `Image` component (apps/docs/components/ui/image.tsx:13-53) instead of creating a separate implementation. The existing component uses Next.js Image for better performance and includes lightbox functionality.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.


export function ActionVideo({ src, alt }: ActionVideoProps) {
const resolvedSrc = getAssetUrl(src.startsWith('/') ? src.slice(1) : src)

return (
<video
src={resolvedSrc}
autoPlay
loop
muted
playsInline
className='inline-block w-full max-w-[200px] rounded border border-neutral-200 dark:border-neutral-700'
/>
Comment on lines +31 to +38
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing aria-label attribute for accessibility. The existing Video component pattern (apps/docs/components/ui/video.tsx:36-44) doesn't include this either, but it's a best practice for videos to have descriptive ARIA labels.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/docs/components/ui/action-media.tsx
Line: 31:38

Comment:
Missing `aria-label` attribute for accessibility. The existing `Video` component pattern (apps/docs/components/ui/video.tsx:36-44) doesn't include this either, but it's a best practice for videos to have descriptive ARIA labels.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused alt prop in ActionVideo component

Low Severity

The ActionVideo component accepts an alt prop but never uses it. The <video> HTML element doesn't support the alt attribute like <img> does. All callers in the documentation pass alt values expecting them to provide accessibility information, but the values are silently discarded. For video accessibility, aria-label={alt} could be added to the video element.

Fix in Cursor Fix in Web

}
Loading