diff --git a/src/components/docImage/docImage.spec.tsx b/src/components/docImage/docImage.spec.tsx new file mode 100644 index 00000000000000..63d961d1e2defe --- /dev/null +++ b/src/components/docImage/docImage.spec.tsx @@ -0,0 +1,171 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest'; + +import { + cleanUrl, + DIMENSION_PATTERN, + extractHash, + parseDimensionsFromHash, +} from 'sentry-docs/components/docImage'; +import {serverContext} from 'sentry-docs/serverContext'; + +// Mock the serverContext +vi.mock('sentry-docs/serverContext', () => ({ + serverContext: vi.fn(), +})); + +// Mock the ImageLightbox component +vi.mock('../imageLightbox', () => ({ + ImageLightbox: vi.fn(({src, alt, width, height, imgPath}) => ({ + type: 'ImageLightbox', + props: {src, alt, width, height, imgPath}, + })), +})); + +const mockServerContext = serverContext as any; + +describe('DocImage Helper Functions', () => { + beforeEach(() => { + mockServerContext.mockReturnValue({ + path: '/docs/test-page', + }); + }); + + describe('dimension validation bounds', () => { + it('should validate dimensions within acceptable range', () => { + // Test actual boundary conditions used in parseDimensionsFromHash + expect(parseDimensionsFromHash('/image.jpg#1x1')).toEqual([1, 1]); // minimum valid + expect(parseDimensionsFromHash('/image.jpg#10000x10000')).toEqual([10000, 10000]); // maximum valid + expect(parseDimensionsFromHash('/image.jpg#0x600')).toEqual([]); // below minimum + expect(parseDimensionsFromHash('/image.jpg#10001x5000')).toEqual([]); // above maximum + }); + }); + + describe('dimension pattern regex', () => { + it('should match valid dimension patterns', () => { + expect(DIMENSION_PATTERN.test('800x600')).toBe(true); + expect(DIMENSION_PATTERN.test('1920x1080')).toBe(true); + expect(DIMENSION_PATTERN.test('10000x10000')).toBe(true); + }); + + it('should not match invalid dimension patterns', () => { + expect(DIMENSION_PATTERN.test('800x')).toBe(false); + expect(DIMENSION_PATTERN.test('x600')).toBe(false); + expect(DIMENSION_PATTERN.test('800')).toBe(false); + expect(DIMENSION_PATTERN.test('800x600x400')).toBe(false); + expect(DIMENSION_PATTERN.test('abc800x600')).toBe(false); + expect(DIMENSION_PATTERN.test('section-heading')).toBe(false); + }); + }); + + describe('hash extraction', () => { + it('should extract hash from URLs', () => { + expect(extractHash('/image.jpg#800x600')).toBe('800x600'); + expect(extractHash('./img/issue_page.png#1200x800')).toBe('1200x800'); + expect(extractHash('https://example.com/image.jpg#section')).toBe('section'); + expect(extractHash('/image.jpg')).toBe(''); + }); + }); + + describe('dimension parsing from hash', () => { + it('should parse valid dimensions from URL hash', () => { + expect(parseDimensionsFromHash('/image.jpg#800x600')).toEqual([800, 600]); + expect(parseDimensionsFromHash('/image.jpg#1920x1080')).toEqual([1920, 1080]); + expect(parseDimensionsFromHash('/image.jpg#10000x10000')).toEqual([10000, 10000]); + expect(parseDimensionsFromHash('./img/issue_page.png#1200x800')).toEqual([ + 1200, 800, + ]); + }); + + it('should return empty array for invalid dimensions', () => { + const invalidCases = [ + '/image.jpg#0x600', + '/image.jpg#10001x5000', + '/image.jpg#800x', + '/image.jpg#section-heading', + '/image.jpg', + './img/error_level.png#malformed', + ]; + + invalidCases.forEach(url => { + expect(parseDimensionsFromHash(url)).toEqual([]); + }); + }); + }); + + describe('URL cleaning', () => { + it('should remove dimension hashes from URLs', () => { + const testCases = [ + {input: '/image.jpg#800x600', expected: '/image.jpg'}, + { + input: 'https://example.com/image.jpg#1920x1080', + expected: 'https://example.com/image.jpg', + }, + {input: './img/issue_page.png#1200x800', expected: './img/issue_page.png'}, + {input: './img/error_level.png#32x32', expected: './img/error_level.png'}, + ]; + + testCases.forEach(({input, expected}) => { + expect(cleanUrl(input)).toBe(expected); + }); + }); + + it('should preserve non-dimension hashes', () => { + const testCases = [ + './img/issue_page.png#section-heading', + '/image.jpg#important-section', + '/image.jpg#anchor', + ]; + + testCases.forEach(url => { + expect(cleanUrl(url)).toBe(url); + }); + }); + + it('should handle URLs without hashes', () => { + const testCases = [ + '/image.jpg', + 'https://example.com/image.jpg', + './img/issue_page.png', + ]; + + testCases.forEach(url => { + expect(cleanUrl(url)).toBe(url); + }); + }); + }); + + describe('Issues page integration scenarios', () => { + const issuesPageImages = [ + './img/issue_page.png#1200x800', + './img/error_level.png#32x32', + './img/issue_sort.png#600x400', + ]; + + it('should handle Issues page image paths correctly', () => { + issuesPageImages.forEach(path => { + const hash = extractHash(path); + const cleanedUrl = cleanUrl(path); + const dimensions = parseDimensionsFromHash(path); + + expect(hash).toMatch(DIMENSION_PATTERN); + expect(cleanedUrl).not.toContain('#'); + expect(dimensions).toHaveLength(2); + expect(dimensions[0]).toBeGreaterThan(0); + expect(dimensions[1]).toBeGreaterThan(0); + }); + }); + + it('should handle malformed relative paths gracefully', () => { + const malformedPaths = [ + './img/issue_page.png#800x', + './img/error_level.png#invalid', + './img/issue_sort.png#section-anchor', + ]; + + malformedPaths.forEach(path => { + expect(cleanUrl(path)).toBe(path); // Should not clean non-dimension hashes + expect(parseDimensionsFromHash(path)).toEqual([]); // Should return empty array + }); + }); + }); +}); diff --git a/src/components/docImage.tsx b/src/components/docImage/index.tsx similarity index 79% rename from src/components/docImage.tsx rename to src/components/docImage/index.tsx index dd207908314117..c212dc4f2e5091 100644 --- a/src/components/docImage.tsx +++ b/src/components/docImage/index.tsx @@ -3,41 +3,42 @@ import path from 'path'; import {isExternalImage} from 'sentry-docs/config/images'; import {serverContext} from 'sentry-docs/serverContext'; -import {ImageLightbox} from './imageLightbox'; +import {ImageLightbox} from '../imageLightbox'; // Helper function to safely parse dimension values const parseDimension = (value: string | number | undefined): number | undefined => { - if (typeof value === 'number' && value > 0 && value <= 10000) return value; + if (typeof value === 'number' && value > 0 && value <= MAX_DIMENSION) return value; if (typeof value === 'string') { const parsed = parseInt(value, 10); - return parsed > 0 && parsed <= 10000 ? parsed : undefined; + return parsed > 0 && parsed <= MAX_DIMENSION ? parsed : undefined; } return undefined; }; // Dimension pattern regex - used to identify dimension hashes like "800x600" -const DIMENSION_PATTERN = /^(\d+)x(\d+)$/; +export const DIMENSION_PATTERN = /^(\d+)x(\d+)$/; +export const MAX_DIMENSION = 10000; // Helper function to extract hash from URL string (works with both relative and absolute URLs) -const extractHash = (url: string): string => { +export const extractHash = (url: string): string => { const hashIndex = url.indexOf('#'); return hashIndex !== -1 ? url.slice(hashIndex + 1) : ''; }; // Helper function to check if a hash contains dimension information -const isDimensionHash = (hash: string): boolean => { +export const isDimensionHash = (hash: string): boolean => { return DIMENSION_PATTERN.test(hash); }; // Helper function to parse dimensions from URL hash -const parseDimensionsFromHash = (url: string): number[] => { +export const parseDimensionsFromHash = (url: string): number[] => { const hash = extractHash(url); const match = hash.match(DIMENSION_PATTERN); if (match) { const width = parseInt(match[1], 10); const height = parseInt(match[2], 10); - return width > 0 && width <= 10000 && height > 0 && height <= 10000 + return width > 0 && width <= MAX_DIMENSION && height > 0 && height <= MAX_DIMENSION ? [width, height] : []; } @@ -46,7 +47,7 @@ const parseDimensionsFromHash = (url: string): number[] => { }; // Helper function to remove dimension hash from URL while preserving fragment identifiers -const cleanUrl = (url: string): string => { +export const cleanUrl = (url: string): string => { const hash = extractHash(url); // If no hash or hash is not a dimension pattern, return original URL @@ -78,7 +79,9 @@ export default function DocImage({ // For internal images, process the path if (!isExternal) { if (src.startsWith('./')) { - finalSrc = path.join('/mdx-images', src); + // Remove ./ prefix and properly join with mdx-images path + const cleanSrc = src.slice(2); + finalSrc = path.join('/mdx-images', cleanSrc); } else if (!src?.startsWith('/') && !src?.includes('://')) { finalSrc = `/${pagePath.join('/')}/${src}`; } diff --git a/src/components/imageLightbox/imageLightbox.spec.tsx b/src/components/imageLightbox/imageLightbox.spec.tsx new file mode 100644 index 00000000000000..c38103fad92b54 --- /dev/null +++ b/src/components/imageLightbox/imageLightbox.spec.tsx @@ -0,0 +1,173 @@ +import {describe, expect, it, vi} from 'vitest'; + +import {getImageUrl, getValidDimensions} from 'sentry-docs/components/imageLightbox'; +import {isAllowedRemoteImage, isExternalImage} from 'sentry-docs/config/images'; + +// Mock image config functions +vi.mock('sentry-docs/config/images', () => ({ + isExternalImage: vi.fn((src: string) => src.startsWith('http') || src.startsWith('//')), + isAllowedRemoteImage: vi.fn((src: string) => { + try { + // Handle protocol-relative URLs + const url = src.startsWith('//') ? new URL('https:' + src) : new URL(src); + return url.hostname === 'allowed-domain.com'; + } catch { + return false; + } + }), +})); + +const shouldUseNextImage = (width?: number, height?: number, src?: string) => { + const hasValidDimensions = getValidDimensions(width, height) !== null; + if (!hasValidDimensions || !src) return false; + + return !isExternalImage(src) || isAllowedRemoteImage(src); +}; + +// Test data +const issuesPageImages = [ + {src: './img/issue_page.png', resolved: '/docs/product/issues/img/issue_page.png'}, + {src: './img/error_level.png', resolved: '/docs/product/issues/img/error_level.png'}, + {src: './img/issue_sort.png', resolved: '/docs/product/issues/img/issue_sort.png'}, +]; + +describe('ImageLightbox Helper Functions', () => { + describe('getImageUrl', () => { + it('should handle local image URL resolution', () => { + // Test that local images use the resolved imgPath + const localCases = [ + {src: '/local/image.jpg', imgPath: '/resolved/image.jpg'}, + {src: './relative/image.png', imgPath: '/absolute/resolved/image.png'}, + ]; + + localCases.forEach(({src, imgPath}) => { + expect(getImageUrl(src, imgPath)).toBe(imgPath); + }); + }); + + it('should return original src for external HTTP images', () => { + expect(getImageUrl('https://example.com/image.jpg', '/fallback.jpg')).toBe( + 'https://example.com/image.jpg' + ); + }); + + it('should normalize protocol-relative URLs', () => { + expect(getImageUrl('//example.com/image.jpg', '/fallback.jpg')).toBe( + 'https://example.com/image.jpg' + ); + }); + }); + + describe('getValidDimensions', () => { + it('should return valid dimensions object for positive numbers', () => { + const validCases = [ + {width: 800, height: 600}, + {width: 1920, height: 1080}, + {width: 1200, height: 800}, // Issues page dimensions + {width: 32, height: 32}, // Small icon dimensions + ]; + + validCases.forEach(({width, height}) => { + expect(getValidDimensions(width, height)).toEqual({width, height}); + }); + }); + + it('should return null for invalid dimensions', () => { + const invalidCases = [ + [0, 600], + [800, -1], + [NaN, 600], + [800, NaN], + [undefined, 600], + [800, undefined], + [null, 600], + [800, null], + ]; + + invalidCases.forEach(([width, height]) => { + expect(getValidDimensions(width as any, height as any)).toBeNull(); + }); + }); + }); + + describe('shouldUseNextImage logic', () => { + it('should use Next.js Image for local images with valid dimensions', () => { + const localImageCases = [ + {width: 800, height: 600, src: '/local/image.jpg'}, + {width: 1200, height: 800, src: './img/issue_page.png'}, + {width: 32, height: 32, src: './img/error_level.png'}, + ]; + + localImageCases.forEach(({width, height, src}) => { + expect(shouldUseNextImage(width, height, src)).toBe(true); + }); + }); + + it('should use Next.js Image for allowed external images with valid dimensions', () => { + expect(shouldUseNextImage(800, 600, 'https://allowed-domain.com/image.jpg')).toBe( + true + ); + }); + + it('should not use Next.js Image for disallowed external images', () => { + expect(shouldUseNextImage(800, 600, 'https://external-domain.com/image.jpg')).toBe( + false + ); + }); + + it('should not use Next.js Image without valid dimensions', () => { + const invalidCases = [ + {width: undefined, height: 600, src: '/local/image.jpg'}, + {width: 800, height: undefined, src: '/local/image.jpg'}, + {width: 0, height: 600, src: '/local/image.jpg'}, + {width: 1200, height: 800, src: undefined}, + ]; + + invalidCases.forEach(({width, height, src}) => { + expect(shouldUseNextImage(width, height, src)).toBe(false); + }); + }); + }); + + describe('Issues page scenarios', () => { + it('should handle Issues page image URLs correctly', () => { + issuesPageImages.forEach(({src, resolved}) => { + expect(getImageUrl(src, resolved)).toBe(resolved); + }); + }); + + it('should handle relative path resolution context', () => { + // When DocImage processes relative paths, imgPath should be the resolved path + const relativeSrc = './img/issue_page.png#1200x800'; + const resolvedImgPath = '/docs/product/issues/img/issue_page.png'; + + expect(getImageUrl(relativeSrc, resolvedImgPath)).toBe(resolvedImgPath); + }); + }); +}); + +describe('Event Handling Logic', () => { + describe('click and keyboard event patterns', () => { + const shouldOpenInNewTab = (button: number, ctrlKey: boolean, metaKey: boolean) => { + return button === 1 || ctrlKey || metaKey; + }; + + const isValidTriggerKey = (key: string) => key === 'Enter' || key === ' '; + + it('should identify new tab interaction patterns', () => { + // Test actual user interaction scenarios + expect(shouldOpenInNewTab(0, false, false)).toBe(false); // Regular click should open lightbox + expect(shouldOpenInNewTab(1, false, false)).toBe(true); // Middle click should open new tab + expect(shouldOpenInNewTab(0, true, false)).toBe(true); // Ctrl+click should open new tab + expect(shouldOpenInNewTab(0, false, true)).toBe(true); // Cmd+click should open new tab + }); + + it('should identify valid keyboard triggers for lightbox', () => { + // Test accessibility keyboard patterns + expect(isValidTriggerKey('Enter')).toBe(true); // Standard activation + expect(isValidTriggerKey(' ')).toBe(true); // Space bar activation + expect(isValidTriggerKey('Tab')).toBe(false); // Tab should not trigger + expect(isValidTriggerKey('Escape')).toBe(false); // Escape should not trigger + }); + }); +}); diff --git a/src/components/imageLightbox/index.tsx b/src/components/imageLightbox/index.tsx index 63173f41cdb37f..1ceaaf1d97def7 100644 --- a/src/components/imageLightbox/index.tsx +++ b/src/components/imageLightbox/index.tsx @@ -18,7 +18,7 @@ interface ImageLightboxProps width?: number; } -const getImageUrl = (src: string, imgPath: string): string => { +export const getImageUrl = (src: string, imgPath: string): string => { if (isExternalImage(src)) { // Normalize protocol-relative URLs to use https: return src.startsWith('//') ? `https:${src}` : src; @@ -26,12 +26,15 @@ const getImageUrl = (src: string, imgPath: string): string => { return imgPath; }; -type ValidDimensions = { +export type ValidDimensions = { height: number; width: number; }; -const getValidDimensions = (width?: number, height?: number): ValidDimensions | null => { +export const getValidDimensions = ( + width?: number, + height?: number +): ValidDimensions | null => { if ( width != null && height != null && diff --git a/src/components/lightbox/lightbox.spec.tsx b/src/components/lightbox/lightbox.spec.tsx new file mode 100644 index 00000000000000..28c33da0a3e32f --- /dev/null +++ b/src/components/lightbox/lightbox.spec.tsx @@ -0,0 +1,85 @@ +import {describe, expect, it, vi} from 'vitest'; + +// Shared test data and helper functions +const issuesPageImages = { + issue_page: {width: 1200, height: 800, path: './img/issue_page.png'}, + error_level: {width: 32, height: 32, path: './img/error_level.png'}, + issue_sort: {width: 600, height: 400, path: './img/issue_sort.png'}, +}; + +// Helper functions +const testControlledState = ( + controlledOpen?: boolean, + onOpenChange?: (open: boolean) => void +) => { + const isControlled = controlledOpen !== undefined; + + if (isControlled) { + return { + open: controlledOpen, + setOpen: onOpenChange ?? (() => {}), + isControlled: true, + }; + } + return { + open: false, // initial state + setOpen: () => {}, // would be setState + isControlled: false, + }; +}; + +const shouldShowCloseButton = (closeButton?: boolean) => closeButton !== false; + +describe('Lightbox Component Logic', () => { + describe('state management', () => { + it('should correctly implement controlled vs uncontrolled component pattern', () => { + // Controlled state + const mockOnChange = vi.fn(); + const controlled = testControlledState(true, mockOnChange); + expect(controlled.open).toBe(true); + expect(controlled.isControlled).toBe(true); + + // Uncontrolled state + const uncontrolled = testControlledState(); + expect(uncontrolled.open).toBe(false); + expect(uncontrolled.isControlled).toBe(false); + }); + }); + + describe('props and content handling', () => { + describe('closeButton prop handling', () => { + it('should show close button by default', () => { + expect(shouldShowCloseButton()).toBe(true); + }); + + it('should show close button when explicitly enabled', () => { + expect(shouldShowCloseButton(true)).toBe(true); + }); + + it('should hide close button when explicitly disabled', () => { + expect(shouldShowCloseButton(false)).toBe(false); // Future proofing: no explicit disabled state, here for API completeness if reused differently down the line + }); + }); + }); +}); + +describe('Issues Page Specific Lightbox Scenarios', () => { + describe('image content handling', () => { + it('should handle Issues page image dimensions correctly', () => { + Object.values(issuesPageImages).forEach(({width, height}) => { + expect(width).toBeGreaterThan(0); + expect(height).toBeGreaterThan(0); + expect(typeof width).toBe('number'); + expect(typeof height).toBe('number'); + }); + }); + + it('should handle relative path content in lightbox', () => { + Object.values(issuesPageImages).forEach(({path}) => { + expect(path).toMatch(/^\.\/img\/[a-z_]+\.png$/); + expect(path).toContain('./img/'); + expect(path).toContain('.png'); + }); + }); + }); +});