jedmund-svelte/docs/branding-form-refactoring.md

# Project Branding Form Refactoring

**Date**: 2025-10-10
**Status**: ✅ Complete

## Overview

Comprehensive refactoring of `ProjectBrandingForm.svelte` to follow Svelte 5 best practices, proper component composition, semantic HTML5, and BEM CSS naming conventions.

## Goals Achieved

✅ Extracted reusable components
✅ Consolidated reactive state logic
✅ Improved separation of concerns
✅ Implemented semantic HTML5 markup
✅ Applied BEM CSS naming
✅ Simplified maintenance and readability

## New Components Created

### 1. BrandingToggle.svelte
**Purpose**: Reusable toggle switch component
**Location**: `/src/lib/components/admin/BrandingToggle.svelte`

**Features**:
- Two-way binding with `$bindable()`
- Disabled state support
- Optional onChange callback
- BEM naming: `.branding-toggle`, `.branding-toggle__input`, `.branding-toggle__slider`

**Props**:
```typescript
interface Props {
  checked: boolean          // Two-way bindable
  disabled?: boolean        // Optional, defaults to false
  onchange?: (checked: boolean) => void  // Optional callback
}
```

### 2. BrandingSection.svelte
**Purpose**: Wrapper component for form sections with header + toggle pattern
**Location**: `/src/lib/components/admin/BrandingSection.svelte`

**Features**:
- Semantic `<section>` and `<header>` elements
- Optional toggle in header
- Snippet-based children rendering
- BEM naming: `.branding-section`, `.branding-section__header`, `.branding-section__title`, `.branding-section__content`

**Props**:
```typescript
interface Props {
  title: string                    // Section header text
  toggleChecked?: boolean          // Two-way bindable toggle state
  toggleDisabled?: boolean         // Toggle disabled state
  showToggle?: boolean             // Whether to show toggle (default: true)
  children?: import('svelte').Snippet  // Content slot
}
```

## Script Refactoring

### Before
- **6 separate `$effect` blocks** scattered throughout
- **Duplicated Media object creation logic** (2 identical blocks)
- **Poor organization** - no clear sections

### After
- **Organized into 3 clear sections** with comments:
  1. Media State Management
  2. Derived Toggle States
  3. Upload Handlers
- **Extracted helper function** `createMediaFromUrl()` - DRY principle
- **Consolidated $effect blocks**:
  - Single initialization effect for both Media objects
  - Single sync effect for URL cleanup
  - Single auto-disable effect for all three toggles
- **Used `$derived` for computed values**: `hasFeaturedImage`, `hasBackgroundColor`, `hasLogo`

### Key Improvements

**Media Object Creation**:
```typescript
// Before: Duplicated 40-line blocks for logo and featured image

// After: Single reusable function
function createMediaFromUrl(url: string, filename: string, mimeType: string): Media {
  return {
    id: -1,
    filename,
    originalName: filename,
    mimeType,
    size: 0,
    url,
    thumbnailUrl: url,
    width: null,
    height: null,
    altText: null,
    description: null,
    usedIn: [],
    createdAt: new Date(),
    updatedAt: new Date()
  }
}
```

**Derived State**:
```typescript
// Before: Repeated checks in multiple places

// After: Single source of truth
const hasFeaturedImage = $derived(!!(formData.featuredImage && featuredImageMedia) || !!featuredImageMedia)
const hasBackgroundColor = $derived(!!(formData.backgroundColor && formData.backgroundColor.trim()))
const hasLogo = $derived(!!(formData.logoUrl && logoMedia) || !!logoMedia)
```

**Consolidated Auto-disable**:
```typescript
// Before: 3 separate $effect blocks

// After: Single effect
$effect(() => {
  if (!hasFeaturedImage) formData.showFeaturedImageInHeader = false
  if (!hasBackgroundColor) formData.showBackgroundColorInHeader = false
  if (!hasLogo) formData.showLogoInHeader = false
})
```

## Markup Refactoring

### Before
- Mixed `<div>` and `<section>` elements
- Inline toggle markup repeated 3 times
- Conditional rendering of logo section with Button fallback
- Non-semantic class names

### After
- Consistent use of `BrandingSection` component wrapper
- All toggles rendered via reusable `BrandingToggle` component
- Logo uploader always visible (no conditional rendering)
- Semantic HTML5 throughout
- Snippet-based content composition

