|
| 1 | +# Design Review: Picture-ful Design PR |
| 2 | + |
| 3 | +## Overview |
| 4 | + |
| 5 | +This PR transitions the design from a **minimal card-based layout** to an **image-rich magazine-style layout**. The changes affect the hero section and activities page. |
| 6 | + |
| 7 | +--- |
| 8 | + |
| 9 | +## Strengths |
| 10 | + |
| 11 | +1. **Visual Impact**: The full-height hero with background imagery creates a stronger first impression than the previous centered text approach |
| 12 | + |
| 13 | +2. **Reusable Component**: `ImageSection.svelte` is well-structured with clean alternating left/right alignment logic |
| 14 | + |
| 15 | +3. **Content Density**: The magazine layout allows more visual storytelling while maintaining readability |
| 16 | + |
| 17 | +4. **Join CTA**: The new dark section with image creates a strong closing statement |
| 18 | + |
| 19 | +--- |
| 20 | + |
| 21 | +## Design Concerns |
| 22 | + |
| 23 | +### 1. Image Performance (Critical) |
| 24 | + |
| 25 | +| File | Size | Issue | |
| 26 | +|------|------|-------| |
| 27 | +| `lab-cafe.jpg` | **6.8 MB** | Far too large | |
| 28 | +| `hero.jpg` | **2.1 MB** | Should be < 300KB | |
| 29 | +| `solo-dev.jpg` | **1.7 MB** | Needs compression | |
| 30 | +| `hackathon.jpg` | **1.3 MB** | Needs compression | |
| 31 | + |
| 32 | +**Recommendation**: Compress all images. Target < 200-300KB each. Consider using AVIF format (only one image uses it currently). The activities page will load 10+ MB of images. |
| 33 | + |
| 34 | +### 2. Hero Section Issues |
| 35 | + |
| 36 | +```svelte |
| 37 | +<div class="absolute top-[40%] left-6 mr-6 max-w-lg bg-white p-8 shadow-xl md:left-10"> |
| 38 | +``` |
| 39 | + |
| 40 | +- **No rounded corners** on the floating card (inconsistent with `ImageSection`'s `rounded-2xl`) |
| 41 | +- **`top-[40%]`** is unusual positioning—may clip on shorter viewports |
| 42 | +- **No dark overlay** on background image—text readability depends entirely on image content |
| 43 | +- **`left-6`** feels cramped on mobile; consider `left-4` or full-width on mobile |
| 44 | + |
| 45 | +### 3. ImageSection Mobile Experience |
| 46 | + |
| 47 | +On mobile, the image bleeds edge-to-edge but text has no background. This creates a jarring visual break. Consider: |
| 48 | +- Adding a subtle background to text section |
| 49 | +- Or keeping consistent edge spacing |
| 50 | + |
| 51 | +### 4. Activities Page Header Icon |
| 52 | + |
| 53 | +```svelte |
| 54 | +<div class="flex h-12 w-12 items-center justify-center rounded-lg bg-zinc-900 text-white"> |
| 55 | + <BookOpen class="h-6 w-6" /> |
| 56 | +</div> |
| 57 | +``` |
| 58 | + |
| 59 | +Uses `BookOpen` (the Learn section icon) for the page header. Consider a more generic icon or removing it since it's redundant with section icons below. |
| 60 | + |
| 61 | +### 5. Missing Design System Elements |
| 62 | + |
| 63 | +The new design drops several patterns from the design language: |
| 64 | + |
| 65 | +| Pattern | Before | After | |
| 66 | +|---------|--------|-------| |
| 67 | +| Section labels | `font-mono uppercase tracking-widest text-primary` | Removed | |
| 68 | +| Hover states | `hover:bg-primary/5 hover:border-primary/30` | Not on ImageSection | |
| 69 | +| Card styling | `backdrop-blur-md border-zinc-200/50` | Plain white card in hero | |
| 70 | + |
| 71 | +--- |
| 72 | + |
| 73 | +## Recommendations |
| 74 | + |
| 75 | +1. **Compress images** before merging—this is blocking |
| 76 | +2. Add `rounded-2xl` to hero card for consistency |
| 77 | +3. Add overlay or gradient to hero image for text readability |
| 78 | +4. Consider responsive hero card (full-width on mobile with proper spacing) |
| 79 | +5. Add subtle hover state to ImageSection CTA buttons |
| 80 | +6. Replace page header icon or remove it |
| 81 | + |
| 82 | +--- |
| 83 | + |
| 84 | +## Questions for Discussion |
| 85 | + |
| 86 | +1. Is the full-height hero intentional for landing? On the activities page, users must scroll past empty space to see content |
| 87 | +2. Should `ImageSection` support a `caption` prop for image credits/alt descriptions? |
| 88 | +3. Is the sharp contrast between hero card (no radius) and content sections (rounded) intentional? |
| 89 | + |
| 90 | +--- |
| 91 | + |
| 92 | +## Response from Author |
| 93 | + |
| 94 | +Thank you for the thorough review. You've raised valid concerns. |
| 95 | + |
| 96 | +### Answers to Questions |
| 97 | + |
| 98 | +1. **Full-height hero**: Yes, intentional for homepage only. The activities page does NOT have a full-height hero—it has a `h-72 md:h-96` cover image (same as utcode.net's activity pages). The full-height hero is only on the homepage. |
| 99 | + |
| 100 | +2. **Caption prop**: Good idea, but I'd defer this—we don't currently have image credits in the source content. Can add when needed. |
| 101 | + |
| 102 | +3. **Sharp contrast on hero card**: Not intentional—this was an oversight. Will fix. |
| 103 | + |
| 104 | +### Fixes I'll Apply |
| 105 | + |
| 106 | +| Issue | Action | |
| 107 | +|-------|--------| |
| 108 | +| Image sizes (blocking) | Compress all to AVIF < 200KB | |
| 109 | +| Hero card no radius | Add `rounded-2xl` | |
| 110 | +| Hero no overlay | Add gradient overlay for text readability | |
| 111 | +| Activities header icon | Remove redundant BookOpen icon, use generic layout | |
| 112 | +| ImageSection hover | CTA buttons already have hover states (`hover:bg-primary hover:text-zinc-900`) | |
| 113 | +| Design system patterns | The section labels were intentionally simplified to reduce visual noise with large images | |
| 114 | + |
| 115 | +### On Mobile Experience |
| 116 | + |
| 117 | +The edge-to-edge image with text below is intentional—it matches utcode.net's pattern. The text section has sufficient padding (`p-8`). Adding a background would create extra visual weight. Happy to discuss further if you feel strongly. |
| 118 | + |
| 119 | +### Image Compression Plan |
| 120 | + |
| 121 | +Will convert to AVIF and target: |
| 122 | +- Hero/cover images: < 200KB |
| 123 | +- Section images: < 150KB |
| 124 | +- Total page weight: < 1.5MB (down from 15MB+) |
| 125 | + |
| 126 | +Proceeding with fixes now. |
| 127 | + |
| 128 | +--- |
| 129 | + |
| 130 | +## Fixes Applied |
| 131 | + |
| 132 | +All fixes have been implemented: |
| 133 | + |
| 134 | +1. **Images compressed** - Total reduced from 15MB+ to ~2MB |
| 135 | + - Largest image now 240KB (was 6.8MB) |
| 136 | + - All images under 250KB |
| 137 | + |
| 138 | +2. **Hero card** - Added `rounded-2xl`, `backdrop-blur-sm`, gradient overlay |
| 139 | + |
| 140 | +3. **Activities header** - Replaced icon with monospace section label (matches design system) |
| 141 | + |
| 142 | +4. **Mobile hero** - Card now uses `left-4 right-4` on mobile for full-width feel |
| 143 | + |
| 144 | +Ready for re-review. |
0 commit comments