Skip to content

Commit 052325c

Browse files
jaffrepauljaffrepaul
committed
add PR feedback
1 parent 896228e commit 052325c

File tree

3 files changed

+187
-95
lines changed

3 files changed

+187
-95
lines changed

‎src/components/imageLightbox/index.tsx

Lines changed: 41 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,12 @@
11
'use client';
22

33
import {useState} from 'react';
4-
import {X} from 'react-feather';
54
import * as Dialog from '@radix-ui/react-dialog';
65
import Image from 'next/image';
76

7+
import {Lightbox} from 'sentry-docs/components/lightbox';
88
import {isAllowedRemoteImage} from 'sentry-docs/config/images';
99

10-
import styles from './imageLightbox.module.scss';
11-
1210
interface ImageLightboxProps
1311
extends Omit<
1412
React.HTMLProps<HTMLImageElement>,
@@ -68,59 +66,58 @@ export function ImageLightbox({
6866
const shouldUseNextImage =
6967
!!dimensions && (!isExternalImage(src) || isAllowedRemoteImage(src));
7068

71-
const handleClick = (e: React.MouseEvent) => {
72-
// If Ctrl/Cmd+click, open image in new tab
69+
const handleModifierClick = (e: React.MouseEvent) => {
70+
// If Ctrl/Cmd+click, open image in new tab instead of lightbox
7371
if (e.ctrlKey || e.metaKey) {
72+
e.preventDefault();
73+
e.stopPropagation();
7474
const url = getImageUrl(src, imgPath);
7575
const newWindow = window.open(url, '_blank');
7676
if (newWindow) {
7777
newWindow.opener = null; // Security: prevent opener access
7878
}
79-
return;
8079
}
81-
// Normal click - open lightbox
82-
setOpen(true);
80+
// Normal click will be handled by Dialog.Trigger
8381
};
8482

85-
const handleKeyDown = (e: React.KeyboardEvent) => {
86-
// Handle Enter and Space keys
87-
if (e.key === 'Enter' || e.key === ' ') {
83+
const handleModifierKeyDown = (e: React.KeyboardEvent) => {
84+
// Handle Ctrl/Cmd+Enter or Ctrl/Cmd+Space to open in new tab
85+
if ((e.key === 'Enter' || e.key === ' ') && (e.ctrlKey || e.metaKey)) {
8886
e.preventDefault();
89-
// If Ctrl/Cmd+key, open image in new tab
90-
if (e.ctrlKey || e.metaKey) {
91-
const url = getImageUrl(src, imgPath);
92-
const newWindow = window.open(url, '_blank');
93-
if (newWindow) {
94-
newWindow.opener = null; // Security: prevent opener access
95-
}
96-
return;
87+
e.stopPropagation();
88+
const url = getImageUrl(src, imgPath);
89+
const newWindow = window.open(url, '_blank');
90+
if (newWindow) {
91+
newWindow.opener = null; // Security: prevent opener access
9792
}
98-
// Normal key press - open lightbox
99-
setOpen(true);
10093
}
94+
// Normal key presses will be handled by Dialog.Trigger
10195
};
10296

10397
// Filter out props that are incompatible with Next.js Image component
10498
// Next.js Image has stricter typing for certain props like 'placeholder'
10599
const {placeholder: _placeholder, ...imageCompatibleProps} = props;
106100

107101
// Render the appropriate image component
108-
const renderImage = () => {
102+
const renderImage = (isInline: boolean = true) => {
109103
const renderedSrc = getImageUrl(src, imgPath);
104+
const imageClassName = isInline
105+
? className
106+
: 'max-h-[90vh] max-w-[90vw] object-contain';
107+
const imageStyle = isInline
108+
? {width: '100%', height: 'auto', ...style}
109+
: {width: 'auto', height: 'auto'};
110+
110111
if (shouldUseNextImage && dimensions) {
111-
// TypeScript knows validDimensions.width and validDimensions.height are both numbers
112112
return (
113113
<Image
114114
src={renderedSrc}
115115
width={dimensions.width}
116116
height={dimensions.height}
117-
style={{
118-
width: '100%',
119-
height: 'auto',
120-
...style,
121-
}}
122-
className={className}
117+
style={imageStyle}
118+
className={imageClassName}
123119
alt={alt}
120+
priority={!isInline}
124121
{...imageCompatibleProps}
125122
/>
126123
);
@@ -130,77 +127,27 @@ export function ImageLightbox({
130127
<img
131128
src={renderedSrc}
132129
alt={alt}
133-
loading="lazy"
130+
loading={isInline ? 'lazy' : 'lazy'}
134131
decoding="async"
135-
style={{
136-
width: '100%',
137-
height: 'auto',
138-
...style,
139-
}}
140-
className={className}
132+
style={imageStyle}
133+
className={imageClassName}
141134
{...props}
142135
/>
143136
);
144137
};
145138

146139
return (
147-
<Dialog.Root open={open} onOpenChange={setOpen}>
148-
{/* Custom trigger that handles modifier keys properly */}
149-
<div
150-
role="button"
151-
tabIndex={0}
152-
className="cursor-pointer border-none bg-transparent p-0 block w-full no-underline"
153-
onClick={handleClick}
154-
onKeyDown={handleKeyDown}
155-
aria-label={`View image: ${alt}`}
156-
>
157-
{renderImage()}
158-
</div>
159-
160-
<Dialog.Portal>
161-
<Dialog.Overlay className={styles.lightboxOverlay} />
162-
163-
<Dialog.Content className={styles.lightboxContent}>
164-
{/* Close button */}
165-
<Dialog.Close className="absolute right-4 top-4 z-10 rounded-sm bg-[var(--flame0)] border border-white/20 p-2 text-white opacity-90 transition-all duration-200 hover:opacity-100 hover:bg-[var(--flame1)] hover:border-white/30 hover:scale-105 focus:outline-none focus:ring-2 focus:ring-white active:scale-95 flex items-center justify-center">
166-
<X className="h-4 w-4 stroke-[2.5]" />
167-
<span className="sr-only">Close</span>
168-
</Dialog.Close>
169-
170-
{/* Image container */}
171-
<div className="relative flex items-center justify-center">
172-
{shouldUseNextImage && dimensions ? (
173-
<Image
174-
src={getImageUrl(src, imgPath)}
175-
alt={alt}
176-
width={dimensions.width}
177-
height={dimensions.height}
178-
className="max-h-[90vh] max-w-[90vw] object-contain"
179-
style={{
180-
width: 'auto',
181-
height: 'auto',
182-
}}
183-
priority
184-
{...imageCompatibleProps}
185-
/>
186-
) : (
187-
/* eslint-disable-next-line @next/next/no-img-element */
188-
<img
189-
src={getImageUrl(src, imgPath)}
190-
alt={alt}
191-
loading="lazy"
192-
decoding="async"
193-
className="max-h-[90vh] max-w-[90vw] object-contain"
194-
style={{
195-
width: 'auto',
196-
height: 'auto',
197-
}}
198-
{...props}
199-
/>
200-
)}
201-
</div>
202-
</Dialog.Content>
203-
</Dialog.Portal>
204-
</Dialog.Root>
140+
<Lightbox.Root open={open} onOpenChange={setOpen} content={renderImage(false)}>
141+
<Dialog.Trigger asChild>
142+
<div
143+
onClick={handleModifierClick}
144+
onKeyDown={handleModifierKeyDown}
145+
className="cursor-pointer border-none bg-transparent p-0 block w-full no-underline"
146+
aria-label={`View image: ${alt}`}
147+
>
148+
{renderImage()}
149+
</div>
150+
</Dialog.Trigger>
151+
</Lightbox.Root>
205152
);
206153
}

‎src/components/lightbox/index.tsx

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
/**
2+
* Reusable Lightbox component built on top of Radix UI Dialog
3+
*
4+
* @example
5+
* // Simple usage with children as trigger
6+
* <Lightbox.Root content={<img src="large.jpg" />}>
7+
* <img src="thumbnail.jpg" alt="Click to expand" />
8+
* </Lightbox.Root>
9+
*
10+
* @example
11+
* // Advanced usage with custom trigger and controlled state
12+
* const [open, setOpen] = useState(false);
13+
*
14+
* <Lightbox.Root
15+
* open={open}
16+
* onOpenChange={setOpen}
17+
* content={<MyLargeContent />}
18+
* trigger={
19+
* <Lightbox.Trigger onClick={handleClick}>
20+
* <MyThumbnail />
21+
* </Lightbox.Trigger>
22+
* }
23+
* />
24+
*/
25+
26+
'use client';
27+
28+
import {useState} from 'react';
29+
import {X} from 'react-feather';
30+
import * as Dialog from '@radix-ui/react-dialog';
31+
32+
import styles from './lightbox.module.scss';
33+
34+
interface LightboxProps {
35+
children?: React.ReactNode;
36+
content: React.ReactNode;
37+
trigger?: React.ReactNode;
38+
onOpenChange?: (open: boolean) => void;
39+
open?: boolean;
40+
closeButton?: boolean;
41+
}
42+
43+
interface LightboxTriggerProps {
44+
children: React.ReactNode;
45+
onClick?: (e: React.MouseEvent) => void;
46+
onKeyDown?: (e: React.KeyboardEvent) => void;
47+
className?: string;
48+
'aria-label'?: string;
49+
}
50+
51+
interface LightboxContentProps {
52+
children: React.ReactNode;
53+
className?: string;
54+
}
55+
56+
interface LightboxCloseProps {
57+
children?: React.ReactNode;
58+
className?: string;
59+
}
60+
61+
// Root component
62+
function LightboxRoot({
63+
children,
64+
content,
65+
trigger,
66+
onOpenChange,
67+
open: controlledOpen,
68+
closeButton = true,
69+
}: LightboxProps) {
70+
const [internalOpen, setInternalOpen] = useState(false);
71+
const isControlled = controlledOpen !== undefined;
72+
const open = isControlled ? controlledOpen : internalOpen;
73+
const setOpen = isControlled ? (onOpenChange ?? (() => {})) : setInternalOpen;
74+
75+
return (
76+
<Dialog.Root open={open} onOpenChange={setOpen}>
77+
{trigger || (children && <Dialog.Trigger asChild>{children}</Dialog.Trigger>)}
78+
79+
<Dialog.Portal>
80+
<Dialog.Overlay className={styles.lightboxOverlay} />
81+
<Dialog.Content className={styles.lightboxContent}>
82+
{closeButton && (
83+
<Dialog.Close className="absolute right-4 top-4 z-10 rounded-sm bg-[var(--flame0)] border border-white/20 p-2 text-white opacity-90 transition-all duration-200 hover:opacity-100 hover:bg-[var(--flame1)] hover:border-white/30 hover:scale-105 focus:outline-none focus:ring-2 focus:ring-white active:scale-95 flex items-center justify-center">
84+
<X className="h-4 w-4 stroke-[2.5]" />
85+
<span className="sr-only">Close</span>
86+
</Dialog.Close>
87+
)}
88+
<div className="relative flex items-center justify-center">{content}</div>
89+
</Dialog.Content>
90+
</Dialog.Portal>
91+
</Dialog.Root>
92+
);
93+
}
94+
95+
// Trigger component for custom triggers
96+
function LightboxTrigger({
97+
children,
98+
onClick,
99+
onKeyDown,
100+
className = '',
101+
'aria-label': ariaLabel,
102+
}: LightboxTriggerProps) {
103+
return (
104+
<div
105+
role="button"
106+
tabIndex={0}
107+
className={`cursor-pointer border-none bg-transparent p-0 block w-full no-underline ${className}`}
108+
onClick={onClick}
109+
onKeyDown={onKeyDown}
110+
aria-label={ariaLabel}
111+
>
112+
{children}
113+
</div>
114+
);
115+
}
116+
117+
// Content wrapper (optional, for additional styling)
118+
function LightboxContent({children, className = ''}: LightboxContentProps) {
119+
return <div className={className}>{children}</div>;
120+
}
121+
122+
// Close button component
123+
function LightboxClose({children, className = ''}: LightboxCloseProps) {
124+
return (
125+
<Dialog.Close className={className}>
126+
{children || (
127+
<>
128+
<X className="h-4 w-4 stroke-[2.5]" />
129+
<span className="sr-only">Close</span>
130+
</>
131+
)}
132+
</Dialog.Close>
133+
);
134+
}
135+
136+
// Compound component exports
137+
export const Lightbox = {
138+
Root: LightboxRoot,
139+
Trigger: LightboxTrigger,
140+
Content: LightboxContent,
141+
Close: LightboxClose,
142+
};
143+
144+
// For backward compatibility, export the root as default
145+
export default LightboxRoot;

‎src/components/imageLightbox/imageLightbox.module.scss renamed to ‎src/components/lightbox/lightbox.module.scss

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,4 @@
7373

7474
.lightboxOverlay[data-state='closed'] {
7575
animation: dialogOverlayHide 200ms cubic-bezier(0.16, 1, 0.3, 1);
76-
}
76+
}

0 commit comments

Comments
 (0)