Feature/image block #21
Conversation
WalkthroughThis change introduces a comprehensive, fully featured Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ImageBlock
participant IntersectionObserver
participant Browser
participant ZoomModal
User->>ImageBlock: Renders with props (src, alt, etc.)
ImageBlock->>IntersectionObserver: Observe image in viewport (if lazy)
IntersectionObserver-->>ImageBlock: Notify when image enters view
ImageBlock->>Browser: Load image (src/srcSet)
Browser-->>ImageBlock: onLoad/onError event
ImageBlock-->>User: Show image or error UI
User->>ImageBlock: Clicks image (if zoomable)
ImageBlock->>ZoomModal: Open modal with enlarged image
User->>ZoomModal: Clicks outside image
ZoomModal->>ImageBlock: Close modal
Possibly related issues
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm warn config production Use Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. ✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 22
🧹 Nitpick comments (12)
docs/.storybook/preview-head.html (1)
1-14: Include global box-sizing reset (optional)
It's recommended to add a globalbox-sizing: border-box;rule to ensure predictable sizing across elements.
Apply this diff at the top of your style block:<style> + *, *::before, *::after { + box-sizing: border-box; + } /* Add global styles here */examples/ImageBlockExample.tsx (2)
4-101: Consider breaking down the large component into smaller sections.The component has 92 lines which exceeds the recommended limit of 50 lines. While this is acceptable for a demo component, it could be improved for maintainability.
Consider extracting each section into separate demo components:
+ const BasicImageDemo = () => ( + <section style={{ marginBottom: '3rem' }}> + <h2>Basic Image</h2> + <ImageBlock + id="basic-image" + src="https://picsum.photos/800/450" + alt="A beautiful landscape" + width={800} + height={450} + /> + </section> + ); + const StyledImageDemo = () => ( + <section style={{ marginBottom: '3rem' }}> + <h2>Image with Border and Shadow</h2> + <ImageBlock + id="styled-image" + src="https://picsum.photos/800/500" + alt="Styled image with border and shadow" + width={800} + height={500} + borderRadius="lg" + shadow="lg" + hasBorder + /> + </section> + ); const ImageBlockExample = () => { return ( <div style={{ maxWidth: '1200px', margin: '0 auto', padding: '2rem', fontFamily: 'sans-serif' }}> <h1>ImageBlock Examples</h1> - <section style={{ marginBottom: '3rem' }}> - <h2>Basic Image</h2> - <ImageBlock - id="basic-image" - src="https://picsum.photos/800/450" - alt="A beautiful landscape" - width={800} - height={450} - /> - </section> + <BasicImageDemo /> + <StyledImageDemo /> {/* ... other sections */} </div> ); };🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[warning] 4-4: examples/ImageBlockExample.tsx#L4
Method ImageBlockExample has 92 lines of code (limit is 50)
13-17: Consider adding error handling for external image dependencies.The component relies on external image services (picsum.photos) which may be unavailable, causing broken images in the demo.
Consider adding fallback images or error handling:
<ImageBlock id="basic-image" src="https://picsum.photos/800/450" alt="A beautiful landscape" width={800} height={450} + onError={() => console.warn('Failed to load demo image')} />src/App.tsx (1)
5-7: Remove duplicate container styling.There are nested containers with similar styling which creates redundant structure and inconsistent spacing.
Consolidate the container styling:
- <div style={{ padding: '2rem', maxWidth: '800px', margin: '0 auto' }}> - <h1>ImageBlock Test</h1> - <div style={{ maxWidth: '800px', margin: '0 auto', padding: '2rem' }}> + <div style={{ maxWidth: '800px', margin: '0 auto', padding: '2rem' }}> <h1>ImageBlock Component Demo</h1>src/blocks/media-block/ImageZoomDemo.tsx (1)
56-61: Consider extracting the LQIP data URI to a constant.The hardcoded LQIP data URI makes the code less readable and harder to maintain.
Extract the LQIP to a constant:
+ const DEFAULT_LQIP = "data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSI4MDAiIGhlaWdodD0iNjAwIiB2aWV3Qm94PSIwIDAgODAwIDYwMCI+PHJlY3Qgd2lkdGg9IjEwMCUiIGhlaWdodD0iMTAwJSIgZmlsbD0iI2YzZjRmNiIvPjx0ZXh0IHg9IjUwJSIgeT0iNTAlIiBmb250LWZhbWlseT0ic2Fucy1zZXJpZiIgZm9udC1zaXplPSIyNCIgZG9taW5hbnQtYmFzZWxpbmU9Im1pZGRsZSIgdGV4dC1hbmNob3I9Im1pZGRsZSIgZmlsbD0jOWM5YzljPkxvYWRpbmc8L3RleHQ+PC9zdmc+"; <ImageBlock id={image.id} src={image.src} alt={image.alt} width={image.width} height={image.height} caption={image.caption} zoomable borderRadius="md" hasBorder={false} loading="lazy" - // Add a low-quality image placeholder (LQIP) for better UX - lqip="data:image/svg+xml;base64,PHN2ZyB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciIHdpZHRoPSI4MDAiIGhlaWdodD0iNjAwIiB2aWV3Qm94PSIwIDAgODAwIDYwMCI+PHJlY3Qgd2lkdGg9IjEwMCUiIGhlaWdodD0iMTAwJSIgZmlsbD0iI2YzZjRmNiIvPjx0ZXh0IHg9IjUwJSIgeT0iNTAlIiBmb250LWZhbWlseT0ic2Fucy1zZXJpZiIgZm9udC1zaXplPSIyNCIgZG9taW5hbnQtYmFzZWxpbmU9Im1pZGRsZSIgdGV4dC1hbmNob3I9Im1pZGRsZSIgZmlsbD0jOWM5YzljPkxvYWRpbmc8L3RleHQ+PC9zdmc+" + lqip={DEFAULT_LQIP} />src/blocks/media-block/image-block/ImageBlock.test.tsx (2)
68-80: Consider improving the image load test reliability.The test acknowledges that it can't reliably test the onLoad callback without mocking image loading behavior.
Consider adding a more comprehensive image load test:
it('handles image load', () => { const mockOnLoad = vi.fn(); render(<ImageBlock {...defaultProps} onLoad={mockOnLoad} loading="eager" />); // The image should be in the document with correct attributes const img = screen.getByRole('img'); expect(img).toBeInTheDocument(); expect(img).toHaveAttribute('src', defaultProps.src); expect(img).toHaveAttribute('alt', defaultProps.alt); - // The onLoad is handled internally, we can't reliably test it - // without mocking the image loading behavior + // Simulate image load event + fireEvent.load(img); + expect(mockOnLoad).toHaveBeenCalledTimes(1); });You'll need to import
fireEventfrom@testing-library/react.
95-103: Enhance zoom functionality test specificity.The zoom test only checks for the presence of a style attribute, which is quite generic. Consider making it more specific to zoom-related styling.
Make the zoom test more specific:
it('applies zoomable styles when zoomable prop is true', () => { const { container } = render(<ImageBlock {...defaultProps} zoomable />); - // The component uses Framer Motion for zooming, which adds a div wrapper const wrapper = container.firstChild as HTMLElement; - // Check if the wrapper has the motion component class or style - expect(wrapper).toHaveAttribute('style'); + // Check for zoom-specific attributes or data attributes + expect(wrapper).toHaveAttribute('data-zoomable', 'true'); + // Or check for cursor pointer style indicating clickable zoom + expect(wrapper).toHaveStyle('cursor: zoom-in'); });docs/stories/blocks/ImageZoomDemo.stories.tsx (1)
48-62: Consider using local or more reliable image sources.Using external Unsplash URLs in stories can lead to inconsistent behavior if the images become unavailable or change. Consider using local placeholder images or a more reliable CDN for better development experience and testing consistency.
Consider replacing with local images or a reliable placeholder service:
- src="https://images.unsplash.com/photo-1470071459604-3b5ec3a7fe05?w=1200" + src="/assets/demo-images/landscape.jpg"src/blocks/media-block/ImageBlock.final.tsx (1)
79-82: Enhance error handling with more descriptive error information.The error handling could provide more context about what failed (image source, component ID) to improve debugging.
Apply this diff to enhance error handling:
- const handleImageError = (e: React.SyntheticEvent<HTMLImageElement, Event>) => { - console.error('Failed to load image:', e); - onImageError?.(new Error('Failed to load image')); - }; + const handleImageError = (e: React.SyntheticEvent<HTMLImageElement, Event>) => { + const errorMessage = `Failed to load image: ${src} (component ID: ${id})`; + console.error(errorMessage, e); + onImageError?.(new Error(errorMessage)); + };src/blocks/media-block/image-block/ImageBlock.tsx (1)
92-106: Improve type safety in handleLoad function.The type casting on line 103 can be improved for better type safety.
Apply this diff to improve type safety:
-onLoad?.(e as unknown as React.SyntheticEvent<HTMLImageElement, Event>); +if (onLoad && e instanceof Event) { + const syntheticEvent = { + ...e, + currentTarget: img, + target: img, + } as React.SyntheticEvent<HTMLImageElement, Event>; + onLoad(syntheticEvent); +}src/blocks/media-block/image-block/SimpleImageBlock.tsx (2)
107-131: Improve IntersectionObserver cleanup and implementation.The observer cleanup could be more robust, and the implementation uses both a local variable and ref unnecessarily.
React.useEffect(() => { if (!lazyLoad || hasError || isInView || !imgRef.current) return; - const observer = new IntersectionObserver( + observerRef.current = new IntersectionObserver( ([entry]) => { if (entry.isIntersecting) { setIsInView(true); - observer.disconnect(); + observerRef.current?.disconnect(); + observerRef.current = null; } }, { rootMargin: '200px', threshold: 0.01 } ); - observer.observe(imgRef.current); - observerRef.current = observer; + observerRef.current.observe(imgRef.current); return () => { if (observerRef.current) { observerRef.current.disconnect(); + observerRef.current = null; } }; }, [lazyLoad, hasError, isInView]);
155-210: Consider optimizing style calculations for better performance.Style objects are recreated on every render. For better performance when rendering multiple images, consider memoizing the styles or moving static styles outside the component.
+// Move outside component or to a separate styles file +const staticStyles = { + placeholder: { + position: 'absolute' as const, + top: 0, + left: 0, + width: '100%', + height: '100%', + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + backgroundColor: '#f5f5f5', + }, + caption: { + textAlign: 'center' as const, + marginTop: '8px', + fontSize: '0.875rem', + color: '#666', + }, +}; export const SimpleImageBlock: React.FC<SimpleImageBlockProps> = ({...}) => { // ... existing code ... - const containerStyle: React.CSSProperties = { + const containerStyle = React.useMemo<React.CSSProperties>(() => ({ position: 'relative', width: typeof width === 'number' ? `${width}px` : width, height: typeof height === 'number' ? `${height}px` : height, overflow: 'hidden', borderRadius: typeof borderRadius === 'number' ? `${borderRadius}px` : borderRadius, border: hasBorder ? '1px solid #e2e8f0' : 'none', backgroundColor: '#f8f8f8', - }; + }), [width, height, borderRadius, hasBorder]); - const imageStyle: React.CSSProperties = { + const imageStyle = React.useMemo<React.CSSProperties>(() => ({ display: 'block', width: '100%', height: '100%', objectFit, transition: `opacity ${fadeInDuration}ms ease-in-out`, opacity: isLoaded ? 1 : 0, - }; + }), [objectFit, fadeInDuration, isLoaded]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/test-image.jpgis excluded by!**/*.jpg
📒 Files selected for processing (25)
docs/.storybook/preview-head.html(1 hunks)docs/.storybook/preview.tsx(1 hunks)docs/.storybook/test-imports.ts(1 hunks)docs/.storybook/tsconfig.json(1 hunks)docs/stories/blocks/ImageZoomDemo.stories.tsx(1 hunks)docs/stories/blocks/SimpleImageTest.stories.tsx(1 hunks)docs/stories/blocks/types.ts(1 hunks)examples/ImageBlockExample.tsx(1 hunks)public/test-image.html(1 hunks)src/App.tsx(1 hunks)src/blocks/ai-block/types.ts(1 hunks)src/blocks/media-block/ImageBlock.final.tsx(1 hunks)src/blocks/media-block/ImageBlock.test.tsx(1 hunks)src/blocks/media-block/ImageZoomDemo.tsx(1 hunks)src/blocks/media-block/image-block/ImageBlock.test.tsx(1 hunks)src/blocks/media-block/image-block/ImageBlock.tsx(1 hunks)src/blocks/media-block/image-block/SimpleImageBlock.tsx(1 hunks)src/blocks/media-block/image-block/index.ts(1 hunks)src/blocks/media-block/image-block/tsconfig.json(1 hunks)src/blocks/media-block/image-block/types.ts(1 hunks)src/blocks/media-block/tsconfig.json(1 hunks)src/blocks/media-block/types.ts(1 hunks)src/tsconfig.json(1 hunks)tsconfig.imageblock.json(1 hunks)tsconfig.paths.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/blocks/media-block/ImageZoomDemo.tsx (1)
src/blocks/media-block/ImageBlock.final.tsx (1)
ImageBlock(34-119)
docs/stories/blocks/types.ts (2)
src/blocks/media-block/types.ts (2)
BorderRadiusScale(1-1)ShadowScale(2-2)docs/stories/surfaces/NotificationToast.stories.tsx (1)
Error(66-76)
src/blocks/media-block/types.ts (1)
docs/stories/blocks/types.ts (2)
BorderRadiusScale(3-3)ShadowScale(4-4)
src/blocks/media-block/image-block/types.ts (1)
src/types.ts (1)
BlockProps(11-26)
🪛 GitHub Check: Codacy Static Code Analysis
examples/ImageBlockExample.tsx
[warning] 4-4: examples/ImageBlockExample.tsx#L4
Method ImageBlockExample has 92 lines of code (limit is 50)
src/blocks/media-block/ImageBlock.final.tsx
[failure] 51-51: src/blocks/media-block/ImageBlock.final.tsx#L51
'onError' is defined but never used.
src/blocks/media-block/ImageBlock.test.tsx
[warning] 24-24: src/blocks/media-block/ImageBlock.test.tsx#L24
Unexpected aliasing of 'this' to local variable.
[warning] 51-51: src/blocks/media-block/ImageBlock.test.tsx#L51
Unexpected any. Specify a different type.
[warning] 113-113: src/blocks/media-block/ImageBlock.test.tsx#L113
Unexpected any. Specify a different type.
src/blocks/media-block/image-block/SimpleImageBlock.tsx
[failure] 2-2: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L2
'motion' is defined but never used.
[failure] 7-7: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L7
'Position' is defined but never used.
[failure] 62-62: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L62
'id' is defined but never used.
[failure] 82-82: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L82
'props' is defined but never used.
[notice] 244-244: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L244
Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs to an interactive content element.
[notice] 244-244: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L244
Visible, non-interactive elements with click handlers must have at least one keyboard listener.
src/blocks/media-block/image-block/ImageBlock.tsx
[notice] 274-274: src/blocks/media-block/image-block/ImageBlock.tsx#L274
Non-interactive elements should not be assigned mouse or keyboard event listeners.
[notice] 274-274: src/blocks/media-block/image-block/ImageBlock.tsx#L274
Visible, non-interactive elements with click handlers must have at least one keyboard listener.
🪛 Biome (1.9.4)
src/blocks/media-block/ImageBlock.test.tsx
[error] 24-24: This aliasing of this is unnecessary.
Arrow functions inherits this from their enclosing scope.
Safe fix: Use this instead of an alias.
(lint/complexity/noUselessThisAlias)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (21)
src/blocks/ai-block/types.ts (1)
1-6: ChatMessage interface is well-defined
The properties (id,role,content, and optionaltimestamp) accurately capture a chat message contract and align with typical usage.src/blocks/media-block/types.ts (1)
1-2: Type aliases align with existing scale definitions
BorderRadiusScaleandShadowScalemirror the union types used elsewhere (docs/stories/blocks/types.ts), ensuring consistency in styling options.src/blocks/media-block/tsconfig.json (1)
1-7:✅ Verification successful
Verify the base
tsconfig.jsonpath
Ensure that the file being extended (../../../tsconfig.json) actually exists at this relative path in the repo.
🏁 Script executed:
#!/bin/bash # Check for root tsconfig.json if [ -f tsconfig.json ]; then echo "✅ tsconfig.json found at project root" else echo "❌ tsconfig.json not found at project root" exit 1 fiLength of output: 111
Base tsconfig.json path verified
The roottsconfig.jsonfile exists and is correctly referenced by../../../tsconfig.jsonfromsrc/blocks/media-block/tsconfig.json. No further changes needed.tsconfig.paths.json (1)
1-9: Alias configuration looks correct
ThebaseUrlandpathsentries ("@/*"and"@vibing-ai/block-kit/*") properly map tosrc/*, enabling cleaner imports across the codebase.src/blocks/media-block/image-block/index.ts (1)
1-2: Well-structured index file. The defaultImageBlockcomponent and its type exports are correctly re-exported, making downstream imports clean and consistent.src/tsconfig.json (1)
1-12: Solid base TypeScript configuration forsrc. Extending from the root config and setting upbaseUrlwith path aliases (@vibing-ai/block-kit/*and@/*) ensures cleaner module resolution. Theincludeandexcludepatterns appropriately capture all TS/TSX sources while omittingnode_modules.docs/.storybook/tsconfig.json (1)
1-12: Appropriate Storybook TypeScript config. Extending the root config, this enables the new JSX transform and necessary interoperability flags (esModuleInterop,allowSyntheticDefaultImports), enforces strict type checks, and skips library declaration checks. The include/exclude globs correctly cover your Storybook files.src/blocks/media-block/ImageZoomDemo.tsx (1)
2-2:✅ Verification successful
Verify the ImageBlock import path.
The import uses a relative path
./which may not resolve correctly depending on the file structure.Please verify the import resolves correctly:
🏁 Script executed:
#!/bin/bash # Description: Check the file structure and available ImageBlock exports # Check the current directory structure fd "ImageBlock" --type f src/blocks/media-block/ # Check what's exported from the index file cat src/blocks/media-block/index.ts 2>/dev/null || echo "No index.ts found"Length of output: 513
Import path is valid
Theindex.tsinsrc/blocks/media-block/re-exports from./ImageBlock.tsx, soimport { ImageBlock } from './';correctly resolves to
src/blocks/media-block/ImageBlock.tsx. No changes needed.src/blocks/media-block/image-block/ImageBlock.test.tsx (2)
5-26: Excellent IntersectionObserver mocking setup.The IntersectionObserver mocking is properly implemented for both window and global scopes, which ensures compatibility across different test environments.
40-52: Good test lifecycle management.The use of fake timers and proper cleanup in beforeAll, afterEach, and afterAll hooks demonstrates excellent test hygiene and prevents test interference.
docs/stories/blocks/types.ts (1)
7-13: Good interface design for Storybook story props.The interface properly extends the base
ImageBlockPropswhile customizing event handlers for Storybook's needs. The addition of optional styling properties (borderRadius,shadow,lightbox) provides good flexibility for story demonstrations.docs/.storybook/preview.tsx (3)
1-1: Consider the implications of TypeScript no-check directive.While
@ts-nocheckmight be necessary for Storybook compatibility, it disables type checking which could hide potential issues. Ensure this is truly necessary and consider enabling checks if possible.
8-50: Well-implemented ErrorBoundary component.The ErrorBoundary follows React best practices with proper lifecycle methods, error logging, and styled error display. The inline styles provide clear visual feedback for errors in Storybook stories.
52-76: Solid Storybook configuration with good defaults.The preview configuration properly sets up actions, controls, error boundary decoration, and theme switching. The decorators provide a robust development environment for testing components.
docs/stories/blocks/ImageZoomDemo.stories.tsx (1)
1-83: Well-structured Storybook stories with comprehensive documentation.The stories provide good examples of ImageBlock usage with proper meta configuration, documentation, and demonstration of zoom functionality. The custom images story effectively shows practical usage patterns.
src/blocks/media-block/ImageBlock.final.tsx (1)
34-119: Well-structured component with good styling and accessibility support.The component provides comprehensive styling options, proper event handling, and good accessibility with alt text support. The implementation follows React best practices for functional components.
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[failure] 51-51: src/blocks/media-block/ImageBlock.final.tsx#L51
'onError' is defined but never used.docs/stories/blocks/SimpleImageTest.stories.tsx (1)
1-58: LGTM! Well-structured test component for Storybook.The
TestImagecomponent is appropriately simple for testing image loading behavior, with proper event handlers and responsive styling. The Storybook configuration follows best practices.public/test-image.html (1)
1-189: LGTM! Comprehensive manual test page for image loading.This test page effectively demonstrates three different image loading approaches and provides clear visual feedback for success/failure states. The use of CDNs and inline scripts is appropriate for a standalone test page.
src/blocks/media-block/image-block/ImageBlock.tsx (1)
1-358: Well-implemented image component with comprehensive features!The
ImageBlockcomponent is feature-rich with excellent support for:
- Lazy loading with IntersectionObserver
- Responsive images via
<picture>element- LQIP (blur-up placeholder) support
- Zoom/lightbox functionality with smooth animations
- Comprehensive error handling and loading states
- Flexible styling options
Please address the accessibility issue for keyboard users and the minor code improvements suggested above.
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 274-274: src/blocks/media-block/image-block/ImageBlock.tsx#L274
Non-interactive elements should not be assigned mouse or keyboard event listeners.
[notice] 274-274: src/blocks/media-block/image-block/ImageBlock.tsx#L274
Visible, non-interactive elements with click handlers must have at least one keyboard listener.src/blocks/media-block/image-block/SimpleImageBlock.tsx (1)
244-244:❓ Verification inconclusive
Add keyboard support and ARIA attributes for zoomable images.
Interactive elements must be keyboard accessible. The container has a click handler but lacks keyboard support and proper ARIA attributes.
- <div style={containerStyle} className={className} onClick={toggleZoom}> + <div + style={containerStyle} + className={className} + onClick={toggleZoom} + onKeyDown={(e) => { + if (zoomable && (e.key === 'Enter' || e.key === ' ')) { + e.preventDefault(); + toggleZoom(); + } + }} + role={zoomable ? 'button' : undefined} + tabIndex={zoomable ? 0 : undefined} + aria-label={zoomable ? `${isZoomed ? 'Close' : 'Open'} image zoom` : undefined} + style={{ + ...containerStyle, + cursor: zoomable ? 'zoom-in' : 'default', + outline: 'none', + }} + >Additionally, consider adding focus styles:
/* Add to your CSS */ .image-container:focus-visible { outline: 2px solid #4A90E2; outline-offset: 2px; }Let me verify if there are existing accessibility patterns in the codebase:
🏁 Script executed:
#!/bin/bash # Search for keyboard event handlers and ARIA attributes in other components rg -A 3 "onKeyDown|role=|aria-" --type tsx --type tsLength of output: 82
🏁 Script executed:
#!/bin/bash # Search for existing accessibility patterns (keyboard handlers, roles, ARIA attributes) rg -E -g '*.ts' -g '*.tsx' -A3 '(onKeyDown|role=|aria-)'Length of output: 465
🏁 Script executed:
#!/bin/bash # Retry searching for accessibility handlers in TypeScript/TSX files rg --type ts --type tsx -A3 'onKeyDown|role=|aria-'Length of output: 81
Add keyboard support and ARIA attributes to the image‐zoom container
The clickable
<div>atsrc/blocks/media-block/image-block/SimpleImageBlock.tsx:244needs to be keyboard-accessible and announce its functionality to assistive technologies.• Add
role="button"andtabIndex={0}to expose it as an interactive element.
• Implement anonKeyDownhandler for Enter/Space activation.
• Include anaria-labelthat describes the action.
• Add focus‐visible styles so keyboard users see when it’s focused.Suggested diff:
- <div style={containerStyle} className={className} onClick={toggleZoom}> + <div + style={{ + ...containerStyle, + cursor: 'zoom-in', + }} + className={`${className} image-container`} + role="button" + tabIndex={0} + aria-label={isZoomed ? 'Close image zoom' : 'Open image zoom'} + onClick={toggleZoom} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + toggleZoom(); + } + }} + >And in your CSS (or module):
.image-container:focus-visible { outline: 2px solid #4A90E2; outline-offset: 2px; }These changes ensure that Enter/Space toggles zoom, assistive tech announces the control, and focus is visible. Please verify this approach against any existing accessibility patterns in our codebase.
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 244-244: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L244
Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs to an interactive content element.
[notice] 244-244: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L244
Visible, non-interactive elements with click handlers must have at least one keyboard listener.src/blocks/media-block/image-block/types.ts (1)
3-96: Well-structured type definitions!The type definitions are comprehensive and well-documented with:
- Clear JSDoc comments for all properties
- Appropriate use of union types for constrained values
- Good separation of concerns with dedicated types for scales
- Proper typing for event handlers
| /** Callback when image loads successfully */ | ||
| onLoad?: () => void; | ||
| /** Callback when image fails to load */ | ||
| onError?: (error: Error) => void; |
There was a problem hiding this comment.
Fix callback signatures to match React event handlers
The current onLoad?: () => void and onError?: (error: Error) => void signatures don't match the React <img> event APIs (which pass a SyntheticEvent). This can lead to type mismatches when passing these props directly to an <img>.
Apply this diff to update the types and import React for the event types:
export interface ImageBlockBaseProps {
- /** Callback when image loads successfully */
- onLoad?: () => void;
- /** Callback when image fails to load */
- onError?: (error: Error) => void;
+ /** Callback when image loads successfully */
+ onLoad?: (event: React.SyntheticEvent<HTMLImageElement, Event>) => void;
+ /** Callback when image fails to load */
+ onError?: (event: React.SyntheticEvent<HTMLImageElement, Event>) => void;
}And add at the top:
+import React from 'react';🤖 Prompt for AI Agents
In src/blocks/media-block/types.ts around lines 27 to 30, the onLoad and onError
callback types do not match React's <img> event handler signatures, which expect
a SyntheticEvent parameter. Fix this by importing React at the top of the file
and updating onLoad to accept a React.SyntheticEvent<HTMLImageElement> parameter
and onError to accept a React.SyntheticEvent<HTMLImageElement, Event> parameter
to align with React's event types.
| { | ||
| "extends": "./tsconfig.json", | ||
| "compilerOptions": { | ||
| "jsx": "react-jsx", | ||
| "jsxImportSource": "react", | ||
| "esModuleInterop": true, | ||
| "allowSyntheticDefaultImports": true, | ||
| "skipLibCheck": true | ||
| }, | ||
| "include": ["src/blocks/media-block/ImageBlock.tsx"], | ||
| "exclude": ["node_modules"] | ||
| } |
There was a problem hiding this comment.
Invalid include path for ImageBlock.tsx. The include references
"src/blocks/media-block/ImageBlock.tsx"but the actual file lives at
src/blocks/media-block/image-block/ImageBlock.tsx
This mismatch will prevent the component from being type-checked.
Please update to:
- "include": ["src/blocks/media-block/ImageBlock.tsx"],
+ "include": ["src/blocks/media-block/image-block/ImageBlock.tsx"],Also verify that "extends": "./tsconfig.json" accurately points to your base config—if your base lives under src/tsconfig.json, adjust the path accordingly.
🤖 Prompt for AI Agents
In tsconfig.imageblock.json lines 1 to 12, the include path for ImageBlock.tsx
is incorrect; it currently points to "src/blocks/media-block/ImageBlock.tsx" but
the actual file is located at
"src/blocks/media-block/image-block/ImageBlock.tsx". Update the include path to
the correct location. Also verify that the "extends" path correctly points to
your base tsconfig.json file and adjust it if your base config is under a
different directory such as "src/tsconfig.json".
| { | ||
| "extends": "../../../../tsconfig.json", | ||
| "compilerOptions": { | ||
| "composite": true, | ||
| "outDir": "./dist", | ||
| "rootDir": ".", | ||
| "baseUrl": ".", | ||
| "jsx": "react-jsx", | ||
| "esModuleInterop": true, | ||
| "allowSyntheticDefaultImports": true, | ||
| "moduleResolution": "node" | ||
| }, | ||
| "include": ["./SimpleImageBlock.tsx"], | ||
| "exclude": ["node_modules", "dist"] | ||
| } |
There was a problem hiding this comment.
Limited include pattern may omit key files. This config only includes SimpleImageBlock.tsx, excluding ImageBlock.tsx, types.ts, and index.ts. Those files won’t be type-checked or built under this composite project.
Recommend broadening the include and enabling declarations for composites:
- "include": ["./SimpleImageBlock.tsx"],
+ "include": ["*.ts", "*.tsx"],Additionally, since "composite": true is set, add "declaration": true under compilerOptions, and ensure your root tsconfig has a corresponding "references" entry to integrate this sub-project.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| "extends": "../../../../tsconfig.json", | |
| "compilerOptions": { | |
| "composite": true, | |
| "outDir": "./dist", | |
| "rootDir": ".", | |
| "baseUrl": ".", | |
| "jsx": "react-jsx", | |
| "esModuleInterop": true, | |
| "allowSyntheticDefaultImports": true, | |
| "moduleResolution": "node" | |
| }, | |
| "include": ["./SimpleImageBlock.tsx"], | |
| "exclude": ["node_modules", "dist"] | |
| } | |
| { | |
| "extends": "../../../../tsconfig.json", | |
| "compilerOptions": { | |
| "composite": true, | |
| "outDir": "./dist", | |
| "rootDir": ".", | |
| "baseUrl": ".", | |
| "jsx": "react-jsx", | |
| "esModuleInterop": true, | |
| "allowSyntheticDefaultImports": true, | |
| "moduleResolution": "node" | |
| }, | |
| "include": ["*.ts", "*.tsx"], | |
| "exclude": ["node_modules", "dist"] | |
| } |
🤖 Prompt for AI Agents
In src/blocks/media-block/image-block/tsconfig.json lines 1 to 15, the include
pattern only covers SimpleImageBlock.tsx, missing other important files like
ImageBlock.tsx, types.ts, and index.ts, which prevents them from being
type-checked or built. To fix this, broaden the include pattern to cover all
relevant source files, for example by including all .ts and .tsx files in the
directory. Also, since "composite": true is set, add "declaration": true to
compilerOptions to generate declaration files. Finally, ensure the root
tsconfig.json has a "references" entry pointing to this sub-project to properly
integrate it into the build.
| // Run the test when this file is imported | ||
| testImports(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid side effects on module import.
Executing the test function immediately when the file is imported creates side effects that could interfere with testing environments or other consumers of this module.
Consider exporting the function without immediate execution:
// Run the test when this file is imported
- testImports();
+ // testImports(); // Uncomment to run immediately, or call explicitly when needed📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Run the test when this file is imported | |
| testImports(); | |
| // Run the test when this file is imported | |
| // testImports(); // Uncomment to run immediately, or call explicitly when needed |
🤖 Prompt for AI Agents
In docs/.storybook/test-imports.ts at lines 34 to 35, avoid calling
testImports() immediately upon module import to prevent side effects. Instead,
remove the immediate invocation and export the testImports function so that
consumers can call it explicitly when needed.
| 'process.cwd()': process.cwd(), | ||
| '__dirname': __dirname, | ||
| '__filename': __filename, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential undefined values in ES module environments.
The usage of __dirname and __filename may not be available in ES module environments and could throw runtime errors.
Consider adding fallback handling:
root: {
'process.cwd()': process.cwd(),
- '__dirname': __dirname,
- '__filename': __filename,
+ '__dirname': typeof __dirname !== 'undefined' ? __dirname : 'undefined (ES module)',
+ '__filename': typeof __filename !== 'undefined' ? __filename : 'undefined (ES module)',
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'process.cwd()': process.cwd(), | |
| '__dirname': __dirname, | |
| '__filename': __filename, | |
| root: { | |
| 'process.cwd()': process.cwd(), | |
| '__dirname': typeof __dirname !== 'undefined' ? __dirname : 'undefined (ES module)', | |
| '__filename': typeof __filename !== 'undefined' ? __filename : 'undefined (ES module)', | |
| }, |
🤖 Prompt for AI Agents
In docs/.storybook/test-imports.ts around lines 8 to 10, __dirname and
__filename are used directly, which can cause runtime errors in ES module
environments where these variables are undefined. Modify the code to check if
__dirname and __filename are defined before using them, and provide fallback
values or alternatives to safely handle cases when they are undefined.
| } | ||
|
|
||
| export const SimpleImageBlock: React.FC<SimpleImageBlockProps> = ({ | ||
| id, |
There was a problem hiding this comment.
Handle or remove unused destructured properties.
The id prop and spread ...props are destructured but never used in the component. Consider either:
- Pass
idto the root container for block identification - Forward additional props to the container element
- Remove them if not needed
return (
- <div style={containerStyle} className={className} onClick={toggleZoom}>
+ <div id={id} style={containerStyle} className={className} onClick={toggleZoom} {...props}>Also applies to: 82-82
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[failure] 62-62: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L62
'id' is defined but never used.
🤖 Prompt for AI Agents
In src/blocks/media-block/image-block/SimpleImageBlock.tsx at lines 62 and 82,
the destructured `id` and `...props` are not used in the component. To fix this,
either pass the `id` prop to the root container element to maintain block
identification, forward the `...props` to the container to allow additional
properties, or remove these destructured variables entirely if they are
unnecessary.
| import { BlockProps } from '../../../types'; | ||
|
|
||
| // Type definitions | ||
| type Position = 'static' | 'relative' | 'absolute' | 'fixed' | 'sticky'; |
There was a problem hiding this comment.
Remove unused Position type definition.
The Position type is defined but never used in the component.
-type Position = 'static' | 'relative' | 'absolute' | 'fixed' | 'sticky';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| type Position = 'static' | 'relative' | 'absolute' | 'fixed' | 'sticky'; |
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[failure] 7-7: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L7
'Position' is defined but never used.
🤖 Prompt for AI Agents
In src/blocks/media-block/image-block/SimpleImageBlock.tsx at line 7, the
Position type is defined but not used anywhere in the file. Remove the entire
Position type definition to clean up unused code and improve maintainability.
| @@ -0,0 +1,285 @@ | |||
| import * as React from 'react'; | |||
| import { motion, AnimatePresence } from 'framer-motion'; | |||
There was a problem hiding this comment.
Remove unused framer-motion imports.
Neither motion nor AnimatePresence from framer-motion are used in this component.
-import { motion, AnimatePresence } from 'framer-motion';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { motion, AnimatePresence } from 'framer-motion'; |
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[failure] 2-2: src/blocks/media-block/image-block/SimpleImageBlock.tsx#L2
'motion' is defined but never used.
🤖 Prompt for AI Agents
In src/blocks/media-block/image-block/SimpleImageBlock.tsx at line 2, remove the
unused imports `motion` and `AnimatePresence` from the 'framer-motion' package
since they are not used anywhere in the component. This will clean up the import
statements and avoid unnecessary dependencies.
| <img | ||
| ref={imgRef} | ||
| src={defaultSource.src} | ||
| alt={alt} | ||
| style={imageStyle} | ||
| onLoad={handleLoad} | ||
| onError={handleError} | ||
| loading={lazyLoad ? 'lazy' : 'eager'} | ||
| decoding="async" | ||
| /> | ||
|
|
||
| {hasMultipleSources && ( | ||
| <picture> | ||
| {sources | ||
| .filter(source => source.media) | ||
| .map((source, index) => ( | ||
| <source | ||
| key={index} | ||
| srcSet={source.srcSet || source.src} | ||
| media={source.media} | ||
| type={source.type} | ||
| sizes={source.sizes} | ||
| /> | ||
| ))} | ||
| </picture> | ||
| )} |
There was a problem hiding this comment.
Fix incorrect <picture> element structure for responsive images.
The <picture> element should wrap the <img> element, not be a sibling. The current implementation won't provide responsive image functionality.
- <img
- ref={imgRef}
- src={defaultSource.src}
- alt={alt}
- style={imageStyle}
- onLoad={handleLoad}
- onError={handleError}
- loading={lazyLoad ? 'lazy' : 'eager'}
- decoding="async"
- />
-
{hasMultipleSources && (
<picture>
{sources
.filter(source => source.media)
.map((source, index) => (
<source
key={index}
srcSet={source.srcSet || source.src}
media={source.media}
type={source.type}
sizes={source.sizes}
/>
))}
+ <img
+ ref={imgRef}
+ src={defaultSource.src}
+ alt={alt}
+ style={imageStyle}
+ onLoad={handleLoad}
+ onError={handleError}
+ loading={lazyLoad ? 'lazy' : 'eager'}
+ decoding="async"
+ />
</picture>
)}
+ {!hasMultipleSources && (
+ <img
+ ref={imgRef}
+ src={defaultSource.src}
+ alt={alt}
+ style={imageStyle}
+ onLoad={handleLoad}
+ onError={handleError}
+ loading={lazyLoad ? 'lazy' : 'eager'}
+ decoding="async"
+ />
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img | |
| ref={imgRef} | |
| src={defaultSource.src} | |
| alt={alt} | |
| style={imageStyle} | |
| onLoad={handleLoad} | |
| onError={handleError} | |
| loading={lazyLoad ? 'lazy' : 'eager'} | |
| decoding="async" | |
| /> | |
| {hasMultipleSources && ( | |
| <picture> | |
| {sources | |
| .filter(source => source.media) | |
| .map((source, index) => ( | |
| <source | |
| key={index} | |
| srcSet={source.srcSet || source.src} | |
| media={source.media} | |
| type={source.type} | |
| sizes={source.sizes} | |
| /> | |
| ))} | |
| </picture> | |
| )} | |
| {hasMultipleSources && ( | |
| <picture> | |
| {sources | |
| .filter(source => source.media) | |
| .map((source, index) => ( | |
| <source | |
| key={index} | |
| srcSet={source.srcSet || source.src} | |
| media={source.media} | |
| type={source.type} | |
| sizes={source.sizes} | |
| /> | |
| ))} | |
| <img | |
| ref={imgRef} | |
| src={defaultSource.src} | |
| alt={alt} | |
| style={imageStyle} | |
| onLoad={handleLoad} | |
| onError={handleError} | |
| loading={lazyLoad ? 'lazy' : 'eager'} | |
| decoding="async" | |
| /> | |
| </picture> | |
| )} | |
| {!hasMultipleSources && ( | |
| <img | |
| ref={imgRef} | |
| src={defaultSource.src} | |
| alt={alt} | |
| style={imageStyle} | |
| onLoad={handleLoad} | |
| onError={handleError} | |
| loading={lazyLoad ? 'lazy' : 'eager'} | |
| decoding="async" | |
| /> | |
| )} |
🤖 Prompt for AI Agents
In src/blocks/media-block/image-block/SimpleImageBlock.tsx between lines 253 and
278, the <picture> element is incorrectly placed as a sibling of the <img>
element instead of wrapping it. To fix this, move the <img> element inside the
<picture> element so that <picture> wraps around <source> elements and the <img>
fallback, enabling proper responsive image behavior.
| * Custom class name for additional styling | ||
| */ | ||
| className?: string; | ||
| } |
There was a problem hiding this comment.
Remove duplicate className property.
The className property is already inherited from BlockProps. Since you're only omitting onChange, this creates a duplicate definition.
- /**
- * Custom class name for additional styling
- */
- className?: string;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * Custom class name for additional styling | |
| */ | |
| className?: string; | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/blocks/media-block/image-block/types.ts at lines 98 to 101, remove the
duplicate declaration of the className property since it is already inherited
from BlockProps. Keep the omission of onChange but do not redeclare className to
avoid redundancy.
Added image block
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Tests
Chores