Skip to content

Commit 0770228

Browse files
committed
better abstraction refactor & cleanup
1 parent 052325c commit 0770228

File tree

5 files changed

+51
-94
lines changed

5 files changed

+51
-94
lines changed

src/components/docImage.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import path from 'path';
22

3+
import {isExternalImage} from 'sentry-docs/config/images';
34
import {serverContext} from 'sentry-docs/serverContext';
45

56
import {ImageLightbox} from './imageLightbox';
@@ -64,12 +65,12 @@ export default function DocImage({
6465
return null;
6566
}
6667

67-
const isExternalImage = src.startsWith('http') || src.startsWith('//');
68+
const isExternal = isExternalImage(src);
6869
let finalSrc = src;
6970
let imgPath = src;
7071

7172
// For internal images, process the path
72-
if (!isExternalImage) {
73+
if (!isExternal) {
7374
if (src.startsWith('./')) {
7475
finalSrc = path.join('/mdx-images', src);
7576
} else if (!src?.startsWith('/') && !src?.includes('://')) {

src/components/imageLightbox/index.tsx

Lines changed: 23 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,10 @@
11
'use client';
22

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

76
import {Lightbox} from 'sentry-docs/components/lightbox';
8-
import {isAllowedRemoteImage} from 'sentry-docs/config/images';
7+
import {isAllowedRemoteImage, isExternalImage} from 'sentry-docs/config/images';
98

109
interface ImageLightboxProps
1110
extends Omit<
@@ -19,15 +18,9 @@ interface ImageLightboxProps
1918
width?: number;
2019
}
2120

22-
// Helper functions
23-
const isExternalImage = (src: string): boolean =>
24-
src.startsWith('http') || src.startsWith('//');
25-
2621
const getImageUrl = (src: string, imgPath: string): string =>
2722
isExternalImage(src) ? src : imgPath;
2823

29-
// Using shared allowlist logic from src/config/images
30-
3124
type ValidDimensions = {
3225
height: number;
3326
width: number;
@@ -59,46 +52,36 @@ export function ImageLightbox({
5952
}: ImageLightboxProps) {
6053
const [open, setOpen] = useState(false);
6154

62-
// Check if we should use Next.js Image or regular img
63-
// Use Next.js Image for internal images with valid dimensions
64-
// Use regular img for external images or when dimensions are invalid/missing
6555
const dimensions = getValidDimensions(width, height);
6656
const shouldUseNextImage =
6757
!!dimensions && (!isExternalImage(src) || isAllowedRemoteImage(src));
6858

69-
const handleModifierClick = (e: React.MouseEvent) => {
70-
// If Ctrl/Cmd+click, open image in new tab instead of lightbox
71-
if (e.ctrlKey || e.metaKey) {
59+
const openInNewTab = () => {
60+
window.open(getImageUrl(src, imgPath), '_blank');
61+
};
62+
63+
const handleClick = (e: React.MouseEvent) => {
64+
// Middle-click or Ctrl/Cmd+click opens in new tab
65+
if (e.button === 1 || e.ctrlKey || e.metaKey) {
7266
e.preventDefault();
73-
e.stopPropagation();
74-
const url = getImageUrl(src, imgPath);
75-
const newWindow = window.open(url, '_blank');
76-
if (newWindow) {
77-
newWindow.opener = null; // Security: prevent opener access
78-
}
67+
openInNewTab();
68+
return;
7969
}
80-
// Normal click will be handled by Dialog.Trigger
70+
// Regular click falls through to Dialog.Trigger
8171
};
8272

83-
const handleModifierKeyDown = (e: React.KeyboardEvent) => {
84-
// Handle Ctrl/Cmd+Enter or Ctrl/Cmd+Space to open in new tab
73+
const handleKeyDown = (e: React.KeyboardEvent) => {
8574
if ((e.key === 'Enter' || e.key === ' ') && (e.ctrlKey || e.metaKey)) {
8675
e.preventDefault();
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
92-
}
76+
openInNewTab();
9377
}
94-
// Normal key presses will be handled by Dialog.Trigger
78+
// Regular Enter/Space falls through to Dialog.Trigger
9579
};
9680

9781
// Filter out props that are incompatible with Next.js Image component
9882
// Next.js Image has stricter typing for certain props like 'placeholder'
9983
const {placeholder: _placeholder, ...imageCompatibleProps} = props;
10084

101-
// Render the appropriate image component
10285
const renderImage = (isInline: boolean = true) => {
10386
const renderedSrc = getImageUrl(src, imgPath);
10487
const imageClassName = isInline
@@ -127,27 +110,30 @@ export function ImageLightbox({
127110
<img
128111
src={renderedSrc}
129112
alt={alt}
130-
loading={isInline ? 'lazy' : 'lazy'}
113+
loading={isInline ? 'lazy' : 'eager'}
131114
decoding="async"
132115
style={imageStyle}
133116
className={imageClassName}
134-
{...props}
117+
{...imageCompatibleProps}
135118
/>
136119
);
137120
};
138121

139122
return (
140123
<Lightbox.Root open={open} onOpenChange={setOpen} content={renderImage(false)}>
141-
<Dialog.Trigger asChild>
124+
<Lightbox.Trigger asChild>
142125
<div
143-
onClick={handleModifierClick}
144-
onKeyDown={handleModifierKeyDown}
126+
onClick={handleClick}
127+
onAuxClick={handleClick}
128+
onKeyDown={handleKeyDown}
145129
className="cursor-pointer border-none bg-transparent p-0 block w-full no-underline"
146130
aria-label={`View image: ${alt}`}
131+
role="button"
132+
tabIndex={0}
147133
>
148134
{renderImage()}
149135
</div>
150-
</Dialog.Trigger>
136+
</Lightbox.Trigger>
151137
</Lightbox.Root>
152138
);
153139
}

src/components/lightbox/index.tsx

Lines changed: 17 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -8,49 +8,38 @@
88
* </Lightbox.Root>
99
*
1010
* @example
11-
* // Advanced usage with custom trigger and controlled state
11+
* // Advanced usage with Lightbox.Trigger and controlled state
1212
* const [open, setOpen] = useState(false);
1313
*
14-
* <Lightbox.Root
15-
* open={open}
16-
* onOpenChange={setOpen}
17-
* content={<MyLargeContent />}
18-
* trigger={
19-
* <Lightbox.Trigger onClick={handleClick}>
14+
* <Lightbox.Root open={open} onOpenChange={setOpen} content={<MyLargeContent />}>
15+
* <Lightbox.Trigger asChild>
16+
* <button onClick={handleClick}>
2017
* <MyThumbnail />
21-
* </Lightbox.Trigger>
22-
* }
23-
* />
18+
* </button>
19+
* </Lightbox.Trigger>
20+
* </Lightbox.Root>
2421
*/
2522

2623
'use client';
2724

28-
import {useState} from 'react';
25+
import {Fragment, useState} from 'react';
2926
import {X} from 'react-feather';
3027
import * as Dialog from '@radix-ui/react-dialog';
3128

3229
import styles from './lightbox.module.scss';
3330

3431
interface LightboxProps {
35-
children?: React.ReactNode;
3632
content: React.ReactNode;
37-
trigger?: React.ReactNode;
33+
children?: React.ReactNode;
34+
closeButton?: boolean;
3835
onOpenChange?: (open: boolean) => void;
3936
open?: boolean;
40-
closeButton?: boolean;
37+
trigger?: React.ReactNode;
4138
}
4239

4340
interface LightboxTriggerProps {
4441
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;
42+
asChild?: boolean;
5443
}
5544

5645
interface LightboxCloseProps {
@@ -92,54 +81,29 @@ function LightboxRoot({
9281
);
9382
}
9483

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>;
84+
// Trigger component
85+
function LightboxTrigger({children, asChild = false}: LightboxTriggerProps) {
86+
return <Dialog.Trigger asChild={asChild}>{children}</Dialog.Trigger>;
12087
}
12188

12289
// Close button component
12390
function LightboxClose({children, className = ''}: LightboxCloseProps) {
12491
return (
12592
<Dialog.Close className={className}>
12693
{children || (
127-
<>
94+
<Fragment>
12895
<X className="h-4 w-4 stroke-[2.5]" />
12996
<span className="sr-only">Close</span>
130-
</>
97+
</Fragment>
13198
)}
13299
</Dialog.Close>
133100
);
134101
}
135102

136-
// Compound component exports
137103
export const Lightbox = {
138104
Root: LightboxRoot,
139105
Trigger: LightboxTrigger,
140-
Content: LightboxContent,
141106
Close: LightboxClose,
142107
};
143108

144-
// For backward compatibility, export the root as default
145109
export default LightboxRoot;

src/config/images.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,15 @@ export const REMOTE_IMAGE_PATTERNS = REMOTE_IMAGE_HOSTNAMES.map(hostname => ({
88
hostname,
99
}));
1010

11+
export function isExternalImage(src: string): boolean {
12+
return src.startsWith('http') || src.startsWith('//');
13+
}
14+
1115
export function isAllowedRemoteImage(src: string): boolean {
1216
try {
13-
const url = new URL(src);
17+
// Handle protocol-relative URLs by adding https: protocol
18+
const normalizedSrc = src.startsWith('//') ? `https:${src}` : src;
19+
const url = new URL(normalizedSrc);
1420
return (
1521
url.protocol === 'https:' &&
1622
(REMOTE_IMAGE_HOSTNAMES as readonly string[]).includes(url.hostname)

src/remark-image-size.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ export default function remarkImageSize(options) {
1313
return tree =>
1414
visit(tree, 'image', node => {
1515
// don't process external images
16-
if (node.url.startsWith('http')) {
16+
if (node.url.startsWith('http') || node.url.startsWith('//')) {
1717
return;
1818
}
1919
const fullImagePath = path.join(

0 commit comments

Comments
 (0)