**Example Section**:
```svelte
<BrandingSection
  title="Featured image"
  bind:toggleChecked={formData.showFeaturedImageInHeader}
  toggleDisabled={!hasFeaturedImage}
>
  {#snippet children()}
    <ImageUploader
      label=""
      bind:value={featuredImageMedia}
      onUpload={handleFeaturedImageUpload}
      onRemove={handleFeaturedImageRemove}
      placeholder="Drag and drop a featured image here, or click to browse"
      showBrowseLibrary={true}
      compact={true}
    />
  {/snippet}
</BrandingSection>
```

## SCSS Refactoring

### Before
- 117 lines of SCSS
- Multiple unused classes:
  - `.section-header-inline`
  - `.section-toggle-inline`
  - `.form-row`
- Global `.form` class name
- Toggle styles duplicated with multiple selectors

### After
- **8 lines of SCSS** (93% reduction)
- BEM naming: `.branding-form`
- All component-specific styles moved to component files
- Only container-level styles remain

**Final Styles**:
```scss
.branding-form {
  display: flex;
  flex-direction: column;
  gap: $unit-4x;
  margin-bottom: $unit-6x;

  &:last-child {
    margin-bottom: 0;
  }
}
```

## Files Modified

### Created
1. `/src/lib/components/admin/BrandingToggle.svelte` (58 lines)
2. `/src/lib/components/admin/BrandingSection.svelte` (46 lines)

### Modified
1. `/src/lib/components/admin/ProjectBrandingForm.svelte`
   - Script: 139 lines → 103 lines (26% reduction)
   - Markup: 129 lines → 93 lines (28% reduction)
   - Styles: 117 lines → 8 lines (93% reduction)
   - **Total**: 385 lines → 204 lines (47% overall reduction)

## Benefits

### Developer Experience
- **Easier to understand**: Clear section organization with comments
- **Easier to maintain**: Single source of truth for derived state
- **Easier to test**: Extracted components can be tested independently
- **Easier to extend**: New sections follow same pattern

### Code Quality
- **DRY principle**: No duplicated Media creation logic
- **Separation of concerns**: Each component has single responsibility
- **Type safety**: Maintained throughout with TypeScript interfaces
- **Svelte 5 patterns**: Proper use of runes ($state, $derived, $effect, $bindable)

### Performance
- **Fewer reactivity subscriptions**: Consolidated effects reduce overhead
- **Optimized re-renders**: Derived state only recalculates when dependencies change

## TypeScript Fixes Applied

During refactoring, the following TypeScript issues were identified and resolved:

1. **Media Type Mismatch**: The `createMediaFromUrl()` function was using non-existent properties (`altText`) from an outdated Media interface. Fixed by matching the actual Prisma schema with all required fields.

2. **Optional Chaining**: Added optional chaining (`?.`) to `backgroundColor.trim()` to handle potentially undefined values.

3. **Bindable Default Value**: Added default value `false` to `$bindable()` in BrandingSection to satisfy type requirements when `toggleChecked` is optional.

**Changes Made**:
```typescript
// Fixed optional chaining
const hasBackgroundColor = $derived(!!(formData.backgroundColor && formData.backgroundColor?.trim()))

// Fixed bindable default
toggleChecked = $bindable(false)

// Fixed Media object creation
function createMediaFromUrl(url: string, filename: string, mimeType: string): Media {
  return {
    // ... all required Prisma Media fields including:
    // isPhotography, exifData, photoCaption, photoTitle, photoDescription,
    // photoSlug, photoPublishedAt, dominantColor, colors, aspectRatio,
    // duration, videoCodec, audioCodec, bitrate
  }
}
```

## Verification

✅ Build passes: `npm run build` - no errors
✅ Type checking passes: No TypeScript errors in refactored components
✅ All existing functionality preserved:
  - Live preview updates
  - Toggle enable/disable logic
  - Image upload/remove with auto-save
  - Media object synchronization
  - Form validation integration

## Future Considerations

### Optional Enhancements
1. **Extract Media utilities**: Could create `$lib/utils/media.ts` with `createMediaFromUrl()` if needed elsewhere
2. **Add accessibility**: ARIA labels and keyboard shortcuts for toggles
3. **Add animations**: Transitions when sections enable/disable
4. **Add tests**: Unit tests for BrandingToggle and BrandingSection

### Related Files That Could Use Similar Refactoring
- `ProjectForm.svelte` - Could benefit from similar section-based organization
- `ImageUploader.svelte` - Could extract toggle pattern if it uses similar UI

## Notes

- Removed unused `showLogoSection` state variable
- Removed unused `Button` import
- All toggle states now managed consistently through derived values
- BEM naming convention applied to maintain CSS specificity without deep nesting