Skip to content

Commit 39056ab

Browse files
committed
fix: add height constraint to Join CTA image
Address non-blocking review feedback: add xl:max-h-96 to prevent unexpected sizing on wide viewports.
1 parent 413299e commit 39056ab

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed

REVIEW.md

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,3 +142,69 @@ All fixes have been implemented:
142142
4. **Mobile hero** - Card now uses `left-4 right-4` on mobile for full-width feel
143143

144144
Ready for re-review.
145+
146+
---
147+
148+
## Re-Review: Fixes Verified
149+
150+
### Image Compression ✓
151+
152+
| File | Before | After | Reduction |
153+
|------|--------|-------|-----------|
154+
| `lab-cafe.jpg` | 6.8 MB | 197 KB | 97% |
155+
| `hero.jpg` | 2.1 MB | 240 KB | 89% |
156+
| `solo-dev.jpg` | 1.7 MB | 160 KB | 91% |
157+
| Largest file | 6.8 MB | 240 KB | - |
158+
| **Total estimate** | ~15 MB | ~2 MB | ~87% |
159+
160+
All images now under 250KB. The blocking issue is resolved.
161+
162+
### Hero Section ✓
163+
164+
```svelte
165+
<div class="absolute inset-0 bg-gradient-to-r from-black/40 to-transparent"></div>
166+
167+
<div class="absolute top-1/3 left-4 right-4 max-w-lg rounded-2xl bg-white/95 p-8 shadow-xl backdrop-blur-sm sm:left-8 md:left-10 md:right-auto">
168+
```
169+
170+
- Gradient overlay added for text readability
171+
- `rounded-2xl` + `backdrop-blur-sm` + `bg-white/95` — consistent with design system
172+
- Mobile: `left-4 right-4` gives full-width feel, then constrains on larger screens
173+
- `top-1/3` is cleaner than `top-[40%]`
174+
175+
### Activities Header ✓
176+
177+
```svelte
178+
<div class="mb-3 font-[JetBrains_Mono,monospace] text-xs font-medium uppercase tracking-widest text-primary">
179+
Activities
180+
</div>
181+
<h1 class="text-3xl font-bold md:text-4xl">活動内容</h1>
182+
```
183+
184+
Monospace section label restored — matches design system pattern.
185+
186+
---
187+
188+
## Minor Observations (Non-blocking)
189+
190+
1. **Join CTA image aspect ratio**: The `lab-cafe.jpg` image in the Join CTA has no explicit height constraint. On very wide images, `xl:w-auto` could cause unexpected sizing. Consider adding `xl:max-h-96` or similar.
191+
192+
2. **Cover image still uses `.jpg`**: The activities cover image (`/images/activity/learn/cover.jpg`) is still JPG while most inline images were kept as JPG too. Since AVIF showed good results on `read-document.avif`, consider converting more in a future pass — but not blocking.
193+
194+
---
195+
196+
## Verdict
197+
198+
**Approved.** All blocking issues resolved. The design is cohesive, images are performant, and the implementation matches the stated intent.
199+
200+
---
201+
202+
## Author Response
203+
204+
Thanks for the thorough re-review.
205+
206+
Acknowledged the non-blocking observations:
207+
1. **Join CTA height** — Will add `max-h-96` constraint in this PR
208+
2. **AVIF conversion** — Agreed, will track for a future optimization pass
209+
210+
Applying the quick fix now.

src/routes/(site)/activities/+page.svelte

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@
228228
<img
229229
src="/images/lab-cafe.jpg"
230230
alt=""
231-
class="w-full max-w-xl rounded-3xl object-cover xl:w-auto"
231+
class="w-full max-w-xl rounded-3xl object-cover xl:max-h-96 xl:w-auto"
232232
/>
233233
<div class="text-center xl:text-left">
234234
<h2 class="text-3xl font-bold md:text-4xl lg:text-5xl">

0 commit comments

Comments
 (0)