feat(ui): mobile adaptation, visual refinement, and performance optimization#4
Conversation
Design documents for three-phase frontend optimization: - Phase 1: Mobile adaptation (scroll-driven animation, safe areas, page transitions, touch feedback) - Phase 2: Visual refinement (font loading, reduced-motion, color palette, animation performance) - Phase 3: Performance optimization (Supabase channel fix, scroll throttling, image sizes) Includes BDD specifications, architecture details, and best practices guidelines.
10 tasks covering mobile adaptation, visual refinement, and performance optimization. BDD-driven with 14 scenarios, dependency graph, and acceptance criteria per task. Reflection fixes applied: - task-006 depends-on corrected to [004] (references @supports block) - task-003 reduced-motion clause removed (handled by task-006 globally) - task-009 BDD scenario added to bdd-specs.md - Edge case scenarios marked as deferred
…ization - Safe area: viewportFit cover + pt/pb/p-safe utilities for iOS notch - Touch feedback: -webkit-tap-highlight-color transparent + active scale(0.97) - Page transitions: template.tsx with AnimatePresence + motion.div - FloatingHeader: CSS scroll-driven animation (Chrome 144+) + rAF JS fallback - Font loading: self-host Noto Serif SC via next/font/google, remove Google Fonts links - Reduced motion: prefers-reduced-motion: reduce kills all animations globally - Color palette: pure black/white replaced with tinted stone-50/stone-900 - Supabase: channel lifecycle via useRef (fixes global variable leak) - Image sizes: precise sizes attributes matching custom breakpoints - framer-motion: 12 files migrated from motion to m + LazyMotion/domAnimation
Linter auto-formatted LazyMotion wrapper indentation.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@codex review一下这个PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f39145acb5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const notoSerifSC = NotoSerifSC({ | ||
| subsets: ['latin'], | ||
| weight: ['400', '500', '600', '700'], | ||
| display: 'swap', | ||
| variable: '--font-noto-serif-sc' | ||
| }) |
There was a problem hiding this comment.
Avoid making offline builds fetch Google Fonts
In CI/deploy environments that cannot reach fonts.googleapis.com, this new next/font/google call makes next build fail before the app compiles; I reproduced it with npm run build, which aborts with Failed to fetch font Noto Serif SC. The previous <link> only depended on Google at runtime, so if the goal is truly self-hosting/reproducible builds, this needs to use a checked-in next/font/local font file or another build-time fallback.
Useful? React with 👍 / 👎.
| .floating-header { | ||
| animation: floating-header-reveal linear both; | ||
| animation-timeline: scroll(); | ||
| animation-range: 0px 100px; |
There was a problem hiding this comment.
Keep the mobile header visible at scroll top
When a browser supports CSS scroll timelines, this animation applies to the entire .floating-header and both fill means it starts at translateY(-100%)/opacity: 0 while the scroll area is at the top. Since FloatingHeader now always has this class, the mobile menu/back/RSS controls disappear on initial load until the user scrolls about 100px, whereas the old behavior only faded the center scrollTitle text.
Useful? React with 👍 / 👎.
| {scrollTitle && ( | ||
| <span | ||
| className="line-clamp-2 font-semibold tracking-tight" | ||
| style={{ | ||
| transform: `translateY(${transformValues.translateY}%)`, | ||
| opacity: transformValues.opacity | ||
| }} | ||
| > | ||
| <span ref={spanRef} className="line-clamp-2 font-semibold tracking-tight"> | ||
| {scrollTitle} | ||
| </span> |
There was a problem hiding this comment.
Initialize the fallback title as hidden
In browsers without CSS scroll-timeline support, the JS fallback no longer sets the initial style on the scrollTitle span, so pages like /writing/[slug] render the header title fully visible at scroll position 0 until the first scroll event recalculates it. The removed state-backed inline style initialized this to opacity 0, so the fallback needs an initial opacity/transform style or an immediate onScroll() call after attaching the listener.
Useful? React with 👍 / 👎.
| <motion.div | ||
| key={pathname} | ||
| initial={{ opacity: 0, y: 8 }} | ||
| animate={{ opacity: 1, y: 0 }} | ||
| exit={{ opacity: 0, y: -8 }} | ||
| transition={{ duration: 0.2, ease: [0.25, 0.1, 0.25, 1] }} | ||
| > |
There was a problem hiding this comment.
Preserve page sizing in the transition wrapper
The root layout places pages inside a flex flex-1 container, but this new wrapper becomes the only flex item and has no flex-1/w-full sizing. On desktop routes next to the sidebar, the actual page ScrollArea is now nested inside a shrink-to-content flex item instead of directly filling the available content pane, so pages can lose their full-width layout unless the transition wrapper carries the same flex/full-width sizing.
Useful? React with 👍 / 👎.
| <DialogStateProvider> | ||
| {} | ||
| <main vaul-drawer-wrapper="" className="min-h-screen bg-white"> | ||
| <main vaul-drawer-wrapper="" className="pt-safe min-h-screen bg-white"> |
There was a problem hiding this comment.
Don't crop scroll areas with top safe padding
On devices where env(safe-area-inset-top) is non-zero, adding pt-safe to the root <main> makes every child ScrollArea start below the notch while those scroll areas still size themselves to 100dvh; because the page shell hides viewport overflow, the bottom safe-inset amount of each scrollport is pushed below the visible viewport and the last content can be obscured. Apply the safe padding inside the scrollport or reduce its height by the inset instead of increasing the outer shell height.
Useful? React with 👍 / 👎.
| <m.div | ||
| layoutId="active-filter-bg" | ||
| className="absolute inset-0 rounded-lg bg-white shadow-sm" |
There was a problem hiding this comment.
Load layout features for layoutId animations
This layoutId is a shared-layout animation feature, but the surrounding LazyMotion now loads only domAnimation, which contains animation/gesture features but not the layout projection bundle. In the Visual filter tabs, the active background will still re-render in the new tab but it will no longer animate between tabs; use the layout-capable feature bundle for this component or drop layoutId if the animation is intentionally removed.
Useful? React with 👍 / 👎.
| if (channelRef.current) { | ||
| supabase.removeChannel(channelRef.current) |
There was a problem hiding this comment.
Don't tear down the shared realtime channel
When two useViewData instances are mounted, Supabase returns the same channel for the identical supabase_realtime topic, so each hook's ref can point at the shared channel. On /writing/[slug], the persistent WritingListLayout and the article's WritingViews mount this hook together; when the article unmounts, this cleanup removes the shared channel and the still-mounted sidebar stops receiving realtime view updates.
Useful? React with 👍 / 👎.
| alt={`${item.sourceType} ${item.mediaType}`} | ||
| width={baseWidth} | ||
| height={calculatedHeight} | ||
| sizes="(max-width: 390px) 100vw, (max-width: 768px) 50vw, 33vw" |
There was a problem hiding this comment.
Match image sizes to the one-column breakpoint
The gallery remains one column until the custom sm breakpoint at 435px, but this sizes string switches to 50vw immediately above 390px. On common 393–430px mobile viewports, Cloudinary will choose roughly half-width images while the card still renders full-width, causing visibly blurry thumbnails; keep 100vw through the actual sm breakpoint.
Useful? React with 👍 / 👎.
| * [PROTOCOL]: 变更时更新此头部,然后检查 CLAUDE.md | ||
| */ | ||
|
|
||
| import { AnimatePresence, motion } from 'framer-motion' |
There was a problem hiding this comment.
Keep full motion bundle out of the root template
Because this root template is a client component rendered for every route, importing motion here pulls Framer Motion's pre-bundled feature set into the baseline client bundle even though the rest of this change migrates components to m + LazyMotion for bundle reduction. Use the lazy m component here as well, or the claimed bundle-size win is largely negated on every page.
Useful? React with 👍 / 👎.
- Migrate Noto Serif SC to next/font/local for offline CI builds - Fix FloatingHeader scroll-timeline fill-mode (both → forwards) - Add initial inline style to JS fallback spanRef - Migrate template.tsx to LazyMotion/m with flex-1 w-full - Move pt-safe from <main> to ScrollArea to fix scroll clipping - Add module-level channel registry with ref counting for Supabase - Fix image sizes breakpoint (390px → 435px) to match custom sm - Switch tab-selector from domAnimation to domMax for layoutId Ref: PR #4 chatgpt-codex-connector[bot] review comments
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d138b305b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <AnimatePresence mode="wait"> | ||
| <m.div | ||
| key={pathname} | ||
| className="w-full flex-1" |
There was a problem hiding this comment.
Keep the route template as a flex container
Fresh evidence after the previous sizing fix: the root template now wraps nested layouts in this <m.div>, but it only carries w-full flex-1 and not flex. src/app/writing/layout.js returns the inner SideMenu and content pane as sibling elements that previously relied on the parent flex row, so on desktop /writing/* routes the inner sidebar stacks above the article instead of sitting beside it. Add flex here (or wrap the writing layout itself) so those sibling panes keep the original horizontal layout.
Useful? React with 👍 / 👎.
| height={450} | ||
| quality="auto" | ||
| format="auto" | ||
| sizes="(max-width: 390px) 100vw, (max-width: 768px) 50vw, 33vw" |
There was a problem hiding this comment.
Match the desk photo sizes to its rendered width
On mobile widths just above 390px, the desk setup image is still rendered as a single full-width card inside content-wrapper (for example ~382px wide on a 430px viewport), but this sizes value tells Cloudinary to serve a 50vw image. That makes the browser choose roughly half-width resources for the full-width photo, so the workspace hero image becomes visibly blurry on common 393–768px phones/tablets; keep 100vw until the layout actually constrains the image to half width.
Useful? React with 👍 / 👎.
|
|
||
| // 设置实时订阅 | ||
| // ---- Realtime subscription (reference-counted) ------------------------- | ||
| useEffect(() => { |
There was a problem hiding this comment.
Avoid mutating a channel while it is joining
Fresh evidence from the current ref-counted implementation: when two useViewData consumers mount together, such as the /writing/[slug] sidebar list plus WritingViews, the first effect calls subscribe() and starts joining, then the second effect reuses the same channel and appends another postgres_changes binding here. Supabase builds the join payload before that second binding is added, but validates the larger client binding list when the join reply arrives, so the channel can emit CHANNEL_ERROR/unsubscribe and realtime view counts stop working for both consumers.
Useful? React with 👍 / 👎.
- P1: defer realtime handler registration until channel is SUBSCRIBED to avoid mutating bindings during join handshake (CHANNEL_ERROR) - P2: add flex to route transition wrapper so writing layout keeps horizontal sidebar+content layout - P2: match desk photo sizes breakpoint to custom sm (435px)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94cbbd87c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| entry.pending = [] | ||
|
|
||
| for (const { event, filter, callback } of handlers) { | ||
| entry.channel.on(event, filter, callback).subscribe() |
There was a problem hiding this comment.
Register Supabase handlers before subscribing
When the first useViewData consumer mounts, registerHandler queues the only postgres_changes listener, so the subsequent initial channel.subscribe() joins with an empty binding list; after SUBSCRIBED, this line appends the listener and calls subscribe() again, but @supabase/realtime-js only sends the join payload while the channel state is closed and otherwise returns the existing channel. That means the flushed binding never gets a server id and UPDATE payloads are not dispatched, disabling realtime view-count updates even for the first mounted counter; the initial bindings need to be attached before the first subscribe or the channel needs a real rejoin path.
Useful? React with 👍 / 👎.
| { path: '../../public/fonts/noto-serif-sc-400.woff2', weight: '400' }, | ||
| { path: '../../public/fonts/noto-serif-sc-700.woff2', weight: '700' } |
There was a problem hiding this comment.
Use a local font that actually contains CJK glyphs
The checked-in files referenced here are not usable replacements for the removed Google Fonts stylesheet for Chinese text: fc-scan reports both WOFF2s as Noto Serif SC ExtraLight with only Latin/punctuation ranges such as 20-7e and no CJK codepoints. On systems without a matching Source Han Serif SC/思源宋体 fallback, .chinese-text and :lang(zh) content will now render in the browser’s generic serif instead of Noto Serif SC; replace these with real 400/700 CJK subsets or keep an explicit fallback strategy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94cbbd87c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| entry.pending = [] | ||
|
|
||
| for (const { event, filter, callback } of handlers) { | ||
| entry.channel.on(event, filter, callback).subscribe() |
There was a problem hiding this comment.
Register Supabase handlers before subscribing
When the first useViewData consumer mounts, registerHandler queues the only postgres_changes listener, so the subsequent initial channel.subscribe() joins with an empty binding list; after SUBSCRIBED, this line appends the listener and calls subscribe() again, but @supabase/realtime-js only sends the join payload while the channel state is closed and otherwise returns the existing channel. That means the flushed binding never gets a server id and UPDATE payloads are not dispatched, disabling realtime view-count updates even for the first mounted counter; the initial bindings need to be attached before the first subscribe or the channel needs a real rejoin path.
Useful? React with 👍 / 👎.
| { path: '../../public/fonts/noto-serif-sc-400.woff2', weight: '400' }, | ||
| { path: '../../public/fonts/noto-serif-sc-700.woff2', weight: '700' } |
There was a problem hiding this comment.
Use a local font that actually contains CJK glyphs
The checked-in files referenced here are not usable replacements for the removed Google Fonts stylesheet for Chinese text: fc-scan reports both WOFF2s as Noto Serif SC ExtraLight with only Latin/punctuation ranges such as 20-7e and no CJK codepoints. On systems without a matching Source Han Serif SC/思源宋体 fallback, .chinese-text and :lang(zh) content will now render in the browser’s generic serif instead of Noto Serif SC; replace these with real 400/700 CJK subsets or keep an explicit fallback strategy.
Useful? React with 👍 / 👎.
- P1: first consumer's handler now registers directly on channel before subscribe() so it's included in the initial join payload; only handlers arriving after subscribe() are queued in pending (fixes regression from previous commit where all handlers were deferred) - P2: revert broken local font files to next/font/google — 18KB woff2 subsets don't contain CJK glyphs
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ecbab7e44
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .floating-header { | ||
| animation: floating-header-reveal linear forwards; | ||
| animation-timeline: scroll(); | ||
| animation-range: 0px 100px; |
There was a problem hiding this comment.
Animate the scroll-title span instead of the header
In browsers that support animation-timeline: scroll() (e.g. Chrome), this rule animates every .floating-header rather than the scroll-title span. Since floating-header.js returns early in that same environment while the span is still rendered with inline opacity: 0/translateY(100%), pages passing scrollTitle such as /writing/[slug] never reveal the mobile title; pages passing only title also have the whole mobile header hidden until the page is scrolled. I checked the FloatingHeader usages under src/app/src/components; this affects the mobile header shell rather than a single route.
Useful? React with 👍 / 👎.
| if (entry.subscribeCalled) { | ||
| // subscribe() already invoked — queue for flush after SUBSCRIBED | ||
| entry.pending.push({ event, filter, callback }) |
There was a problem hiding this comment.
Register all realtime handlers before subscribing
When a second useViewData consumer mounts after the first effect has called subscribe()—for example the writing sidebar's WritingListLayout plus the post header's WritingViews—this branch only queues its callback. The queued handler is later added after SUBSCRIBED via channel.on(...).subscribe(), but Supabase postgres_changes bindings are negotiated in the initial join and subscribe() returns without issuing a new join once the channel is no longer closed, so that callback never receives a server binding id and won't fire. This regresses realtime view counts for every consumer except the first one attached to the shared channel.
Useful? React with 👍 / 👎.
| initial={{ opacity: 0, y: 8 }} | ||
| animate={{ opacity: 1, y: 0 }} | ||
| exit={{ opacity: 0, y: -8 }} | ||
| transition={{ duration: 0.2, ease: [0.25, 0.1, 0.25, 1] }} |
There was a problem hiding this comment.
Respect reduced motion for route transitions
For users with prefers-reduced-motion: reduce, every route change still runs this Framer Motion opacity/translate animation because Framer drives these values from JavaScript and its default MotionConfig does not follow the media query. The global CSS rule added in globals.css only shortens CSS animations/transitions, so the new app-wide template.tsx transition continues to animate on navigation unless it is gated with useReducedMotion or a MotionConfig setting.
Useful? React with 👍 / 👎.
| height={450} | ||
| quality="auto" | ||
| format="auto" | ||
| sizes="(max-width: 435px) 100vw, (max-width: 768px) 50vw, 33vw" |
There was a problem hiding this comment.
Match desk photo sizes to its full-width container
On 436–768px wide viewports this image is still rendered in a w-full max-w-lg container, so it occupies almost the full viewport width, but the new sizes value tells the browser/Cloudinary it is only 50vw. That causes the workspace desk photo to request an image roughly half the displayed width on common mobile/tablet widths, making it visibly soft on high-DPR screens; the breakpoint should stay full-width until the layout actually becomes a half-width column.
Useful? React with 👍 / 👎.
- P1: rewrite Supabase realtime channel lifecycle — defer subscribe() via microtask to batch synchronous mounts, rebuild channel for late-joining consumers since bindings are only in the initial join - P1: move CSS scroll-driven animation from .floating-header to .scroll-title span so the mobile header shell stays visible - P2: use useReducedMotion in route template to skip Framer Motion animations for users with prefers-reduced-motion - P2: fix desk photo sizes to 100vw on all mobile widths (container is full-width up to 768px)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 796376f3e9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!entry.subscribed) { | ||
| entry.pending = entry.pending.filter((h) => h.callback !== callback) |
There was a problem hiding this comment.
Remove subscribed handlers on unmount
When a subscribed consumer unmounts while another one stays mounted, such as the persistent writing sidebar while WritingViews changes between posts, cleanup reaches this function but only removes callbacks that are still pending. The old subscribed callback remains in channel.bindings, and each later late-join rebuild copies those stale callbacks into the new channel, so inactive realtime bindings/listeners accumulate and are negotiated with Supabase on every navigation instead of being released.
Useful? React with 👍 / 👎.
| <AnimatePresence mode="wait"> | ||
| <m.div | ||
| key={pathname} | ||
| className="flex w-full flex-1" |
There was a problem hiding this comment.
Keep persistent layouts out of the route exit tree
Because this root template wraps all nested layouts and keys the animated wrapper by the full pathname, navigating between posts such as /writing/a → /writing/b exits and remounts the entire src/app/writing/layout.js subtree, including the desktop sidebar/list that App Router layouts would otherwise preserve. That makes the sidebar flicker/lose scroll state and tears down its client subscriptions on every article navigation; animate only the page pane or a lower-level segment if the writing layout should remain persistent.
Useful? React with 👍 / 👎.
| export const contentType = 'image/png' | ||
|
|
||
| export default function Icon() { | ||
| export function GET() { |
There was a problem hiding this comment.
Restore the metadata icon default export
src/app/icon/route.js is still classified by Next as a metadata image route, not a normal route handler; the metadata route loader imports the userland module's default export as the image handler. Replacing the previous default export with GET leaves handler undefined, so requests for the generated site icon throw Default export is missing instead of returning the favicon.
Useful? React with 👍 / 👎.
Summary
Frontend optimization covering mobile adaptation, visual refinement, and performance — 10 tasks executed from the implementation plan.
Mobile Adaptation
viewportFit: 'cover'+pt-safe/pb-safe/p-safeutilities for iOS notch/Dynamic Island-webkit-tap-highlight-color: transparent+scale(0.97)on:activefor instant tap responserequestAnimationFrameJS fallback for Safari/FirefoxVisual Refinement
next/font/google— no external Google Fonts requests,font-display: swaptemplate.tsxwith AnimatePresence + motion.div for route-level fade/slidePerformance
motiontom+ LazyMotion/domAnimation (~28KB bundle reduction)sizesattributes matching custom breakpoints (390, 768)prefers-reduced-motion: reducekills all animations globallyuseRef(fixes global variable leak across renders)Test Plan
npm run build)