Merge pull request #37 from zentered/fix/critical-bugs-and-testing

fix: resolve critical bugs and add comprehensive testing infrastructure

Author: PatrickHeneise

package-lock.json

@@ -27,12 +27,16 @@
   ],
   "type": "module",
   "main": "src/index.jsx",
+  "types": "types/index.d.ts",
   "files": [
-    "src"
+    "src",
+    "types"
   ],
   "scripts": {
     "prepare": "husky install",
-    "test": "echo \"Error: no test specified\" && exit 1",
+    "test": "vitest run",
+    "test:watch": "vitest",
+    "test:coverage": "vitest run --coverage",
     "lint": "eslint . -c eslint.config.js",
     "codestyle": "prettier --check \"./**/*.{js,md}\""
   },
@@ -55,21 +59,26 @@
     ]
   },
   "dependencies": {
-    "cropperjs": "^2.0.0",
+    "cropperjs": "^2.1.0",
     "solid-heroicons": "^3.2.4",
     "solid-js": ">=1.6.0",
     "terracotta": "^1.0.6"
   },
   "devDependencies": {
-    "@commitlint/cli": "^19.8.0",
-    "@commitlint/config-conventional": "^19.8.0",
-    "eslint": "^9.22.0",
-    "eslint-config-prettier": "^10.1.1",
+    "@commitlint/cli": "^20.1.0",
+    "@commitlint/config-conventional": "^20.0.0",
+    "@solidjs/testing-library": "^0.8.10",
+    "@testing-library/user-event": "^14.6.1",
+    "eslint": "^9.39.1",
+    "eslint-config-prettier": "^10.1.8",
     "eslint-plugin-solid": "^0.14.5",
-    "globals": "^16.0.0",
+    "globals": "^16.5.0",
     "husky": "^9.1.7",
-    "lint-staged": "^15.5.0",
-    "prettier": "^3.5.3"
+    "jsdom": "^27.2.0",
+    "lint-staged": "^16.2.7",
+    "prettier": "^3.6.2",
+    "vite-plugin-solid": "^2.11.10",
+    "vitest": "^4.0.13"
   },
   "peerDependencies": {
     "solid-js": ">=1.6.0"

package.json

@@ -9,6 +9,17 @@ import {
 import ImageDrop from './ImageDrop.jsx'
 import { createSignal } from 'solid-js'
 
+/**
+ * ImageUploadDialog component provides a modal dialog for image upload and cropping with aspect ratio selection.
+ *
+ * @param {Object} props - Component props
+ * @param {string} props.title - Title displayed in the dialog header
+ * @param {Function} props.isOpen - Accessor function that returns whether the dialog is open
+ * @param {Function} props.closeModal - Callback function to close the dialog
+ * @param {Function} props.openModal - Callback function to open the dialog
+ * @param {Function} props.saveImage - Callback function called when the user saves the cropped image. Receives the state object containing error, loading, file, and croppedImage.
+ * @returns {JSX.Element} The ImageUploadDialog component
+ */
 export default function ImageUploadDialog(props) {
   const [width, setWidth] = createSignal(16)
   const [height, setHeight] = createSignal(9)

src/Dialog.jsx

@@ -1,6 +1,5 @@
 import {
   createEffect,
-  createRenderEffect,
   createSignal,
   Show,
   on,
@@ -11,9 +10,27 @@ import 'cropperjs'
 import { Icon } from 'solid-heroicons'
 import { cloudArrowUp, photo } from 'solid-heroicons/solid'
 
+/**
+ * ImageDrop component provides a drag-and-drop interface for uploading and cropping images.
+ *
+ * @param {Object} props - Component props
+ * @param {Function} props.saveImage - Callback function called when the user saves the cropped image. Receives the state object containing error, loading, file, and croppedImage.
+ * @param {number} [props.aspectRatioWidth=1] - Initial aspect ratio width
+ * @param {number} [props.aspectRatioHeight=1] - Initial aspect ratio height
+ * @param {string[]} [props.acceptedFileTypes=['image/jpeg', 'image/png', 'image/webp', 'image/gif']] - Array of accepted file MIME types
+ * @param {number} [props.maxFileSizeMB=10] - Maximum file size in megabytes
+ * @param {Function} [props.onError] - Optional callback function called when an error occurs. Receives the error message as a string.
+ * @returns {JSX.Element} The ImageDrop component
+ */
 export default function ImageDrop(props) {
   let cropperImage
   let cropperSelection
+
+  // Set default values for validation props
+  const acceptedFileTypes = props.acceptedFileTypes || ['image/jpeg', 'image/png', 'image/webp', 'image/gif']
+  const maxFileSizeMB = props.maxFileSizeMB || 10
+  const maxFileSizeBytes = maxFileSizeMB * 1024 * 1024
+
   const [state, setState] = createStore({
       error: null,
       loading: false,
@@ -28,8 +45,32 @@ export default function ImageDrop(props) {
     noPropagate = (e) => {
       e.preventDefault()
     },
+    handleError = (message) => {
+      setState('error', message)
+      if (props.onError) {
+        props.onError(message)
+      }
+    },
     uploadFile = async (file) => {
       if (!file) return
+
+      // Reset error state
+      setState('error', null)
+
+      // Validate file type
+      if (!acceptedFileTypes.includes(file.type)) {
+        const message = `Invalid file type. Please upload one of: ${acceptedFileTypes.join(', ')}`
+        handleError(message)
+        return
+      }
+
+      // Validate file size
+      if (file.size > maxFileSizeBytes) {
+        const message = `File size exceeds ${maxFileSizeMB}MB limit. Current file: ${(file.size / 1024 / 1024).toFixed(2)}MB`
+        handleError(message)
+        return
+      }
+
       setUploading(true)
       setState('loading', true)
       setState('file', file)
@@ -38,12 +79,11 @@ export default function ImageDrop(props) {
         reader.onload = (e) => {
           setPreview(e.target.result)
         }
-        createRenderEffect(() => {})
         reader.readAsDataURL(file)
       } catch (e) {
         console.error('upload failed', e)
         const message = e instanceof Error ? e.message : String(e)
-        setState('error', message)
+        handleError(message)
       }
       setState('loading', false)
       setUploading(false)
@@ -73,8 +113,8 @@ export default function ImageDrop(props) {
 
   createEffect(
     on(preview, () => {
-      if (cropperImage) {
-        cropperImage.src = `${import.meta.env.BASE_URL}picture1.png`
+      if (cropperImage && preview()) {
+        cropperImage.src = preview()
       }
     })
   )
@@ -166,12 +206,18 @@ export default function ImageDrop(props) {
               id="image-upload"
               name="file"
               type="file"
+              accept={acceptedFileTypes.join(',')}
               disabled={uploading()}
               multiple={false}
               onInput={handleFileInput}
               class="sr-only"
             />
           </div>
+          <Show when={state.error}>
+            <div class="mt-4 p-4 bg-red-100 border border-red-400 text-red-700 rounded-md">
+              <p class="text-sm">{state.error}</p>
+            </div>
+          </Show>
           <div class="h-8" />
         </form>
       </Show>

src/ImageDrop.jsx

@@ -0,0 +1,141 @@
+import { render, screen, fireEvent } from '@solidjs/testing-library'
+import { describe, it, expect, vi } from 'vitest'
+import { createSignal } from 'solid-js'
+import Dialog from '../Dialog.jsx'
+
+// Mock cropperjs web components
+globalThis.customElements = {
+  define: vi.fn()
+}
+
+describe('Dialog', () => {
+  it('should render when isOpen is true', () => {
+    const [isOpen] = createSignal(true)
+    const mockCloseModal = vi.fn()
+    const mockOpenModal = vi.fn()
+    const mockSaveImage = vi.fn()
+
+    render(() => (
+      <Dialog
+        title="Test Dialog"
+        isOpen={isOpen}
+        closeModal={mockCloseModal}
+        openModal={mockOpenModal}
+        saveImage={mockSaveImage}
+      />
+    ))
+
+    expect(screen.getByText('Test Dialog')).toBeTruthy()
+  })
+
+  it('should not render when isOpen is false', () => {
+    const [isOpen] = createSignal(false)
+    const mockCloseModal = vi.fn()
+    const mockOpenModal = vi.fn()
+    const mockSaveImage = vi.fn()
+
+    const { container } = render(() => (
+      <Dialog
+        title="Test Dialog"
+        isOpen={isOpen}
+        closeModal={mockCloseModal}
+        openModal={mockOpenModal}
+        saveImage={mockSaveImage}
+      />
+    ))
+
+    // The dialog should not be visible
+    expect(screen.queryByText('Test Dialog')).toBeNull()
+  })
+
+  it('should render aspect ratio selector with default options', () => {
+    const [isOpen] = createSignal(true)
+    const mockCloseModal = vi.fn()
+    const mockOpenModal = vi.fn()
+    const mockSaveImage = vi.fn()
+
+    render(() => (
+      <Dialog
+        title="Upload Image"
+        isOpen={isOpen}
+        closeModal={mockCloseModal}
+        openModal={mockOpenModal}
+        saveImage={mockSaveImage}
+      />
+    ))
+
+    const select = document.querySelector('select')
+    expect(select).toBeTruthy()
+
+    const options = select.querySelectorAll('option')
+    expect(options.length).toBe(3)
+    expect(options[0].value).toBe('16:9')
+    expect(options[1].value).toBe('4:3')
+    expect(options[2].value).toBe('1:1')
+  })
+
+  it('should update aspect ratio when selector changes', () => {
+    const [isOpen] = createSignal(true)
+    const mockCloseModal = vi.fn()
+    const mockOpenModal = vi.fn()
+    const mockSaveImage = vi.fn()
+
+    render(() => (
+      <Dialog
+        title="Upload Image"
+        isOpen={isOpen}
+        closeModal={mockCloseModal}
+        openModal={mockOpenModal}
+        saveImage={mockSaveImage}
+      />
+    ))
+
+    const select = document.querySelector('select')
+
+    // Change to 1:1 aspect ratio
+    fireEvent.change(select, { target: { value: '1:1' } })
+
+    // The component should handle the change without errors
+    expect(select.value).toBe('1:1')
+  })
+
+  it('should pass saveImage callback to ImageDrop', () => {
+    const [isOpen] = createSignal(true)
+    const mockCloseModal = vi.fn()
+    const mockOpenModal = vi.fn()
+    const mockSaveImage = vi.fn()
+
+    render(() => (
+      <Dialog
+        title="Upload Image"
+        isOpen={isOpen}
+        closeModal={mockCloseModal}
+        openModal={mockOpenModal}
+        saveImage={mockSaveImage}
+      />
+    ))
+
+    // The ImageDrop component should be rendered inside the dialog
+    const dropzone = document.getElementById('dropzone')
+    expect(dropzone).toBeTruthy()
+  })
+
+  it('should render with correct title', () => {
+    const [isOpen] = createSignal(true)
+    const mockCloseModal = vi.fn()
+    const mockOpenModal = vi.fn()
+    const mockSaveImage = vi.fn()
+
+    render(() => (
+      <Dialog
+        title="Custom Title"
+        isOpen={isOpen}
+        closeModal={mockCloseModal}
+        openModal={mockOpenModal}
+        saveImage={mockSaveImage}
+      />
+    ))
+
+    expect(screen.getByText('Custom Title')).toBeTruthy()
+  })
+})