-
Notifications
You must be signed in to change notification settings - Fork 19
Fix/typescript config #42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Fixed TypeScript type errors in image components - Resolved all ESLint warnings and errors - Improved type safety with proper type annotations - Updated TypeScript configuration for better project references - Enhanced accessibility in image components - Removed unused variables and imports
- Fix picture element structure in SimpleImageBlock - Improve srcSet generation for responsive images - Fix memory leak in image preloading - Correct fetchPriority attribute usage - Use consistent types for action buttons - Add proper TypeScript types and cleanup
- Rename CleanImageBlock to EnhancedImageBlock to avoid naming conflicts - Remove duplicate type definitions and import from parent module - Remove redundant className property from ImageBlockProps - Add proper type guards for srcset generation - Improve code organization and maintainability
- Make buttonVariants and buttonSizes objects immutable with 'as const' - Update image module declarations in vite-env.d.ts for better type safety - Remove random image generation from SimpleImageBlock - Remove duplicate ImageSource interface and import from types - Remove unused ImageBlockExample component - Add proper error handling and loading states - Improve type safety across components
- Fix import path in PromptContainerFullLineBottomActionsLarge - Update ImageBlock tests to be more reliable - Improve error handling in image loading states
- Fix TypeScript errors in CleanImageBlock - Remove duplicate code and improve code organization - Enhance test coverage for zoom functionality - Improve error handling in image loading
- Fixed TypeScript errors in setupTests.ts - Improved test reliability by checking for zoom state changes - Added proper type declarations for test utilities - Simplified test assertions to focus on behavior over implementation
- Fix TypeScript configuration for media-block - Update Storybook imports and configuration - Clean up duplicate configuration files - Add environment utilities for media-block - Fix module resolution issues
- Fixed TypeScript errors in ImageBlock and CleanImageBlock components - Updated framer-motion mock for testing - Improved type safety and test coverage - Fixed fetchPriority warnings by using correct HTML attribute name
- Fix TypeScript configuration for proper module resolution - Update Storybook configuration to work with Vite and TypeScript - Fix TypeScript errors in ImageBlock component - Add proper type definitions and improve type safety - Update test files to work with new TypeScript configuration - Add proper Storybook decorators and theming
WalkthroughThis update introduces a comprehensive set of new components, utilities, types, tests, Storybook stories, and configuration files. Major features include a robust and accessible Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ImageBlock
participant Browser
participant ZoomOverlay
User->>ImageBlock: Renders with props (src, alt, zoomable, etc.)
ImageBlock->>Browser: Loads image (src, srcSet, etc.)
Browser-->>ImageBlock: onLoad/onError event
User->>ImageBlock: Clicks image (if zoomable)
ImageBlock->>ZoomOverlay: Opens zoom overlay/modal
User->>ZoomOverlay: Interacts (click outside/Escape key)
ZoomOverlay->>ImageBlock: Closes zoom overlay
sequenceDiagram
participant User
participant PromptContainerFullLineBottomActions
participant Actions
participant ParentComponent
User->>PromptContainerFullLineBottomActions: Types in textarea
PromptContainerFullLineBottomActions->>ParentComponent: onChange(value)
User->>PromptContainerFullLineBottomActions: Clicks submit or presses Cmd+Enter
PromptContainerFullLineBottomActions->>ParentComponent: onSubmit(value)
User->>Actions: Clicks custom action button
Actions->>ParentComponent: onClick()
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 error Exit handler never called! ✨ 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 38
🧹 Nitpick comments (49)
src/composition/views/MindMapView.tsx (1)
28-31
: Redundant ESLint disable – underscore already silences the ruleSince the prop is aliased to
_onNodeEdit
, the@typescript-eslint/no-unused-vars
directive is no longer necessary and just clutters the code.-// onNodeEdit is intentionally unused in this component -// eslint-disable-next-line @typescript-eslint/no-unused-vars -onNodeEdit: _onNodeEdit, +// onNodeEdit is intentionally unused in this component +onNodeEdit: _onNodeEdit,docs/.storybook/test-imports.ts (1)
9-36
: Avoid shadowing the exported function nameUsing the same identifier for the function and an inner constant hampers stack traces and readability.
-export function testImports() { - const testImports = { +export function testImports() { + const results = { @@ - console.dir(testImports, { depth: null, colors: true }); + console.dir(results, { depth: null, colors: true }); - return testImports; + return results; }src/__tests__/types.test.ts (1)
1-8
: Consider expanding TypeScript verification coverage.The basic TypeScript test is functional but minimal. Consider adding tests for more TypeScript features to ensure comprehensive setup verification.
Consider adding these additional TypeScript feature tests:
+// Test interfaces and types +interface TestInterface { + name: string; + value: number; +} + +type TestUnion = 'option1' | 'option2' | 'option3'; + // Simple test to verify TypeScript is working const sum = (a: number, b: number): number => a + b; +const createTestObject = (name: string, value: number): TestInterface => ({ + name, + value, +}); + +const testUnion: TestUnion = 'option1'; describe('TypeScript Configuration', () => { it('should work with TypeScript', () => { expect(sum(1, 2)).toBe(3); }); + + it('should handle interfaces and types', () => { + const obj = createTestObject('test', 42); + expect(obj.name).toBe('test'); + expect(obj.value).toBe(42); + expect(testUnion).toBe('option1'); + }); });docs/.storybook/tsconfig.json (1)
10-10
: Consider narrowing the include pattern for better performance.The include pattern
["../**/*.ts", "../**/*.tsx"]
is very broad and might include files that shouldn't be part of the Storybook build. Consider being more specific about which directories and files to include.Consider a more targeted approach:
- "include": ["../**/*.ts", "../**/*.tsx"], + "include": [ + "../stories/**/*", + "../../src/**/*.stories.*", + "../../src/**/*.story.*", + "./**/*" + ],src/types/hmr.d.ts (1)
3-4
: Consider more specific types instead ofany
.The static analysis correctly flags the use of
any
types. Since these represent previous React and ReactDOM references, consider using more specific types:-declare const __vite_react_hmr_previous_React: any; -declare const __vite_react_hmr_previous_ReactDOM: any; +declare const __vite_react_hmr_previous_React: typeof React | undefined; +declare const __vite_react_hmr_previous_ReactDOM: typeof ReactDOM | undefined;This provides better type safety while still handling the HMR state management.
docs/.storybook/preview.ts (1)
6-7
: Track the temporary Provider disabling.The Provider import is properly documented as temporarily disabled to resolve type issues. Consider creating a task to re-enable the Provider once the type issues are resolved.
Do you want me to help investigate the type issues with the Provider or create a tracking issue for this temporary workaround?
src/App.tsx (3)
5-7
: Consider extracting inline styles to CSS modules or styled componentsWhile the inline styles work functionally, consider extracting them to improve maintainability and reusability, especially since there's style duplication between lines 5 and 7.
- <div style={{ padding: '2rem', maxWidth: '800px', margin: '0 auto' }}> + <div className="app-container"> <h1>ImageBlock Test</h1> - <div style={{ maxWidth: '800px', margin: '0 auto', padding: '2rem' }}> + <div className="demo-container">Then create a corresponding CSS file:
.app-container { padding: 2rem; max-width: 800px; margin: 0 auto; } .demo-container { max-width: 800px; margin: 0 auto; padding: 2rem; }
33-33
: Consider using a placeholder service or local images for consistencyThe same Unsplash image URL is used multiple times. Consider using different images or a placeholder service to better demonstrate the component's versatility.
- src="https://images.unsplash.com/photo-1506744038136-46273834b3fb" + src="https://picsum.photos/600/400?random=1"Or use different images:
src="https://images.unsplash.com/photo-1506744038136-46273834b3fb" + src="https://images.unsplash.com/photo-1519904981063-b0cf448d479e"
Also applies to: 48-48, 62-62
24-25
: Add proper error handling for production useThe console.log and console.error calls are fine for demo purposes, but consider more user-friendly error handling for production.
- onLoad={() => console.log('Hero image loaded')} - onError={(e: Error) => console.error('Failed to load hero image', e)} + onLoad={() => console.log('Hero image loaded')} + onError={(e: Error) => { + console.error('Failed to load hero image', e); + // Consider showing a user-friendly error message or fallback + }}scripts/update-storybook-imports.ts (2)
16-20
: Consider adding feedback about successful updatesThe script silently processes files without indicating which ones were updated. Consider adding logging for better visibility.
if (content.includes("@storybook/react-vite")) { const newContent = content.replace( /from ['"]@storybook\/react-vite['"]/g, "from '@storybook/react'" ); writeFileSync(file, newContent, 'utf-8'); - // File updated + console.log(`Updated imports in: ${file}`); }
17-19
: Enhance regex pattern for more robust matchingThe current regex only matches exact import statements. Consider handling edge cases like different quote styles or spacing.
- const newContent = content.replace( - /from ['"]@storybook\/react-vite['"]/g, - "from '@storybook/react'" - ); + const newContent = content.replace( + /from\s+['"]@storybook\/react-vite['"]/g, + "from '@storybook/react'" + );src/stories/button.css (2)
19-30
: Consider using rem units for better accessibilityThe font sizes use px units, which don't scale with user's browser font size preferences. Consider using rem units for better accessibility.
.storybook-button--small { - font-size: 12px; + font-size: 0.75rem; padding: 10px 16px; } .storybook-button--medium { - font-size: 14px; + font-size: 0.875rem; padding: 11px 20px; } .storybook-button--large { - font-size: 16px; + font-size: 1rem; padding: 12px 24px; }
12-12
: Consider using CSS custom properties for colorsHardcoded color values make theming and maintenance more difficult. Consider using CSS custom properties.
+:root { + --storybook-button-primary-bg: #1ea7fd; + --storybook-button-secondary-color: #333; +} + .storybook-button--primary { color: white; - background-color: #1ea7fd; + background-color: var(--storybook-button-primary-bg); } .storybook-button--secondary { - color: #333; + color: var(--storybook-button-secondary-color); background-color: transparent; box-shadow: rgba(0, 0, 0, 0.15) 0px 0px 0px 1px inset; }Also applies to: 17-17
src/types/vibing-ai.d.ts (1)
5-6
: Consider improving type safety for component declarations.While
ComponentType<any>
is a common pattern in declaration files, it bypasses TypeScript's type checking benefits. If the actual prop interfaces forCleanImageBlock
andImageBlock
are available, consider using them for better type safety.- export const CleanImageBlock: ComponentType<any>; - export const ImageBlock: ComponentType<any>; + // If interfaces are available: + export const CleanImageBlock: ComponentType<CleanImageBlockProps>; + export const ImageBlock: ComponentType<ImageBlockProps>;Do you have the actual prop interfaces for these components that I could help you integrate?
src/blocks/media-block/tsconfig.json (1)
13-25
: Consider alignment between test exclusion and type definitions.The configuration excludes test files from compilation (lines 23-24) but includes
@testing-library/jest-dom
types (line 13). This suggests tests are compiled elsewhere, which is fine, but consider whether the jest-dom types are actually needed in this particular compilation unit.src/blocks/media-block/image-block/ImageBlock.test.tsx (2)
94-96
: Consider more robust class checking.The test directly checks for specific CSS classes like
'text-sm', 'text-white', 'text-center'
and'bg-black/50'
. This creates tight coupling between tests and implementation details. Consider testing the visual behavior or computed styles instead of specific class names.- expect(captionElement).toHaveClass('text-sm', 'text-white', 'text-center'); - expect(captionElement).toHaveClass('bg-black/50'); + const styles = window.getComputedStyle(captionElement); + expect(styles.textAlign).toBe('center'); + expect(styles.color).toBe('rgb(255, 255, 255)'); // white
116-122
: Expand accessibility testing coverage.The accessibility tests only verify alt text. Consider adding tests for keyboard navigation, ARIA attributes, focus management, and screen reader compatibility to ensure the component is fully accessible.
describe('Accessibility', () => { it('has proper alt text', () => { render(<ImageBlock {...defaultProps} alt="Test alt text" />); const img = screen.getByAltText('Test alt text'); expect(img).toBeInTheDocument(); }); + + it('is keyboard navigable when zoomable', () => { + render(<ImageBlock {...defaultProps} zoomable />); + const img = screen.getByRole('img'); + expect(img).toHaveAttribute('tabIndex', '0'); + }); + + it('has proper ARIA attributes for interactive elements', () => { + render(<ImageBlock {...defaultProps} zoomable />); + const img = screen.getByRole('img'); + expect(img).toHaveAttribute('role', 'img'); + }); });.storybook/decorators/withProviders.tsx (1)
6-57
: Consider extracting the theme object to a separate file.The theme object is comprehensive and well-structured, but it's quite large and hardcoded in the decorator file. Consider extracting it to a separate theme configuration file for better maintainability and reusability across the project.
Create a new file
theme.ts
:export const defaultTheme = { colors: { // ... theme object content }, // ... rest of theme };Then import it:
+import { defaultTheme } from './theme'; -// Define your theme -const theme = { - colors: { - // ... large theme object - } -};src/blocks/icons/Icons.tsx (1)
8-18
: Simplify className handling in createIcon HOC.The className conditional check is redundant since the
cn
utility function already handles undefined values gracefully.const createIcon = (Component: React.FC<IconProps>) => { const Icon = React.forwardRef<SVGSVGElement, IconProps>(({ className, ...props }, ref) => ( <Component ref={ref} - className={cn('inline-block flex-shrink-0', className ? className : '')} + className={cn('inline-block flex-shrink-0', className)} {...props} /> )); Icon.displayName = Component.name; return Icon; };src/blocks/media-block/ImageBlock.clean.test.tsx (1)
32-35
: Simplify unsafe type casting in mock setup.The complex type casting with
(actual as object)
and(actual as { motion: unknown }).motion as object
is potentially unsafe and makes the code harder to maintain.Consider a simpler approach:
return { - ...(actual as object), + ...actual, motion: { - ...(actual as { motion: unknown }).motion as object, + ...actual.motion, div: createMockComponent('div', 'motion-div'), figure: createMockComponent('figure', 'motion-figure'), button: createMockComponent('button', 'motion-button'), },.storybook/vite.config.ts (1)
61-63
: Consider the security implications of the empty process.env definition.Setting
process.env
to an empty object may break environment variable access in some scenarios. Consider whether this is intentional or if specific environment variables should be whitelisted.If you need to expose specific environment variables, consider:
define: { - 'process.env': {} + 'process.env': { + NODE_ENV: JSON.stringify(process.env.NODE_ENV || 'development'), + // Add other needed env vars here + } }src/types/global.d.ts (1)
31-32
: Remove the unnecessary empty export.The empty export is redundant since this file already has imports at the top, which makes TypeScript treat it as a module.
-// This export is needed for TypeScript to treat this file as a module -export {};src/blocks/media-block/image-block/CleanImageBlock.test.tsx (1)
10-11
: Consider using a more specific type instead ofany
in the mockWhile using
any
in test mocks is generally acceptable, you could improve type safety slightly.-vi.mock('./CleanImageBlock', () => ({ - default: React.forwardRef<HTMLImageElement, any>(({ src, alt, onLoad, onError }, ref) => { +interface CleanImageBlockMockProps { + src?: string; + alt?: string; + onLoad?: (event: React.SyntheticEvent<HTMLImageElement>) => void; + onError?: (event: React.SyntheticEvent<HTMLImageElement>) => void; +} + +vi.mock('./CleanImageBlock', () => ({ + default: React.forwardRef<HTMLImageElement, CleanImageBlockMockProps>(({ src, alt, onLoad, onError }, ref) => {src/blocks/media-block/image-block/ImageBlockComponent.tsx (3)
26-49
: Remove commented code if not needed.The commented constants are well-documented but if they're not being used (as indicated by the comment), consider removing them to reduce clutter.
-// Define constants for radius and shadow values with proper typing -// Note: These are kept as comments for reference but not used directly -// as we're using Tailwind classes instead -/* -const radiusMap = { - none: '0', - sm: '0.25rem', - md: '0.5rem', - lg: '1rem', - xl: '1.5rem', - '2xl': '2rem', - full: '9999px', -} as const; - -const shadowMap = { - none: 'none', - sm: '0 1px 2px 0 rgba(0, 0, 0, 0.05)', - md: '0 4px 6px -1px rgba(0, 0, 0, 0.1), 0 2px 4px -1px rgba(0, 0, 0, 0.06)', - lg: '0 10px 15px -3px rgba(0, 0, 0, 0.1), 0 4px 6px -2px rgba(0, 0, 0, 0.05)', - xl: '0 20px 25px -5px rgba(0, 0, 0, 0.1), 0 10px 10px -5px rgba(0, 0, 0, 0.04)', - '2xl': '0 25px 50px -12px rgba(0, 0, 0, 0.25)', - inner: 'inset 0 2px 4px 0 rgba(0, 0, 0, 0.06)', -} as const; -*/
51-52
: Remove unnecessary comments about unused types.If the
ImageOnlyProps
type is truly unused and only kept for potential future use, consider removing this comment to reduce noise.-// ImageOnlyProps type is kept for future use but currently not used in this file -// It's preserved as it might be used by other components that import this file
74-75
: Complete the implementation or remove placeholder comment.The comment indicates "Rest of the component implementation..." followed by "Previous implementation remains the same", but this appears to be a placeholder that should be removed since the actual implementation follows.
- // Rest of the component implementation... - // [Previous implementation remains the same until the end of the component]docs/stories/surfaces/BlockModal.stories.tsx (1)
113-117
: Inconsistent syntax: Mix of JSX and React.createElement.For consistency, consider converting the remaining
React.createElement
calls to JSX syntax to match the pattern used in other stories.- children: React.createElement('div', { style: { padding: '20px' } }, - <TextBlock - id="fullscreen-text" - text="This is a fullscreen modal that can be used for detailed views or complex interactions. It provides maximum space while still maintaining the modal context." - />, - React.createElement('div', { + children: ( + <div style={{ padding: '20px' }}> + <TextBlock + id="fullscreen-text" + text="This is a fullscreen modal that can be used for detailed views or complex interactions. It provides maximum space while still maintaining the modal context." + /> + <div style={{ - style: { - height: '400px', - background: '#f8fafc', - marginTop: '20px', - display: 'flex', - alignItems: 'center', - justifyContent: 'center' - } - }, "Document preview content would appear here") - ), + height: '400px', + background: '#f8fafc', + marginTop: '20px', + display: 'flex', + alignItems: 'center', + justifyContent: 'center' + }}> + Document preview content would appear here + </div> + </div> + )src/blocks/media-block/image-block/ImageBlock.tsx (1)
119-127
: Simplify ref handling logic.The ref callback function is overly complex. Consider using a more straightforward approach or a helper function.
+ const handleRef = React.useCallback((node: HTMLDivElement | null) => { + if (typeof forwardedRef === 'function') { + forwardedRef(node); + } else if (forwardedRef) { + forwardedRef.current = node; + } + containerRef.current = node; + }, [forwardedRef]); - ref={(node) => { - if (typeof forwardedRef === 'function') { - forwardedRef(node); - } else if (forwardedRef) { - forwardedRef.current = node; - } - // Use type assertion to handle the ref assignment - (containerRef as React.MutableRefObject<HTMLDivElement | null>).current = node; - }} + ref={handleRef}src/blocks/media-block/image-block/ImageBlock.tsx.new (2)
9-25
: Consider safer type definitions for the mapping objects.The current type assertion pattern may not provide the best type safety. Consider using a more explicit approach:
-const radiusMap: Record<BorderRadiusScale, string> = { +const radiusMap = { none: '0', sm: '0.25rem', md: '0.5rem', lg: '1rem', full: '9999px', -} as const; +} as const satisfies Record<BorderRadiusScale, string>;This approach provides better type inference while maintaining type safety.
76-111
: Consider simplifying the source resolution logic.The
getCurrentSrc
function has high complexity. Consider extracting helper functions:+const extractSourceFromString = (src: string): string | null => { + const trimmed = src.trim(); + return trimmed || null; +}; + +const extractSourceFromArray = (srcArray: unknown[]): string | null => { + if (srcArray.length === 0) return null; + const firstItem = srcArray[0]; + if (typeof firstItem === 'string') { + return extractSourceFromString(firstItem); + } + if (firstItem && typeof firstItem === 'object' && 'src' in firstItem) { + return extractSourceFromString(String(firstItem.src)); + } + return null; +}; const getCurrentSrc = useCallback((): string | null => { if (!srcProp) return null; if (typeof srcProp === 'string') { - const trimmed = srcProp.trim(); - return trimmed || null; + return extractSourceFromString(srcProp); } if (Array.isArray(srcProp)) { - // ... complex logic + return extractSourceFromArray(srcProp); } if (typeof srcProp === 'object' && 'src' in srcProp) { - const src = String((srcProp as { src: string }).src).trim(); - return src || null; + return extractSourceFromString(String((srcProp as { src: string }).src)); } return null; }, [srcProp]);src/blocks/media-block/image-block/SimpleImageBlock.tsx (2)
6-7
: Remove redundant PointerEvents type definitionReact already provides proper typing for the
pointer-events
CSS property. This custom type is unnecessary and could cause maintenance issues.Remove this type definition and use the built-in type:
-// Type definitions -type PointerEvents = 'auto' | 'none' | 'visiblePainted' | 'visibleFill' | 'visibleStroke' | 'visible' | 'painted' | 'fill' | 'stroke' | 'all' | 'inherit' | 'initial' | 'revert' | 'unset';Then update line 232 to remove the type assertion:
- pointerEvents: 'none' as PointerEvents, + pointerEvents: 'none',
370-370
: Remove redundant default exportThe component is already exported as a named export on line 51. Having both named and default exports can cause confusion.
-export default SimpleImageBlock;
src/components/Text/Text.stories.tsx (2)
1-1
: Remove empty line at the start of the file- import type { Meta, StoryObj } from '@storybook/react';
153-153
: Add missing newline at end of fileAdd a newline character at the end of the file after line 153.
src/blocks/media-block/ImageBlock.stories.tsx (1)
145-145
: Add missing newline at end of fileAdd a newline character at the end of the file after line 145.
src/blocks/prompt/PromptContainerFullLineBottomActions.tsx (1)
194-194
: Add missing newline at end of fileAdd a newline character at the end of the file after line 194.
src/types/storybook.d.ts (2)
21-21
: Improve type safety by replacingany
typesSeveral places use
any
which reduces type safety. Also, theTArgs
generic parameter inArgTypes
is unused.docs?: { disable?: boolean; - page?: any; + page?: React.ComponentType; description?: { component?: string; }; // In StoryContext - globals: Record<string, any>; + globals: Record<string, unknown>; - export interface ArgTypes<TArgs = ArgsType> { + export interface ArgTypes { [key: string]: { name?: string; description?: string; - defaultValue?: any; - [key: string]: any; + defaultValue?: unknown; + [key: string]: unknown; }; } // In Window interface - __STORYBOOK_ADDONS_CHANNEL__: any; - __STORYBOOK_CLIENT_API__: any; - __STORYBOOK_PREVIEW__: any; - __STORYBOOK_STORY_STORE__: any; + __STORYBOOK_ADDONS_CHANNEL__: unknown; + __STORYBOOK_CLIENT_API__: unknown; + __STORYBOOK_PREVIEW__: unknown; + __STORYBOOK_STORY_STORE__: unknown;Also applies to: 51-51, 55-62, 85-88
91-91
: Add missing newline at end of fileAdd a newline character at the end of the file after line 91.
src/blocks/media-block/ImageBlock.new.test.tsx (1)
387-389
: Replace delete operator with undefined assignment.Using the
delete
operator can impact performance. Use undefined assignment instead.Apply this diff:
-// @ts-expect-error - Restore global Image - delete global.Image; +// @ts-expect-error - Restore global Image + global.Image = undefined as any;src/blocks/media-block/ImageBlock.new.tsx (1)
61-70
: Simplify event type handling in toggleZoom.The type narrowing check using
'nativeEvent' in e
is unconventional. Consider using a type guard or accepting the specific event type.-const toggleZoom = React.useCallback((e: React.SyntheticEvent) => { +const toggleZoom = React.useCallback((e: React.MouseEvent<HTMLDivElement> | React.KeyboardEvent) => { e.stopPropagation(); const newIsZoomed = !isZoomedRef.current; setIsZoomed(newIsZoomed); // Call the original onClick if provided - if (onClick && 'nativeEvent' in e) { - onClick(e as React.MouseEvent<HTMLDivElement>); + if (onClick && e.type === 'click') { + onClick(e as React.MouseEvent<HTMLDivElement>); } }, [onClick]);src/blocks/media-block/image-block/SimpleImageBlock.test.tsx.new (1)
200-202
: Avoid @ts-ignore by using proper type assertions.Instead of using
@ts-ignore
, use a proper type assertion.- // @ts-ignore - Mocking global - window.IntersectionObserver = MockIntersectionObserver; + window.IntersectionObserver = MockIntersectionObserver as unknown as typeof IntersectionObserver;.storybook/main.ts (1)
94-98
: Remove empty configuration objects.The empty
alias
object serves no purpose and can be removed to reduce clutter.// Shared Vite config - resolve: { - alias: { - // Add any aliases here - }, - },src/__mocks__/framer-motion.ts (1)
4-21
: Remove async mock methods from synchronous function type.The
MockFunction
type includesmockResolvedValue
andmockResolvedValueOnce
which are meant for async functions, but the base function is synchronous.type MockFunction = { (): void; mockImplementation: (impl: () => void) => void; - mockResolvedValue: (value: any) => MockFunction; - mockResolvedValueOnce: (value: any) => MockFunction; }; const createMockFn = (): MockFunction => { let impl: (() => void) | undefined; const fn = (() => impl?.()) as MockFunction; fn.mockImplementation = (newImpl: () => void) => { impl = newImpl; }; - fn.mockResolvedValue = () => fn; - fn.mockResolvedValueOnce = () => fn; return fn; };src/blocks/media-block/image-block/CleanImageBlock.tsx (2)
64-70
: Use optional chaining for cleaner null checksThe static analysis tool correctly identifies an opportunity to use optional chaining here.
Apply this change to use optional chaining:
return sources .map(src => { - if (src && src.src && isNonEmptyString(src.src)) { + if (src?.src && isNonEmptyString(src.src)) { return src.width ? `${src.src} ${src.width}w` : src.src; } return ''; })
97-540
: Consider splitting this component into smaller, focused componentsAt 540+ lines, this component handles many responsibilities: image loading, zoom functionality, error states, keyboard navigation, and responsive images. Consider extracting some parts into separate components for better maintainability.
Potential extraction opportunities:
- Error state UI (lines 381-426) →
ImageErrorState
component- Zoom overlay (lines 511-537) →
ImageZoomOverlay
component- Image content rendering (lines 281-328) →
ImageContent
componentThis would improve code organization and make testing easier.
src/blocks/data-block/DataGridBlock.tsx (1)
224-225
: Consider using omit utility instead of ESLint disableWhile the current approach works, you could avoid the ESLint disable comment by using a more explicit approach.
- // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { onChange: _onChange, ...cardProps } = props as Omit<DataGridBlockProps, 'onChange'> & Record<string, unknown>; + const cardProps = Object.keys(props).reduce((acc, key) => { + if (key !== 'onChange') { + acc[key] = props[key as keyof typeof props]; + } + return acc; + }, {} as Record<string, unknown>);Alternatively, you could destructure without the variable:
- // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { onChange: _onChange, ...cardProps } = props as Omit<DataGridBlockProps, 'onChange'> & Record<string, unknown>; + const { onChange: _, ...cardProps } = props as Omit<DataGridBlockProps, 'onChange'> & Record<string, unknown>;src/setupTests.ts (3)
46-56
: Consider using thecreateMatchMedia
utility to reduce duplication.The
window.matchMedia
mock andcreateMatchMedia
function both create similarMediaQueryList
objects. You could refactor to reuse the utility function.// 2. Mock window.matchMedia - window.matchMedia = vi.fn().mockImplementation((query: string) => ({ - matches: false, - media: query, - onchange: null, - addListener: vi.fn(), - removeListener: vi.fn(), - addEventListener: vi.fn(), - removeEventListener: vi.fn(), - dispatchEvent: vi.fn(), - })); + window.matchMedia = vi.fn().mockImplementation((query: string) => ({ + ...createMatchMedia(false), + media: query, + }));Also applies to: 128-138
58-70
: Consider enhancing the Image mock with common properties.The current Image mock only handles event listeners. Consider adding common properties for more comprehensive test coverage.
// 3. Mock Image constructor const MockImage = class { + src = ''; + width = 0; + height = 0; + alt = ''; + complete = false; onload: (() => void) | null = null; + onerror: (() => void) | null = null; addEventListener = vi.fn((event: string, callback: () => void) => { if (event === 'load') { this.onload = callback; + } else if (event === 'error') { + this.onerror = callback; } }); removeEventListener = vi.fn(); dispatchEvent = vi.fn(); };
101-104
: Consider adding more motion components for broader test coverage.The current implementation only includes
motion.div
andmotion.img
. Consider adding other commonly used motion components.const motion = { div: createMotionComponent('div', 'motion.div'), img: createMotionComponent('img', 'motion.img'), + span: createMotionComponent('span', 'motion.span'), + button: createMotionComponent('button', 'motion.button'), + a: createMotionComponent('a', 'motion.a'), + section: createMotionComponent('section', 'motion.section'), + ul: createMotionComponent('ul', 'motion.ul'), + li: createMotionComponent('li', 'motion.li'), };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
package-lock.json
is excluded by!**/package-lock.json
public/test-image.jpg
is excluded by!**/*.jpg
src/stories/assets/accessibility.png
is excluded by!**/*.png
src/stories/assets/accessibility.svg
is excluded by!**/*.svg
src/stories/assets/addon-library.png
is excluded by!**/*.png
src/stories/assets/assets.png
is excluded by!**/*.png
src/stories/assets/context.png
is excluded by!**/*.png
src/stories/assets/discord.svg
is excluded by!**/*.svg
src/stories/assets/docs.png
is excluded by!**/*.png
src/stories/assets/figma-plugin.png
is excluded by!**/*.png
src/stories/assets/github.svg
is excluded by!**/*.svg
src/stories/assets/share.png
is excluded by!**/*.png
src/stories/assets/styling.png
is excluded by!**/*.png
src/stories/assets/testing.png
is excluded by!**/*.png
src/stories/assets/theming.png
is excluded by!**/*.png
src/stories/assets/tutorials.svg
is excluded by!**/*.svg
src/stories/assets/youtube.svg
is excluded by!**/*.svg
📒 Files selected for processing (107)
.babelrc
(1 hunks).eslintrc.cjs
(1 hunks).storybook/decorators/withProviders.tsx
(1 hunks).storybook/main.ts
(1 hunks).storybook/preview-head.html
(1 hunks).storybook/preview.ts
(1 hunks).storybook/preview.tsx
(1 hunks).storybook/tsconfig.json
(1 hunks).storybook/types/storybook.d.ts
(1 hunks).storybook/vite.config.ts
(1 hunks)docs/.storybook/main.ts
(1 hunks)docs/.storybook/preview-head.html
(1 hunks)docs/.storybook/preview.ts
(1 hunks)docs/.storybook/preview.tsx
(1 hunks)docs/.storybook/test-imports.ts
(1 hunks)docs/.storybook/tsconfig.json
(1 hunks)docs/stories/blocks/AIChatBlock.stories.tsx
(2 hunks)docs/stories/blocks/FormBlock.stories.tsx
(1 hunks)docs/stories/blocks/IframeBlock.stories.tsx
(2 hunks)docs/stories/blocks/ImageBlock.stories.tsx
(0 hunks)docs/stories/blocks/TableBlock.stories.tsx
(3 hunks)docs/stories/blocks/TextBlock.stories.tsx
(2 hunks)docs/stories/blocks/prompt/PromptContainerFullLineBottomActionsLarge.stories.tsx
(1 hunks)docs/stories/blocks/types.ts
(1 hunks)docs/stories/surfaces/BlockModal.stories.tsx
(4 hunks)docs/stories/surfaces/CommandBar.stories.tsx
(1 hunks)docs/stories/surfaces/ContextPanel.stories.tsx
(3 hunks)docs/stories/surfaces/FormBuilder.stories.tsx
(1 hunks)docs/stories/surfaces/NotificationToast.stories.tsx
(1 hunks)docs/stories/surfaces/ToolPalette.stories.tsx
(2 hunks)docs/stories/surfaces/Widget.stories.tsx
(6 hunks)examples/tsconfig.json
(1 hunks)examples/vite-env.d.ts
(1 hunks)package.json
(4 hunks)postcss.config.cjs
(1 hunks)public/test-image.html
(1 hunks)scripts/update-storybook-imports.ts
(1 hunks)src/App.tsx
(1 hunks)src/BlockKitProvider.tsx
(2 hunks)src/__mocks__/framer-motion.ts
(1 hunks)src/__tests__/types.test.ts
(1 hunks)src/blocks/ai-block/types.ts
(1 hunks)src/blocks/button/Button.tsx
(1 hunks)src/blocks/button/index.ts
(1 hunks)src/blocks/data-block/DataGridBlock.tsx
(9 hunks)src/blocks/icons/Icons.tsx
(1 hunks)src/blocks/media-block/ImageBlock.clean.test.tsx
(1 hunks)src/blocks/media-block/ImageBlock.new.test.tsx
(1 hunks)src/blocks/media-block/ImageBlock.new.tsx
(1 hunks)src/blocks/media-block/ImageBlock.simple.test.tsx
(1 hunks)src/blocks/media-block/ImageBlock.stories.tsx
(1 hunks)src/blocks/media-block/ImageBlock.test.tsx
(1 hunks)src/blocks/media-block/ImageBlock.tsx
(1 hunks)src/blocks/media-block/image-block/CleanImageBlock.test.tsx
(1 hunks)src/blocks/media-block/image-block/CleanImageBlock.tsx
(1 hunks)src/blocks/media-block/image-block/ImageBlock.new.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/ImageBlock.tsx.new
(1 hunks)src/blocks/media-block/image-block/ImageBlockComponent.tsx
(1 hunks)src/blocks/media-block/image-block/SimpleImageBlock.test.tsx
(1 hunks)src/blocks/media-block/image-block/SimpleImageBlock.test.tsx.backup
(1 hunks)src/blocks/media-block/image-block/SimpleImageBlock.test.tsx.new
(1 hunks)src/blocks/media-block/image-block/SimpleImageBlock.tsx
(1 hunks)src/blocks/media-block/image-block/environment.ts
(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/blocks/prompt/PromptContainerFullLineBottomActions.tsx
(1 hunks)src/blocks/prompt/PromptContainerFullLineBottomActionsLarge.tsx
(1 hunks)src/blocks/prompt/__tests__/PromptContainerFullLineBottomActionsLarge.test.tsx
(1 hunks)src/blocks/prompt/index.ts
(1 hunks)src/blocks/prompt/types.ts
(1 hunks)src/components/Button/Button.stories.tsx
(1 hunks)src/components/Button/Button.tsx
(1 hunks)src/components/ScrollArea/ScrollArea.stories.tsx
(1 hunks)src/components/Text/Text.stories.tsx
(1 hunks)src/composition/views/BoardView.tsx
(2 hunks)src/composition/views/MindMapView.tsx
(1 hunks)src/env.d.ts
(1 hunks)src/lib/utils.ts
(1 hunks)src/setupTests.ts
(1 hunks)src/stories/Button.jsx
(1 hunks)src/stories/Button.stories.js
(1 hunks)src/stories/Configure.mdx
(1 hunks)src/stories/Header.jsx
(1 hunks)src/stories/Header.stories.js
(1 hunks)src/stories/Page.jsx
(1 hunks)src/stories/Page.stories.js
(1 hunks)src/stories/button.css
(1 hunks)src/stories/header.css
(1 hunks)src/stories/page.css
(1 hunks)src/surfaces/form/FormBuilder.tsx
(2 hunks)src/surfaces/modal/ModalContainer.tsx
(0 hunks)src/surfaces/notification/NotificationToast.tsx
(2 hunks)src/surfaces/tool-palette/ToolButton.tsx
(1 hunks)src/test-utils.d.ts
(1 hunks)src/tsconfig.json
(1 hunks)src/types.ts
(1 hunks)src/types/global.d.ts
(1 hunks)src/types/hmr.d.ts
(1 hunks)src/types/storybook.d.ts
(1 hunks)src/types/vibing-ai.d.ts
(1 hunks)src/types/vite-env.d.ts
(1 hunks)src/types/vitest.d.ts
(1 hunks)
⛔ Files not processed due to max files limit (17)
- src/utils/blocks/serializeBlock.ts
- src/utils/cn.ts
- src/utils/environment.ts
- src/utils/theme/heroTheme.ts
- src/vite-env.d.ts
- src/vitest-env.d.ts
- src/vitest.d.ts
- tsconfig.build.json
- tsconfig.eslint.json
- tsconfig.json
- tsconfig.node.json
- tsconfig.paths.json
- tsconfig.schema.json
- tsconfig.storybook.json
- tsconfig.test.json
- vite.config.ts
- vitest.config.ts
💤 Files with no reviewable changes (2)
- src/surfaces/modal/ModalContainer.tsx
- docs/stories/blocks/ImageBlock.stories.tsx
🧰 Additional context used
🧬 Code Graph Analysis (13)
src/blocks/icons/Icons.tsx (1)
src/utils/cn.ts (1)
cn
(16-31)
docs/stories/surfaces/ToolPalette.stories.tsx (1)
src/surfaces/tool-palette/ToolPalette.tsx (1)
ToolPalette
(25-119)
docs/stories/surfaces/ContextPanel.stories.tsx (1)
src/blocks/text-block/TextBlock.tsx (1)
TextBlock
(22-48)
docs/stories/blocks/prompt/PromptContainerFullLineBottomActionsLarge.stories.tsx (2)
src/blocks/prompt/PromptContainerFullLineBottomActionsLarge.tsx (1)
PromptContainerFullLineBottomActionsLarge
(19-34)src/blocks/icons/Icons.tsx (3)
Paperclip
(41-57)Smile
(60-79)Send
(21-38)
docs/stories/surfaces/FormBuilder.stories.tsx (1)
src/surfaces/form/FormBuilder.tsx (1)
FormBuilder
(22-148)
src/blocks/prompt/PromptContainerFullLineBottomActionsLarge.tsx (3)
src/blocks/prompt/PromptContainerFullLineBottomActions.tsx (1)
PromptContainerFullLineBottomActions
(27-191)src/blocks/prompt/types.ts (1)
PromptSize
(3-3)src/utils/cn.ts (1)
cn
(16-31)
src/blocks/media-block/image-block/ImageBlock.tsx (3)
src/utils/cn.ts (1)
cn
(16-31)src/blocks/media-block/image-block/types.ts (1)
ImageBlockProps
(29-139)src/blocks/media-block/image-block/index.ts (1)
ImageBlock
(1-1)
docs/stories/blocks/FormBlock.stories.tsx (2)
src/blocks/interactive-block/FormBlock.tsx (1)
FormBlock
(26-78)src/types/storybook.d.ts (1)
Story
(66-66)
src/blocks/media-block/ImageBlock.new.tsx (2)
src/blocks/media-block/types.ts (1)
ImageBlockBaseProps
(4-93)src/blocks/media-block/ImageBlock.new.test.tsx (1)
isZoomed
(127-129)
src/blocks/prompt/types.ts (1)
src/blocks/prompt/PromptContainerFullLineBottomActionsLarge.tsx (1)
PromptActionButton
(39-39)
.storybook/main.ts (1)
vite.config.ts (1)
config
(84-98)
src/blocks/media-block/image-block/CleanImageBlock.tsx (3)
src/blocks/media-block/image-block/types.ts (2)
ImageSource
(12-27)ImageBlockProps
(29-139)src/types/vibing-ai.d.ts (1)
CleanImageBlock
(5-5)src/blocks/media-block/ImageBlock.new.test.tsx (1)
isZoomed
(127-129)
src/blocks/data-block/DataGridBlock.tsx (2)
src/components/Text/Text.tsx (1)
Text
(60-117)src/components/Button/Button.tsx (1)
Button
(80-80)
🪛 HTMLHint (1.5.0)
docs/.storybook/preview-head.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
.storybook/preview-head.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🪛 GitHub Check: Codacy Static Code Analysis
src/test-utils.d.ts
[failure] 3-3: src/test-utils.d.ts#L3
'Assertion' is defined but never used.
src/stories/Header.jsx
[failure] 1-1: src/stories/Header.jsx#L1
'React' is defined but never used.
src/types/hmr.d.ts
[warning] 3-3: src/types/hmr.d.ts#L3
Unexpected any. Specify a different type.
[warning] 4-4: src/types/hmr.d.ts#L4
Unexpected any. Specify a different type.
src/types/vibing-ai.d.ts
[warning] 5-5: src/types/vibing-ai.d.ts#L5
Unexpected any. Specify a different type.
[warning] 6-6: src/types/vibing-ai.d.ts#L6
Unexpected any. Specify a different type.
[warning] 11-11: src/types/vibing-ai.d.ts#L11
Unexpected any. Specify a different type.
src/blocks/media-block/ImageBlock.test.tsx
[warning] 11-11: src/blocks/media-block/ImageBlock.test.tsx#L11
Unexpected any. Specify a different type.
[notice] 12-12: src/blocks/media-block/ImageBlock.test.tsx#L12
img elements must have an alt prop, either with meaningful text, or an empty string for decorative images.
docs/stories/surfaces/FormBuilder.stories.tsx
[warning] 67-67: docs/stories/surfaces/FormBuilder.stories.tsx#L67
Unexpected any. Specify a different type.
[warning] 69-69: docs/stories/surfaces/FormBuilder.stories.tsx#L69
Unexpected any. Specify a different type.
src/blocks/media-block/image-block/ImageBlock.tsx
[warning] 60-60: src/blocks/media-block/image-block/ImageBlock.tsx#L60
Method (anonymous) has a cyclomatic complexity of 13 (limit is 8)
src/blocks/media-block/image-block/ImageBlock.new.tsx
[warning] 56-56: src/blocks/media-block/image-block/ImageBlock.new.tsx#L56
Method (anonymous) has a cyclomatic complexity of 9 (limit is 8)
src/blocks/media-block/image-block/SimpleImageBlock.test.tsx
[warning] 135-135: src/blocks/media-block/image-block/SimpleImageBlock.test.tsx#L135
Method mockImplementation has a cyclomatic complexity of 9 (limit is 8)
src/stories/Button.jsx
[failure] 1-1: src/stories/Button.jsx#L1
'React' is defined but never used.
src/blocks/media-block/image-block/CleanImageBlock.test.tsx
[warning] 11-11: src/blocks/media-block/image-block/CleanImageBlock.test.tsx#L11
Unexpected any. Specify a different type.
[failure] 105-105: src/blocks/media-block/image-block/CleanImageBlock.test.tsx#L105
'C' is defined but never used.
src/components/Button/Button.tsx
[failure] 45-45: src/components/Button/Button.tsx#L45
'_size' is assigned a value but never used.
src/types/storybook.d.ts
[failure] 1-1: src/types/storybook.d.ts#L1
'StoryFn' is defined but never used.
[warning] 21-21: src/types/storybook.d.ts#L21
Unexpected any. Specify a different type.
[warning] 51-51: src/types/storybook.d.ts#L51
Unexpected any. Specify a different type.
[failure] 55-55: src/types/storybook.d.ts#L55
'TArgs' is defined but never used.
[warning] 85-85: src/types/storybook.d.ts#L85
Unexpected any. Specify a different type.
src/blocks/media-block/image-block/CleanImageBlock.tsx
[warning] 203-203: src/blocks/media-block/image-block/CleanImageBlock.tsx#L203
Method (anonymous) has a cyclomatic complexity of 9 (limit is 8)
🪛 Biome (1.9.4)
src/types/global.d.ts
[error] 31-32: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
src/blocks/media-block/ImageBlock.test.tsx
[error] 62-63: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
[error] 62-63: Do not export from a test file.
(lint/suspicious/noExportsInTest)
[error] 12-12: img is a void element tag and must not have children.
Unsafe fix: Remove the children.
(lint/correctness/noVoidElementsWithChildren)
src/blocks/media-block/image-block/SimpleImageBlock.test.tsx
[error] 52-52: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 135-135: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
[error] 208-208: void is confusing inside a union type.
Unsafe fix: Use undefined instead.
(lint/suspicious/noConfusingVoidType)
src/blocks/media-block/ImageBlock.new.test.tsx
[error] 388-388: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 158-173: Do not export from a test file.
(lint/suspicious/noExportsInTest)
[error] 65-65: Shouldn't redeclare 'VITEST'. Consider to delete it or rename it.
'VITEST' is defined here:
(lint/suspicious/noRedeclare)
[error] 318-325: img is a void element tag and must not have children.
Unsafe fix: Remove the children.
(lint/correctness/noVoidElementsWithChildren)
src/blocks/media-block/image-block/CleanImageBlock.tsx
[error] 65-65: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (131)
postcss.config.cjs (1)
1-6
: Config looks good – no functional changes introducedOnly whitespace/formatting was adjusted; nothing to flag.
src/stories/header.css (1)
1-33
: Stylesheet reads well – matches Storybook header requirementsNo issues spotted. Naming, specificity, and flex layout are clear and minimal.
src/composition/views/BoardView.tsx (2)
28-30
: Good improvement in code clarity.The renaming of
onItemResize
to_onItemResize
with an explanatory comment is cleaner than the previous ESLint disable approach. This clearly communicates the intent while maintaining the component's API contract.
12-15
: ```shell
#!/bin/bashSearch for BoardView imports and usages to verify required props are provided
echo "=== Imports ==="
rg "import.*BoardView" -n .
echo "=== Usages ==="
rg "<BoardView\b" -n .</details> <details> <summary>src/surfaces/tool-palette/ToolButton.tsx (1)</summary> `24-32`: **Clean approach to handling unused variables.** The restructured destructuring with `id: _id` is a much cleaner way to handle unused variables compared to ESLint disable comments. This explicitly communicates that the `id` property is intentionally unused while maintaining access to all tool properties. </details> <details> <summary>docs/.storybook/preview-head.html (1)</summary> `1-14`: **Global styles look good, DOCTYPE not required for injection files.** The global CSS styles are well-structured with a sensible font stack and consistent spacing. The HTMLHint warning about missing DOCTYPE can be ignored since this is a Storybook preview head file that gets injected into the main HTML document, not a standalone HTML file. </details> <details> <summary>src/blocks/media-block/image-block/environment.ts (1)</summary> `1-1`: **Clean re-export for module organization.** The re-export statement follows good module organization practices, making environment utilities easily accessible to components within the image-block directory while maintaining a clear dependency structure. </details> <details> <summary>docs/stories/blocks/IframeBlock.stories.tsx (1)</summary> `13-13`: **Verify property name consistency with the IframeBlock component.** The change from `allowFullScreen` to `allowFullscreen` standardizes the property casing. Please ensure this matches the actual property name expected by the `IframeBlock` component. ```shell #!/bin/bash # Description: Verify the actual property name used by the IframeBlock component # Expected: Find the correct property name in the component definition # Search for IframeBlock component definition and its props ast-grep --pattern $'interface $_ { $$$ allowFullscreen$_: $_; $$$ }' ast-grep --pattern $'interface $_ { $$$ allowFullScreen$_: $_; $$$ }' # Also search for the component implementation rg -A 10 -B 5 "allowFull" --type tsx --type ts
Also applies to: 37-37
docs/stories/blocks/TableBlock.stories.tsx (1)
24-26
: Verify column accessor property aligns with TableBlock component API.The change from
accessor
toaccessorKey
suggests an API update, possibly to align with TanStack Table v8+ which usesaccessorKey
. Please ensure this matches the expected column definition format in theTableBlock
component.#!/bin/bash # Description: Verify the correct column property name used by TableBlock # Expected: Find the column interface definition # Search for TableBlock component and column definitions ast-grep --pattern $'interface $_ { $$$ accessorKey: $_; $$$ }' ast-grep --pattern $'interface $_ { $$$ accessor: $_; $$$ }' # Search for TableBlock component usage rg -A 5 -B 5 "TableBlock" --type tsx --type tsAlso applies to: 40-42, 56-58
src/tsconfig.json (1)
1-12
: Well-structured TypeScript configuration.The tsconfig.json follows TypeScript best practices with proper path mapping configuration. The setup enables clean module imports using the defined aliases and correctly extends the parent configuration.
docs/.storybook/main.ts (1)
20-23
: Excellent addition of src directory stories.Adding the src directory to the stories array enables co-location of stories with components, which is a Storybook best practice. This allows for better organization and easier maintenance of component documentation.
src/blocks/media-block/image-block/index.ts (1)
1-2
: Clean barrel export implementation.The index file provides a well-structured public API for the image-block module using the barrel export pattern. This enables clean imports from consumers.
#!/bin/bash # Description: Verify that the exported files exist # Expected: Both ImageBlock.tsx and types.ts should exist in the same directory # Check if the referenced files exist fd -t f "ImageBlock\.(tsx?|jsx?)" src/blocks/media-block/image-block/ fd -t f "types\.ts" src/blocks/media-block/image-block/src/lib/utils.ts (1)
4-6
: LGTM! Clean implementation of a standard Tailwind utility function.The
cn
function follows the common pattern for combining conditional classes with Tailwind CSS conflict resolution. The implementation is concise, well-typed, and will be useful across the project for consistent class name handling..eslintrc.cjs (2)
26-26
: Verify the dedicated ESLint TypeScript configuration file exists.The project reference now points to
./tsconfig.eslint.json
. Ensure this file exists and is properly configured for ESLint's TypeScript integration.#!/bin/bash # Check if the referenced TypeScript config file exists if [ -f "tsconfig.eslint.json" ]; then echo "✓ tsconfig.eslint.json exists" cat tsconfig.eslint.json else echo "✗ tsconfig.eslint.json not found" fi
21-22
: ```shell
#!/bin/bash1. Verify that the dedicated ESLint tsconfig file exists
echo "Checking for tsconfig.eslint.json:"
if [ -f tsconfig.eslint.json ]; then
echo "→ tsconfig.eslint.json found"
else
echo "→ ERROR: tsconfig.eslint.json not found"
fi2. Search for optional chaining (?.) and nullish coalescing (??) in source files
echo ""
echo "Searching for modern JS syntax (?. and ??):"
rg -e '??' -e '?.' --glob '.ts' --glob '.tsx' -A2 || echo "No matches found"</details> <details> <summary>src/blocks/button/index.ts (1)</summary> `1-2`: **Clean barrel export pattern, but check for redundancy.** The index file follows good practices for module organization. However, if the types `ButtonProps`, `ButtonVariant`, and `ButtonSize` are already exported from `./Button`, the explicit type re-export on line 2 might be redundant. Verify the Button module exports and check for redundancy: ```shell #!/bin/bash # Check what's exported from the Button module if [ -f "src/blocks/button/Button.tsx" ] || [ -f "src/blocks/button/Button.ts" ]; then echo "Button module found. Checking exports..." rg -A 10 "export.*Button" src/blocks/button/Button.* else echo "Button module not found" fi
docs/.storybook/tsconfig.json (1)
3-9
: Well-configured TypeScript options for Storybook.The compiler options are appropriate for a Storybook environment:
jsx: "react-jsx"
enables the modern JSX transform- Strict mode and interop settings ensure type safety
skipLibCheck
helps with Storybook's dependency complexitysrc/blocks/prompt/index.ts (1)
1-3
: Clean barrel export pattern for prompt components.The index file properly consolidates exports from related prompt modules, following the same pattern established in the button block. This creates a convenient single entry point for prompt-related components and types.
Verify that all referenced modules exist:
#!/bin/bash # Check if all referenced prompt modules exist modules=("PromptContainerFullLineBottomActions" "PromptContainerFullLineBottomActionsLarge" "types") for module in "${modules[@]}"; do if [ -f "src/blocks/prompt/${module}.tsx" ] || [ -f "src/blocks/prompt/${module}.ts" ]; then echo "✓ ${module} module exists" else echo "✗ ${module} module not found" fi donesrc/blocks/ai-block/types.ts (1)
1-6
: Well-structured ChatMessage interface.The interface definition follows TypeScript best practices with appropriate type constraints for the role field and optional timestamp. This provides good type safety for chat message handling.
.storybook/preview-head.html (1)
1-30
: Appropriate base styles for Storybook preview.The CSS provides sensible defaults including font stack, typography, and layout styles. The doctype warning from static analysis can be safely ignored as this is a partial HTML file injected into Storybook's preview iframe.
src/env.d.ts (1)
1-10
: Correct Vite environment variable typing.The type declarations properly extend Vite's ImportMeta interfaces to provide type-safe access to custom environment variables. The readonly modifiers and triple-slash directive are appropriate for this use case.
src/stories/Header.stories.js (1)
1-22
: Well-structured Storybook story following best practices.The story file correctly implements Storybook 7+ conventions with proper metadata, autodocs integration, and meaningful story variants that cover the key use cases for the Header component.
src/surfaces/notification/NotificationToast.tsx (2)
22-24
: Good refactoring to clarify unused prop.Renaming
id
to_id
with a clear comment is a cleaner approach than ESLint disable comments, while maintaining API compatibility and indicating the prop is intentionally unused internally.
45-47
: Improved useEffect cleanup consistency.Explicitly returning an empty cleanup function when duration is non-positive ensures consistent effect return types and follows React best practices for effect cleanup.
src/types/hmr.d.ts (1)
1-2
: LGTM! Good documentation and purpose.The file correctly addresses TypeScript recognition of HMR globals and includes clear documentation about its purpose.
src/types/vitest.d.ts (1)
1-9
: LGTM! Comprehensive Vitest type declarations.The module declarations correctly ensure that testing utilities (
describe
,it
,expect
,vi
) are properly typed and available from both 'vitest' and '@vitest/globals' import sources. This provides excellent type safety and consistency across the testing infrastructure.examples/vite-env.d.ts (1)
1-10
: LGTM! Proper Vite environment variable typing.The file correctly implements the standard Vite pattern for environment variable type safety. The
ImportMetaEnv
interface definition andImportMeta
extension provide proper TypeScript support forimport.meta.env
access, and the comment indicates good extensibility for future environment variables.docs/.storybook/preview.ts (1)
3-3
: LGTM! React import added for JSX support.The React import is correctly added, likely needed for JSX usage in the Storybook configuration.
docs/stories/blocks/TextBlock.stories.tsx (2)
9-14
: LGTM! Story configuration updated to match component API.The argTypes correctly reflect the updated component interface:
content
→text
prop renamevariant
→type
prop rename with expanded heading options- Addition of
className
control for styling flexibilityThe expanded type options (paragraph + heading1-heading6) provide comprehensive typography control.
21-40
: Story examples properly updated.All story examples correctly use the updated prop names (
text
instead ofcontent
,type
instead ofvariant
) and appropriate type values. The examples demonstrate different use cases effectively.src/App.tsx (2)
1-1
: LGTM: Clean import structureThe import statement is properly structured and follows React conventions.
14-14
: Verify local image path availabilityThe component references local images (
/images/hero.jpg
,/images/hero-small.jpg
, etc.) that may not exist in the public directory.#!/bin/bash # Check if the referenced image files exist in the public directory echo "Checking for referenced image files..." fd -t f "hero" public/ 2>/dev/null || echo "No hero images found in public directory" ls -la public/images/ 2>/dev/null || echo "No public/images directory found"scripts/update-storybook-imports.ts (2)
7-10
: LGTM: Comprehensive file discovery with proper exclusionsThe glob pattern correctly targets TypeScript story files while excluding common build directories. The use of absolute paths is appropriate for file operations.
27-31
: LGTM: Proper error handling with appropriate exit codesThe error handling correctly logs errors and exits with a non-zero code, which is essential for CI/CD pipelines.
src/stories/button.css (1)
1-9
: LGTM: Well-structured base button classThe base button styles are well-organized with sensible defaults. The font stack and cursor pointer are appropriate choices.
docs/stories/surfaces/ContextPanel.stories.tsx (3)
15-15
: LGTM: Improved type flexibility with React.ReactNodeChanging from
string
toReact.ReactNode
provides better flexibility for rich content while maintaining type safety.
39-44
: LGTM: Consistent JSX usage in Basic storyThe update to use JSX with the TextBlock component is well-structured and follows React best practices.
85-90
: LGTM: Consistent TextBlock integrationThe conversion to use TextBlock components with proper props is well-executed and maintains the story's functionality while improving composability.
Also applies to: 96-101
examples/tsconfig.json (3)
12-12
: LGTM: Appropriate type inclusions for the environmentAdding
"node"
and"vite/client"
types is appropriate for a Vite-based project that may use Node.js APIs.
15-15
: LGTM: Proper exclusion of build artifactsExcluding
node_modules
anddist
directories is a best practice to avoid unnecessary type checking of generated files.
16-19
: LGTM: Well-structured project references for monorepoThe project references to both
tsconfig.node.json
andtsconfig.json
indicate a proper monorepo setup with appropriate dependency management.docs/stories/blocks/AIChatBlock.stories.tsx (3)
3-3
: Good addition of UUID generation for dynamic story data.Adding the
uuid
import enables dynamic ID generation, making the stories more realistic and closer to production behavior.
21-26
: Excellent improvements to message object structure.The changes enhance the story data quality by:
- Using dynamic UUIDs instead of static IDs
- Adding realistic timestamps
- Proper TypeScript typing with
as const
for role propertiesThis makes the stories more representative of actual runtime behavior.
Also applies to: 28-32, 34-38
46-51
: Consistent application of improved message structure.The same improvements are consistently applied across all stories (Loading, WithAvatars), maintaining uniformity in the story definitions.
Also applies to: 60-65, 67-71, 73-77
src/types/vibing-ai.d.ts (1)
10-13
: Wildcard module pattern provides necessary flexibility.The wildcard pattern
@vibing-ai/block-kit/*
is appropriate for allowing flexible imports of submodules or components, though it also usesany
typing for maximum compatibility.docs/stories/blocks/types.ts (1)
1-11
: Well-structured story props interface.The
ImageBlockStoryProps
interface demonstrates good TypeScript practices:
- Proper use of
Omit
utility type to exclude specific properties- Clean redefinition of callback signatures for story context
- Addition of story-specific styling properties
- Clear import structure with proper type imports
This provides a clean API for Storybook stories while maintaining type safety.
src/stories/Header.jsx (3)
7-54
: Excellent component structure and accessibility.The Header component demonstrates good practices:
- Clean conditional rendering logic
- Proper ARIA labels for accessibility (
aria-label
,role="group"
)- Semantic button usage with clear labels
- Well-organized JSX structure
The authentication state handling is intuitive and user-friendly.
56-67
: Good use of PropTypes for runtime type checking.The PropTypes definitions provide clear prop validation:
- Proper shape validation for user object
- Required function props clearly marked
- Sensible default props
This is especially valuable in a .jsx file for runtime type safety.
1-1
: Verify if React import is necessary.Static analysis indicates the React import may be unused. With modern React (17+) and automatic JSX transform, explicit React imports are often unnecessary in JSX files.
#!/bin/bash # Check if automatic JSX transform is configured echo "Checking for JSX transform configuration..." fd -t f -e json -e js -e ts . --exec grep -l "jsx.*automatic\|react.*automatic" {} \; # Check React version in package.json echo "Checking React version..." cat package.json | jq '.dependencies.react // .devDependencies.react // "not found"'src/stories/Page.stories.js (2)
1-12
: Well-configured Storybook story setup.The story configuration properly imports testing utilities and sets up the component with appropriate parameters. The fullscreen layout is suitable for a Page component.
17-28
: Excellent interaction testing implementation.The
LoggedIn
story'splay
function demonstrates best practices for Storybook interaction testing:
- Proper use of
within()
to scope queries to the canvas- Clear test flow: check initial state → simulate user action → verify result
- Appropriate async/await usage for user interactions
- Good assertions that verify both positive and negative conditions
This provides valuable automated testing of the login/logout flow.
src/blocks/media-block/image-block/ImageBlock.test.tsx (1)
1-29
: To ensure we capture any framer-motion usage or mocks across all file types, let’s rerun the search without type filters:#!/bin/bash # Search for any imports or mocks referencing framer-motion rg "framer-motion" -nsrc/blocks/media-block/image-block/tsconfig.json (1)
1-30
: Well-structured TypeScript configuration for the sub-module.The configuration properly extends the parent tsconfig, uses modern React JSX transform, and includes appropriate project references. The composite build setup with dedicated tsBuildInfo file will help with incremental compilation performance.
src/BlockKitProvider.tsx (2)
5-9
: Excellent type safety improvement for theme objects.The introduction of
ThemeValueMap
interface provides better type safety for nested theme structures while maintaining flexibility. The recursive type definition properly handles complex theme hierarchies.
51-59
: Good practice: Always return cleanup function in useEffect.The improvement to always return a cleanup function, even when it's empty, is a good practice that makes the effect more predictable and follows React conventions consistently.
.storybook/decorators/withProviders.tsx (2)
60-93
: Comprehensive global styles setup.The global styles provide a solid foundation with proper CSS reset and form element normalization. The use of theme variables in the styles creates good consistency.
111-117
: Clean decorator implementation.The decorator properly wraps stories with the theme provider and applies global styles consistently across all Storybook stories.
src/stories/Button.stories.js (3)
4-17
: LGTM! Well-structured Storybook configuration.The default export follows Storybook v7+ best practices with proper metadata, layout configuration, autodocs generation, and argTypes for interactive controls.
20-45
: Story variants are well-defined.The Primary, Secondary, Large, and Small story variants provide good coverage of the Button component's different states and props.
1-1
: Verify the Button import path.The import references
'./Button'
but based on the files provided, there's a Button component atsrc/blocks/button/Button.tsx
. Ensure this import path is correct or update it to reference the appropriate Button component.#!/bin/bash # Check if there's a Button component in the same directory as the stories file fd -t f "Button\.(js|jsx|ts|tsx)$" src/stories/ # Also check for any Button exports in the stories directory rg -A 3 "export.*Button" src/stories/src/types.ts (1)
7-12
: Excellent type refactoring to avoid circular references.The new JSON-like type system is well-structured and provides better type safety than a direct recursive Record. The intermediate types (Primitive, JsonArray, JsonObject, JsonValue) clearly model the data structure hierarchy.
src/blocks/button/Button.tsx (3)
32-84
: Comprehensive and well-typed Button interface.The ButtonProps interface provides excellent TypeScript support with detailed JSDoc comments, sensible defaults, and good separation of concerns between styling and behavior props.
102-178
: Excellent Button component implementation.The component demonstrates several best practices:
- Proper use of React.forwardRef
- Good accessibility with aria-busy attribute
- Conditional logic for loading and disabled states
- Clean separation of layout logic
- Consistent Tailwind CSS class structure
11-27
: Fix type ordering issue.The
getButtonLayoutClasses
function referencesButtonSize
type before it's defined on line 30. This will cause a TypeScript compilation error.Move the type definitions before their usage:
+export type ButtonVariant = keyof typeof buttonVariants; +export type ButtonSize = keyof typeof buttonSizes; + +const buttonVariants = { + default: 'bg-gray-100 text-gray-900 hover:bg-gray-200', + primary: 'bg-blue-600 text-white hover:bg-blue-700', + secondary: 'bg-gray-200 text-gray-900 hover:bg-gray-300', + ghost: 'bg-transparent hover:bg-gray-100', + link: 'bg-transparent text-blue-600 hover:underline', + outline: 'border border-gray-300 bg-transparent hover:bg-gray-50', + destructive: 'bg-red-600 text-white hover:bg-red-700', +} as const; + +const buttonSizes = { + sm: 'h-8 text-sm px-3', + md: 'h-10 px-4 py-2', + lg: 'h-12 px-6 text-lg', +} as const; + const getButtonLayoutClasses = ( size: ButtonSize, isIconButton: boolean, fullWidth: boolean ): { sizeClass: string; widthClass: string } => {Likely an incorrect or invalid review comment.
.storybook/tsconfig.json (1)
1-35
: Well-configured TypeScript setup for Storybook.The configuration properly extends the base tsconfig.json and includes appropriate compiler options for the Storybook environment. The include patterns correctly target story files across multiple directories, and the type resolution setup looks comprehensive.
src/blocks/icons/Icons.tsx (2)
21-174
: Excellent icon component implementation.The icon components follow consistent patterns with:
- Proper SVG structure and viewBox configuration
- Consistent styling attributes (stroke, strokeWidth, etc.)
- Good use of the createIcon HOC for ref forwarding and styling
- Comprehensive collection of commonly used icons
166-176
: Good export strategy for icon collection.Providing both individual named exports and a collective Icons object gives consumers flexibility in how they import and use the icons.
src/blocks/media-block/ImageBlock.clean.test.tsx (1)
51-129
: Excellent test coverage for ImageBlock functionality.The test suite comprehensively covers all key scenarios including rendering, event handling, prop validation, and conditional rendering. The tests are well-structured and use appropriate assertions.
src/blocks/prompt/__tests__/PromptContainerFullLineBottomActionsLarge.test.tsx (1)
7-153
: Comprehensive and well-structured test suite.The test coverage is excellent, validating all key component behaviors including rendering, user interactions, prop variations, and state management. The test structure follows best practices with proper setup, clear assertions, and good use of testing utilities.
docs/stories/blocks/prompt/PromptContainerFullLineBottomActionsLarge.stories.tsx (3)
7-60
: Well-structured Storybook configuration with comprehensive controls.The meta configuration provides excellent documentation and interactive controls for the component. The argTypes are properly defined with descriptions and default values.
66-88
: Good use of state management in Default story.The story properly manages local state and forwards events to Storybook actions, providing both interactive functionality and action logging.
91-135
: Effective demonstration of size variants.The SizeVariants story clearly shows the different size options with proper labeling and consistent styling.
src/surfaces/form/FormBuilder.tsx (2)
5-6
: Improved type definition consistency.Changing
FormValue
to useundefined
instead ofnull
and makingFormValues
internal are good improvements for type consistency and encapsulation.
101-107
: Enhanced value rendering logic with better edge case handling.The updated logic properly handles:
null
andundefined
values uniformly with== null
check- Array values by joining them into readable strings
- Preserves truthy but falsy values like
0
andfalse
This prevents incorrect rendering of valid values as empty strings.
.babelrc (1)
1-43
: Comprehensive and well-configured Babel setup.The configuration properly supports:
- Modern React with automatic JSX runtime and Emotion integration
- TypeScript with full TSX and namespace support
- Environment-specific optimizations for testing and development
- Proper polyfill handling with core-js integration
This setup aligns well with modern React/TypeScript development practices and the project's Emotion-based styling approach.
.storybook/vite.config.ts (1)
31-33
: Verify the alias path exists and is correct.The alias
@vibing-ai/block-kit
points to../src/index.ts
. Ensure this file exists and correctly exports the intended modules, as this alias will be used throughout the Storybook environment.#!/bin/bash # Description: Verify the alias target file exists and check its exports # Expected: The file should exist and contain proper exports # Check if the target file exists if [ -f "src/index.ts" ]; then echo "✓ src/index.ts exists" echo "File contents:" cat src/index.ts else echo "✗ src/index.ts does not exist" fisrc/stories/page.css (1)
1-69
: Well-structured CSS with good practices.The stylesheet follows CSS best practices with consistent spacing, semantic class names, and a cohesive design system. The typography hierarchy and color scheme are well-defined.
src/blocks/prompt/PromptContainerFullLineBottomActionsLarge.tsx (1)
19-36
: Well-implemented component variant with proper React patterns.The component correctly extends the base component, uses proper TypeScript typing, implements ref forwarding, and applies consistent styling patterns. The default size prop and className merging logic are handled appropriately.
.storybook/preview.tsx (1)
50-91
: LGTM! Well-structured Storybook configuration.The preview configuration is comprehensive and includes appropriate parameters for actions, controls, accessibility, and story organization.
docs/.storybook/preview.tsx (2)
13-61
: LGTM! Excellent error boundary implementation.The ErrorBoundary component is well-implemented with proper error handling, styled error display, and correctly typed decorator.
63-83
: LGTM! Clean preview configuration with proper theming.The preview configuration properly integrates error handling and theme switching with appropriate TypeScript typing.
src/components/Button/Button.stories.tsx (2)
5-33
: LGTM! Excellent Storybook configuration.The meta configuration is comprehensive with proper TypeScript typing, good default args, and detailed argTypes controls for all button variants and states.
39-105
: LGTM! Comprehensive story coverage.The stories provide excellent coverage of all button variants, sizes, and states, making it easy to test and visualize different configurations.
src/components/ScrollArea/ScrollArea.stories.tsx (1)
1-100
: LGTM! Well-structured Storybook storiesThe ScrollArea stories are properly implemented with:
- Type-safe meta configuration using
typeof ScrollArea
- Comprehensive argTypes with descriptions
- Multiple story variants showcasing different use cases
- Clean and readable code structure
src/blocks/media-block/ImageBlock.simple.test.tsx (1)
1-262
: LGTM! Comprehensive test coverage with well-structured mocksExcellent test implementation with:
- Detailed framer-motion mocks that properly simulate motion behavior
- Comprehensive test coverage including rendering, interactions, and accessibility
- Proper cleanup and mock resetting
- Good use of data-testid for reliable element selection
src/blocks/media-block/image-block/CleanImageBlock.test.tsx (2)
101-109
: Type declaration is correct - static analysis false positiveThe static analysis warning about unused 'C' is a false positive. This is the standard syntax for extending TypeScript namespace types where 'C' is a generic parameter in the JestAssertion interface definition.
128-169
: Well-structured tests with proper async handlingThe test suite demonstrates excellent practices:
- Proper async/await handling for promise-based operations
- Comprehensive assertions on event properties
- Good coverage of both success and error scenarios
- Clear test descriptions
src/stories/Button.jsx (3)
1-1
: React import is required for JSX - ignore static analysis warning.The static analysis tool incorrectly flags React as unused. React must be imported when using JSX syntax, even though it's not explicitly referenced in the code. This is a false positive.
8-20
: Well-structured component implementation.The Button component follows React best practices with proper destructuring, conditional styling, and prop spreading. The implementation is clean and appropriate for a Storybook demo component.
22-50
: Comprehensive PropTypes and defaultProps.Good use of PropTypes for runtime type checking and clear documentation. The defaultProps provide sensible fallbacks and the PropTypes comments are helpful for component documentation.
package.json (4)
4-4
: Consider implications of adding "type": "module".Adding
"type": "module"
makes this package an ES module, which affects how it's imported and may cause compatibility issues with CommonJS consumers. Ensure this change aligns with your build setup and consumer expectations.
16-17
: Good test script separation.Splitting test scripts into
test
(watch mode) andtest:ci
(run mode) is a best practice for different environments.
50-80
: Storybook v7.6.x dependency management.The extensive Storybook v7.6.x dependencies look comprehensive. Note that this uses an older version rather than the latest v8.x - ensure this version choice is intentional and meets your requirements.
116-117
: I’d like to confirm where React/React-DOM are declared in package.json. Let’s dump the dependency sections:#!/bin/bash set -e echo "=== peerDependencies ===" grep -n '"peerDependencies"' package.json -A5 || echo "(none)" echo -e "\n=== dependencies ===" grep -n '"dependencies"' package.json -A15 echo -e "\n=== devDependencies ===" grep -n '"devDependencies"' package.json -A15public/test-image.html (2)
146-181
: Well-implemented React component with proper error handling.The ImageBlock component demonstrates good React patterns with hooks for state management and comprehensive error handling. The error details display is helpful for debugging.
116-141
: Clean vanilla JavaScript implementation.The vanilla JS image loading function is well-structured with proper error handling and DOM manipulation. Good comparison test for the React component.
.storybook/preview.ts (4)
17-46
: Comprehensive viewport configuration.Good set of responsive viewport options covering mobile through large desktop sizes. The dimensions are practical and commonly used breakpoints.
104-121
: Excellent accessibility configuration.The a11y addon configuration properly enables color-contrast checking while disabling problematic rules like landmark-one-main. The scroll handling options are thoughtful additions.
128-143
: Proper decorator setup with theme switching.The decorator configuration correctly applies the withProviders decorator and theme switching. The commented test decorator setup shows good planning for future test integration.
146-195
: Well-organized argTypes for better Storybook UX.The argTypes configuration properly categorizes common props and events, making the Storybook controls more user-friendly. Disabling controls for children is a good practice.
.storybook/types/storybook.d.ts (3)
5-12
: Convenient type re-exports for better DX.Re-exporting common Storybook types provides a better developer experience by consolidating imports. This is a good practice for internal type management.
24-28
: Appropriate global Window extension for Storybook.Extending the Window interface to include
__STORYBOOK_ADDONS_CHANNEL__
is the correct approach for Storybook addon communication. The unknown type is appropriately cautious.
31-33
: Proper global constant declarations.The global constant declarations for withThemeProvider and docs components align with typical Storybook usage patterns and provide proper typing.
src/blocks/media-block/image-block/ImageBlockComponent.tsx (3)
6-14
: LGTM! Robust type guard implementation.The
isImageSource
function provides proper type checking with null safety and correct type narrowing.
16-24
: LGTM! Comprehensive source string extraction.The
getSourceString
function handles all the different source formats safely and returns a fallback empty string when needed.
77-172
: LGTM! Excellent accessibility and user experience implementation.The component implementation is comprehensive and well-thought-out:
- Proper keyboard navigation (Enter/Space to toggle zoom, Escape to close)
- Correct ARIA labels and roles for accessibility
- State management for zoom and loading states
- Error handling with extended error information
- Loading placeholder with pulse animation
- Proper event propagation control
The code demonstrates good React practices with proper ref forwarding and imperative handle usage.
src/types/vite-env.d.ts (1)
1-62
: LGTM! Comprehensive and well-structured Vite type declarations.This file provides excellent TypeScript support for:
- Asset imports (SVG with React component support, images, etc.)
- CSS modules with proper readonly typing
- Environment variables with type safety
The SVG declaration correctly handles both default string export and ReactComponent named export, which is the standard pattern for SVG handling in React projects.
docs/stories/blocks/FormBlock.stories.tsx (3)
19-40
: LGTM! Much improved story structure with proper field definitions.The structured
fields
array approach is significantly better than the previous simple children string. This properly demonstrates the FormBlock component's capabilities and aligns with the component's API as seen insrc/blocks/interactive-block/FormBlock.tsx
.
46-60
: LGTM! Good demonstration of textarea field type and custom labels.The story properly showcases the textarea field type and custom button labels, providing comprehensive documentation for the FormBlock component.
62-82
: LGTM! Excellent demonstration of select field and conditional cancel button.The story effectively shows:
- Select field with options array
- Setting
cancelLabel
toundefined
to hide the cancel button- Proper field configuration for different input types
This provides valuable documentation for implementers.
docs/stories/surfaces/BlockModal.stories.tsx (3)
15-37
: LGTM! Good type safety improvement with proper Storybook controls.The local
BlockModalProps
interface provides better type safety, and the expandedargTypes
gives users better interactive controls in Storybook.
48-54
: LGTM! JSX syntax is much more readable than React.createElement.The conversion from
React.createElement
to JSX syntax significantly improves readability and maintainability.
64-103
: LGTM! Excellent structured FormBlock usage with comprehensive field definitions.The FormBlock integration shows proper usage with:
- Detailed field configurations for different input types
- Proper event handling setup
- Good demonstration of the component's capabilities
This serves as excellent documentation for implementers.
src/blocks/media-block/image-block/ImageBlock.tsx (1)
142-183
: LGTM! Excellent implementation of LQIP and image loading features.The component provides comprehensive image handling with:
- LQIP (Low Quality Image Placeholder) with blur effect
- Smooth opacity transitions on load
- Proper caption positioning and styling
- Good accessibility attributes
The implementation demonstrates advanced image optimization techniques.
src/blocks/media-block/types.ts (1)
1-93
: Excellent type definitions with comprehensive coverage.The
ImageBlockBaseProps
interface is well-designed with proper JSDoc documentation, appropriate optional/required prop distinctions, and comprehensive coverage of image block functionality including accessibility, styling, and interaction features.src/stories/Configure.mdx (2)
18-33
: Clean SVG component implementation.The
RightArrow
component is well-implemented with appropriate styling and accessibility considerations.
35-368
: Comprehensive Storybook documentation.The MDX documentation provides excellent coverage of Storybook configuration topics with good visual hierarchy and responsive design.
src/blocks/media-block/image-block/ImageBlock.tsx.new (1)
314-344
: Excellent accessibility implementation for zoom overlay.The zoom overlay properly implements ARIA attributes and keyboard navigation, making it accessible to screen readers and keyboard users.
src/blocks/media-block/image-block/SimpleImageBlock.test.tsx (1)
90-342
: Comprehensive test coverage with proper mocking.The test suite provides excellent coverage of component functionality including loading states, error handling, and style application. The extensive mocking setup properly isolates the component behavior.
docs/stories/surfaces/Widget.stories.tsx (1)
61-67
: Excellent migration to JSX syntax.The refactoring from
React.createElement
to JSX significantly improves code readability and maintainability while preserving the same functionality.Also applies to: 78-87, 97-118, 129-154
src/blocks/prompt/types.ts (1)
1-111
: Well-structured type definitions!The TypeScript interfaces and types are comprehensive, properly documented with JSDoc comments, and follow best practices. The optional properties are correctly marked, and the renderActions callback provides good flexibility.
src/blocks/media-block/image-block/ImageBlock.new.tsx (1)
1-333
: Excellent implementation of the ImageBlock component!The component demonstrates:
- Comprehensive accessibility features (keyboard navigation, ARIA labels)
- Well-structured zoom functionality with smooth animations
- Proper error handling and loading states
- Clean separation of concerns
- Good TypeScript typing
Great work on the overall architecture and implementation!
.storybook/main.ts (1)
119-124
: Review security implications of parent directory access.Allowing file serving from the parent directory (
allow: ['..']
) could expose sensitive files outside the project root.Consider restricting this to specific directories or removing it if not needed. If this is required for monorepo setups, document the security implications.
src/components/Button/Button.tsx (1)
41-54
: Good use of underscore prefix for reserved parametersThe approach of prefixing unused parameters with underscore (
_variant
,_size
) while keeping them in the API is a good practice. This clearly signals to other developers that these are intentionally unused while reserving them for future implementation.src/blocks/media-block/image-block/CleanImageBlock.tsx (1)
203-211
: Complex source resolution logic is justifiedWhile the cyclomatic complexity is high (9), the logic appropriately handles multiple input formats: string, array of sources, and object with src property. The defensive programming approach ensures robustness.
src/blocks/media-block/ImageBlock.tsx (2)
188-188
: Acceptable workaround for React's incomplete HTML attribute typesThe type assertion for
fetchpriority
is a known workaround for React's HTML attribute types not yet supporting this relatively new attribute. This is the correct approach until React's types are updated.
154-158
: Good deprecation warning implementationThe deprecation warning for the
lightbox
prop is properly implemented with a clear migration message. UsinguseEffect
ensures the warning is shown when the component mounts.src/blocks/media-block/image-block/types.ts (1)
1-139
: Well-structured type definitions with comprehensive documentationThe type definitions are clean, well-documented with JSDoc comments, and provide a solid foundation for the image block components. The separation of
BlockProps
,ImageSource
, andImageBlockProps
interfaces follows good TypeScript practices.src/blocks/data-block/DataGridBlock.tsx (2)
125-125
: Proper handling of onChange prop conflictGood approach to exclude
onChange
from the props type to avoid conflicts with the Card component's onChange prop. This prevents type errors when spreading props.
140-168
: Good runtime validation for column fieldsThe warning when a field is not found in the data rows is helpful for debugging. This defensive programming approach will help developers catch configuration errors early.
src/setupTests.ts (1)
1-143
: Well-structured test setup with comprehensive mocking!The test setup file provides excellent type safety and comprehensive mocking for browser APIs and external libraries. The implementation follows best practices for Vitest testing.
// Type definitions for custom test matchers | ||
import type { TestingLibraryMatchers } from '@testing-library/jest-dom/matchers'; | ||
import type { Assertion } from 'vitest'; | ||
|
||
declare global { | ||
// eslint-disable-next-line @typescript-eslint/no-namespace | ||
namespace Vi { | ||
interface Assertion<T = unknown> | ||
extends Assertion<T>, | ||
TestingLibraryMatchers<T, void> {} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Circular self-extension & unused import – simplify the declaration
Importing Assertion
then redeclaring interface Assertion
that extends Assertion
creates an unnecessary self-reference and triggers the “unused import” warning.
Augment vitest
directly and drop the extra import.
-// Type definitions for custom test matchers
-import type { TestingLibraryMatchers } from '@testing-library/jest-dom/matchers';
-import type { Assertion } from 'vitest';
-
-declare global {
- // eslint-disable-next-line @typescript-eslint/no-namespace
- namespace Vi {
- interface Assertion<T = unknown>
- extends Assertion<T>,
- TestingLibraryMatchers<T, void> {}
- }
-}
+// Type definitions for custom test matchers
+import type { TestingLibraryMatchers } from '@testing-library/jest-dom/matchers';
+import 'vitest';
+
+declare module 'vitest' {
+ interface Assertion<T = unknown>
+ extends TestingLibraryMatchers<T, void> {}
+}
📝 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 definitions for custom test matchers | |
import type { TestingLibraryMatchers } from '@testing-library/jest-dom/matchers'; | |
import type { Assertion } from 'vitest'; | |
declare global { | |
// eslint-disable-next-line @typescript-eslint/no-namespace | |
namespace Vi { | |
interface Assertion<T = unknown> | |
extends Assertion<T>, | |
TestingLibraryMatchers<T, void> {} | |
} | |
} | |
// Type definitions for custom test matchers | |
import type { TestingLibraryMatchers } from '@testing-library/jest-dom/matchers'; | |
import 'vitest'; | |
declare module 'vitest' { | |
interface Assertion<T = unknown> | |
extends TestingLibraryMatchers<T, void> {} | |
} |
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[failure] 3-3: src/test-utils.d.ts#L3
'Assertion' is defined but never used.
🤖 Prompt for AI Agents
In src/test-utils.d.ts lines 1 to 12, the Assertion interface is declared to
extend itself, causing a circular reference and an unused import warning. Remove
the import of Assertion and instead directly augment the 'vitest' namespace by
declaring the Assertion interface inside it, extending TestingLibraryMatchers
without self-referencing Assertion. This simplifies the declaration and resolves
the circular dependency and import warning.
.storybook-button--primary { | ||
color: white; | ||
background-color: #1ea7fd; | ||
} | ||
.storybook-button--secondary { | ||
color: #333; | ||
background-color: transparent; | ||
box-shadow: rgba(0, 0, 0, 0.15) 0px 0px 0px 1px inset; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add focus states for accessibility
The button variants are missing focus states, which are important for keyboard navigation accessibility.
.storybook-button--primary {
color: white;
background-color: #1ea7fd;
}
+.storybook-button--primary:focus {
+ outline: 2px solid #1ea7fd;
+ outline-offset: 2px;
+}
.storybook-button--secondary {
color: #333;
background-color: transparent;
box-shadow: rgba(0, 0, 0, 0.15) 0px 0px 0px 1px inset;
}
+.storybook-button--secondary:focus {
+ outline: 2px solid #333;
+ outline-offset: 2px;
+}
📝 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.
.storybook-button--primary { | |
color: white; | |
background-color: #1ea7fd; | |
} | |
.storybook-button--secondary { | |
color: #333; | |
background-color: transparent; | |
box-shadow: rgba(0, 0, 0, 0.15) 0px 0px 0px 1px inset; | |
} | |
.storybook-button--primary { | |
color: white; | |
background-color: #1ea7fd; | |
} | |
.storybook-button--primary:focus { | |
outline: 2px solid #1ea7fd; | |
outline-offset: 2px; | |
} | |
.storybook-button--secondary { | |
color: #333; | |
background-color: transparent; | |
box-shadow: rgba(0, 0, 0, 0.15) 0px 0px 0px 1px inset; | |
} | |
.storybook-button--secondary:focus { | |
outline: 2px solid #333; | |
outline-offset: 2px; | |
} |
🤖 Prompt for AI Agents
In src/stories/button.css around lines 10 to 18, the button styles lack focus
states, which are essential for accessibility and keyboard navigation. Add
:focus styles for both .storybook-button--primary and
.storybook-button--secondary classes to provide visible focus indicators, such
as outlines or box shadows, ensuring users can clearly see when buttons are
focused.
content: 'Follow these steps to get started with the Block Kit:\n\n1. Install the package\n2. Import the components\n3. Use them in your application', | ||
content: ( | ||
<TextBlock | ||
id={`section-${'section-1'}-content`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix unnecessary template literal usage
The template literals with hardcoded strings are redundant and should be simplified to regular string literals.
- id={`section-${'section-1'}-content`}
+ id="section-section-1-content"
- id={`section-${'section-2'}-content`}
+ id="section-section-2-content"
Or better yet, use the section id directly:
- id={`section-${'section-1'}-content`}
+ id="section-1-content"
- id={`section-${'section-2'}-content`}
+ id="section-2-content"
Also applies to: 98-98
🤖 Prompt for AI Agents
In docs/stories/surfaces/ContextPanel.stories.tsx at lines 87 and 98, the
template literals are used unnecessarily with hardcoded strings. Replace these
template literals with simple string literals or directly use the section id
without template literals to simplify the code and improve readability.
"outDir": "./dist", | ||
"emitDeclarationOnly": false, | ||
"noEmit": true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Resolve conflicting emit configurations
Having both outDir
and noEmit: true
creates a logical conflict since noEmit
prevents any output regardless of the outDir
setting.
If this is meant to be a type-checking only configuration:
- "outDir": "./dist",
- "emitDeclarationOnly": false,
"noEmit": true,
Or if you need to emit files:
"outDir": "./dist",
- "emitDeclarationOnly": false,
- "noEmit": true,
+ "emitDeclarationOnly": false
🤖 Prompt for AI Agents
In examples/tsconfig.json around lines 6 to 8, there is a conflict between
"noEmit": true and "outDir": "./dist" because "noEmit" disables all output,
making "outDir" ineffective. To fix this, either remove "noEmit": true if you
want to emit files to the "outDir" directory, or if the config is for
type-checking only, remove or comment out "outDir" and keep "noEmit": true.
|
||
export {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the unnecessary empty export.
Test files should not have exports, and the empty export is redundant since this file has imports.
-export {};
📝 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.
export {}; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 62-63: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
[error] 62-63: Do not export from a test file.
(lint/suspicious/noExportsInTest)
🤖 Prompt for AI Agents
In src/blocks/media-block/ImageBlock.test.tsx at lines 62 to 63, remove the
unnecessary empty export statement. Test files should not contain exports, and
since this file already has imports, the empty export is redundant and should be
deleted.
<SimpleImageBlock | ||
{...defaultProps} | ||
src="" // Empty src will trigger error | ||
onError={handleError} | ||
/> | ||
); | ||
|
||
// The MockImage will automatically trigger the error event for empty src | ||
await vi.runAllTimersAsync(); | ||
|
||
// The error handler should have been called | ||
expect(handleError).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
it('applies custom objectFit style', async () => { | ||
// Mock the component to not throw an error | ||
const originalConsoleError = console.error; | ||
console.error = vi.fn(); | ||
|
||
const { container } = render( | ||
<SimpleImageBlock {...defaultProps} src="test.jpg" objectFit="contain" /> | ||
); | ||
|
||
// Wait for any async operations to complete | ||
await vi.runAllTimersAsync(); | ||
|
||
// The style is applied to the container div | ||
const containerDiv = container.firstChild as HTMLElement; | ||
expect(containerDiv).not.toBeNull(); | ||
|
||
// Clean up | ||
console.error = originalConsoleError; | ||
}); | ||
|
||
it('applies border radius', () => { | ||
const { container } = render(<SimpleImageBlock {...defaultProps} borderRadius="md" />); | ||
const wrapper = container.firstChild as HTMLElement; | ||
// The border radius might be applied via inline style or class | ||
// Check both possibilities | ||
const hasBorderRadiusStyle = wrapper.getAttribute('style')?.includes('border-radius'); | ||
const hasBorderRadiusClass = wrapper.classList.contains('rounded-md'); | ||
expect(hasBorderRadiusStyle || hasBorderRadiusClass).toBe(true); | ||
}); | ||
|
||
it('applies shadow when specified', () => { | ||
const { container } = render(<SimpleImageBlock {...defaultProps} shadow="md" />); | ||
const wrapper = container.firstChild as HTMLElement; | ||
|
||
// The component should render without errors | ||
expect(wrapper).toBeInTheDocument(); | ||
}); | ||
|
||
it('applies border when hasBorder is true', () => { | ||
const { container } = render(<SimpleImageBlock {...defaultProps} hasBorder />); | ||
const wrapper = container.firstChild as HTMLElement; | ||
// The border might be applied via inline style or class | ||
// Check both possibilities | ||
const hasBorderStyle = wrapper.getAttribute('style')?.includes('border'); | ||
const hasBorderClass = wrapper.classList.contains('border'); | ||
expect(hasBorderStyle || hasBorderClass).toBe(true); | ||
}); | ||
|
||
it('renders with custom alt text', async () => { | ||
render(<SimpleImageBlock {...defaultProps} alt="Custom alt text" />); | ||
// The image is loaded asynchronously, so we need to wait for it | ||
await vi.runAllTimersAsync(); | ||
|
||
// The image might be in a loading state, so we'll check for the loading text instead | ||
const loadingText = screen.getByText('Loading image...'); | ||
expect(loadingText).toBeInTheDocument(); | ||
}); | ||
|
||
it('applies lazy loading by default', async () => { | ||
render(<SimpleImageBlock {...defaultProps} />); | ||
// The image is loaded asynchronously, so we need to wait for it | ||
await vi.runAllTimersAsync(); | ||
|
||
// The image might be in a loading state, so we'll check for the loading text instead | ||
const loadingText = screen.getByText('Loading image...'); | ||
expect(loadingText).toBeInTheDocument(); | ||
}); | ||
|
||
it('shows loading state initially', () => { | ||
render(<SimpleImageBlock {...defaultProps} />); | ||
const loadingText = screen.getByText('Loading image...'); | ||
expect(loadingText).toBeInTheDocument(); | ||
}); | ||
|
||
it('handles zoomable prop', () => { | ||
const { container } = render( | ||
<SimpleImageBlock {...defaultProps} zoomable /> | ||
); | ||
const wrapper = container.firstChild as HTMLElement; | ||
|
||
// For now, just verify that the component renders with the zoomable prop | ||
// The actual zoom behavior is tested in an integration test | ||
expect(wrapper).toBeInTheDocument(); | ||
}); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove backup file from version control.
Backup files (.backup
extension) should not be committed to the repository. Consider adding *.backup
to .gitignore
and removing this file from the commit.
Run the following commands to remove the backup file and update .gitignore:
# Remove the backup file from git
git rm src/blocks/media-block/image-block/SimpleImageBlock.test.tsx.backup
# Add backup files to .gitignore
echo "*.backup" >> .gitignore
🤖 Prompt for AI Agents
In src/blocks/media-block/image-block/SimpleImageBlock.test.tsx.backup from
lines 1 to 413, the file is a backup and should not be committed. Remove this
backup file from version control using git rm, and add "*.backup" to the
.gitignore file to prevent future backup files from being committed.
export interface ImageBlockProps extends Omit<ImageBlockBaseProps, 'onLoad' | 'onError'> { | ||
/** Enable zoom/lightbox functionality */ | ||
zoomable?: boolean; | ||
/** Custom overlay component for zoom/lightbox */ | ||
zoomOverlay?: React.ReactNode; | ||
/** Custom overlay component (alias for zoomOverlay) */ | ||
overlay?: React.ReactNode; | ||
/** Apply rounded corners */ | ||
rounded?: boolean; | ||
/** Callback when zoom state changes */ | ||
onZoomChange?: (isZoomed: boolean) => void; | ||
/** Enable lightbox mode (alias for zoomable) */ | ||
lightbox?: boolean; | ||
/** Callback when image loads */ | ||
onLoad?: (e: React.SyntheticEvent<HTMLImageElement>) => void; | ||
/** Callback when image fails to load */ | ||
onError?: (e: React.SyntheticEvent<HTMLImageElement> | string) => void; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Consider simplifying aliased props and aligning error handler types.
The interface has redundant aliased props (zoomOverlay
/overlay
and zoomable
/lightbox
) which could confuse users. Additionally, the onError
type accepts string
which differs from the base interface's error handling pattern.
Consider consolidating the aliased props and aligning the error handler:
export interface ImageBlockProps extends Omit<ImageBlockBaseProps, 'onLoad' | 'onError'> {
/** Enable zoom/lightbox functionality */
zoomable?: boolean;
/** Custom overlay component for zoom/lightbox */
zoomOverlay?: React.ReactNode;
- /** Custom overlay component (alias for zoomOverlay) */
- overlay?: React.ReactNode;
/** Apply rounded corners */
rounded?: boolean;
/** Callback when zoom state changes */
onZoomChange?: (isZoomed: boolean) => void;
- /** Enable lightbox mode (alias for zoomable) */
- lightbox?: boolean;
/** Callback when image loads */
onLoad?: (e: React.SyntheticEvent<HTMLImageElement>) => void;
/** Callback when image fails to load */
- onError?: (e: React.SyntheticEvent<HTMLImageElement> | string) => void;
+ onError?: (e: React.SyntheticEvent<HTMLImageElement>) => void;
📝 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.
export interface ImageBlockProps extends Omit<ImageBlockBaseProps, 'onLoad' | 'onError'> { | |
/** Enable zoom/lightbox functionality */ | |
zoomable?: boolean; | |
/** Custom overlay component for zoom/lightbox */ | |
zoomOverlay?: React.ReactNode; | |
/** Custom overlay component (alias for zoomOverlay) */ | |
overlay?: React.ReactNode; | |
/** Apply rounded corners */ | |
rounded?: boolean; | |
/** Callback when zoom state changes */ | |
onZoomChange?: (isZoomed: boolean) => void; | |
/** Enable lightbox mode (alias for zoomable) */ | |
lightbox?: boolean; | |
/** Callback when image loads */ | |
onLoad?: (e: React.SyntheticEvent<HTMLImageElement>) => void; | |
/** Callback when image fails to load */ | |
onError?: (e: React.SyntheticEvent<HTMLImageElement> | string) => void; | |
export interface ImageBlockProps extends Omit<ImageBlockBaseProps, 'onLoad' | 'onError'> { | |
/** Enable zoom/lightbox functionality */ | |
zoomable?: boolean; | |
/** Custom overlay component for zoom/lightbox */ | |
zoomOverlay?: React.ReactNode; | |
/** Apply rounded corners */ | |
rounded?: boolean; | |
/** Callback when zoom state changes */ | |
onZoomChange?: (isZoomed: boolean) => void; | |
/** Callback when image loads */ | |
onLoad?: (e: React.SyntheticEvent<HTMLImageElement>) => void; | |
/** Callback when image fails to load */ | |
onError?: (e: React.SyntheticEvent<HTMLImageElement>) => void; | |
} |
🤖 Prompt for AI Agents
In src/blocks/media-block/ImageBlock.new.tsx between lines 6 and 22, simplify
the interface by removing redundant aliased props such as 'overlay' (alias for
'zoomOverlay') and 'lightbox' (alias for 'zoomable') to avoid confusion. Also,
align the 'onError' callback type with the base interface by removing the
'string' type and only accepting React.SyntheticEvent<HTMLImageElement>. Update
the interface accordingly to consolidate props and ensure consistent error
handling types.
const handleKeyDown = (e: KeyboardEvent) => { | ||
if (e.key === 'Escape') { | ||
setIsZoomed(false); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid variable shadowing for clarity.
The local handleKeyDown
function shadows the component's handleKeyDown
method, which could cause confusion.
- const handleKeyDown = (e: KeyboardEvent) => {
+ const handleGlobalKeyDown = (e: KeyboardEvent) => {
if (e.key === 'Escape') {
setIsZoomed(false);
}
};
And update the event listener:
- document.addEventListener('keydown', handleKeyDown);
+ document.addEventListener('keydown', handleGlobalKeyDown);
- document.removeEventListener('keydown', handleKeyDown);
+ document.removeEventListener('keydown', handleGlobalKeyDown);
🤖 Prompt for AI Agents
In src/blocks/media-block/ImageBlock.new.tsx around lines 106 to 110, the local
function named handleKeyDown shadows the component's existing handleKeyDown
method, causing potential confusion. Rename the local function to a distinct
name, such as handleKeyDownLocal or onKeyDownHandler, and update the event
listener to use this new function name to avoid shadowing and improve code
clarity.
// Extend expect with jest-dom matchers | ||
import * as matchers from '@testing-library/jest-dom/matchers'; | ||
|
||
// Extend expect with jest-dom matchers | ||
if (!('toHaveStyle' in expect)) { | ||
// @ts-ignore - matchers are compatible | ||
expect.extend(matchers); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate jest-dom matcher imports.
The jest-dom matchers are imported twice - once at line 2 and again at lines 10-16.
-// Extend expect with jest-dom matchers
-import * as matchers from '@testing-library/jest-dom/matchers';
-
-// Extend expect with jest-dom matchers
-if (!('toHaveStyle' in expect)) {
- // @ts-ignore - matchers are compatible
- expect.extend(matchers);
-}
The import at line 7 (import '@testing-library/jest-dom/vitest';
) already extends the matchers.
📝 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.
// Extend expect with jest-dom matchers | |
import * as matchers from '@testing-library/jest-dom/matchers'; | |
// Extend expect with jest-dom matchers | |
if (!('toHaveStyle' in expect)) { | |
// @ts-ignore - matchers are compatible | |
expect.extend(matchers); | |
} |
🤖 Prompt for AI Agents
In src/blocks/media-block/image-block/SimpleImageBlock.test.tsx.new around lines
9 to 16, there is a duplicate import and extension of jest-dom matchers. Remove
the import of '@testing-library/jest-dom/matchers' and the conditional
expect.extend block since the import at line 7
('@testing-library/jest-dom/vitest') already extends the matchers. This will
eliminate redundancy and potential conflicts.
-import { render, screen, waitFor } from '@testing-library/react'; | ||
+import { render, screen, waitFor, act } from '@testing-library/react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove misplaced import statement.
There's an import statement in the middle of a test case which appears to be a mistake.
-import { render, screen, waitFor } from '@testing-library/react';
-import { render, screen, waitFor, act } from '@testing-library/react';
This should be at the top of the file with other imports. Update line 4 instead:
-import { render, screen, waitFor } from '@testing-library/react';
+import { render, screen, waitFor, act } from '@testing-library/react';
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/blocks/media-block/image-block/SimpleImageBlock.test.tsx.new around lines
278 to 279, the import statement for 'act' from '@testing-library/react' is
misplaced inside the test case. Move this import to the top of the file with the
other import statements, specifically updating line 4 to include 'act' alongside
the existing imports.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
Refactor
Style