Add Automatic Seasonal Themes and Theme Controls - #106
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe application now supports automatic and manual seasonal themes, seasonal visual effects, localized seasonal prompts, development theme controls, reusable timetable loading, route animation data flow, and removal of Recoil and the timetable context. ChangesSeasonal theme system
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant ThemeContext
participant SeasonalEffect
participant Shuttle
participant useShuttleTimetable
participant RouteMap
App->>ThemeContext: resolve seasonal theme state
ThemeContext->>SeasonalEffect: provide resolved theme
SeasonalEffect->>App: render seasonal visual effect
Shuttle->>useShuttleTimetable: request timetable data
useShuttleTimetable->>RouteMap: provide upcoming schedules and route data
RouteMap->>App: provide typed animation flags to route visuals
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying hybus-genesis with
|
| Latest commit: |
d84c726
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2cad4ee8.hybus-genesis.pages.dev |
| Branch Preview URL: | https://dev-seasonal-theme.hybus-genesis.pages.dev |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
Pull request overview
This PR introduces automatic, date-based seasonal theming (Spring/Summer/Autumn/Winter/Christmas) while preserving the existing light/dark behavior, adding user controls for enabling/disabling seasonal themes, and keeping manual theme selection limited to development builds.
Changes:
- Added a seasonal theme model (automatic selection, normalization of legacy values, dev-only manual override persistence) and integrated it into the existing theme context + hook.
- Implemented seasonal visual effects (snow/petals/leaves/rain) and a first-visit “Seasonal themes” prompt modal.
- Removed legacy theme modal flows and cleaned up unused dependencies/assets, plus moved dev theme controls to a header debug menu.
Reviewed changes
Copilot reviewed 16 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removes lock entries for deleted dependencies (recoil/react-icons/hamt_plus). |
| src/main.tsx | Removes RecoilRoot/Snowfall usage and mounts the new SeasonalEffect. |
| src/index.css | Adds Summer/Autumn theme variables and a Summer rain animation utility. |
| src/app/context/ThemeContext.tsx | Expands theme model and adds automatic/normalized seasonal selection + storage helpers. |
| src/app/components/useDarkMode.ts | Reworks theme application/persistence to support seasonal enablement and dev-only manual overrides. |
| src/app/components/seasonal/SeasonalEffect.tsx | Adds seasonal visual effects layer (snowfall variants + summer rain). |
| src/app/components/modal/modalOpen.tsx | Replaces legacy seasonal modals with a new “Seasonal” prompt content + enable callback. |
| src/app/components/modal/modal.tsx | Adds seasonal preview wiring + seasonal footer slot and content layout variant. |
| src/app/components/lang/lang.ko.json | Removes legacy seasonal strings and adds new theme/debug/seasonal prompt strings. |
| src/app/components/lang/lang.en.json | Removes legacy seasonal strings and adds new theme/debug/seasonal prompt strings. |
| src/app/components/index.ts | Removes FAB export from components barrel. |
| src/app/components/fulltime/FullTime.tsx | Updates theme variant mapping to include summer/autumn/winter. |
| src/app/components/fab/fab.tsx | Updates FAB behavior/text for new theme states and adds seasonal on/off action. |
| src/app/components/debug/ThemeDebugMenu.tsx | Introduces dev-only header theme debug menu UI. |
| src/App.tsx | Adds first-visit seasonal prompt flow and dev-only debug menu integration. |
| public/image/snowflake.svg | Removes unused legacy asset. |
| public/image/selected.svg | Removes unused legacy asset. |
| public/image/christmas_mode_black_48dp.svg | Removes unused legacy asset. |
| public/image/autumn_orange.svg | Adds autumn leaf SVG asset for effects. |
| public/image/autumn_brown.svg | Adds autumn leaf SVG asset for effects. |
| package.json | Removes unused dependencies (recoil/react-icons). |
| eslint.config.mjs | Simplifies exhaustive-deps config after recoil removal. |
| .pnp.cjs | Updates PnP manifest to reflect dependency removals. |
Files not reviewed (1)
- .pnp.cjs: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const effectTheme = getEffectTheme(theme, manualSeasonalTheme) | ||
| const effectVisible = seasonalThemeEnabled || seasonalThemePreview |
| <ThemeDebugTrigger | ||
| type="button" | ||
| aria-label="테마 설정" | ||
| aria-controls="theme-debug-panel" | ||
| aria-expanded={isOpen} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/app/components/seasonal/SeasonalEffect.tsx (2)
11-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
100vwincludes the scrollbar width.On desktop browsers with classic scrollbars this makes the overlay wider than the viewport.
inset: 0(orwidth: '100%') with the existingposition: fixedavoids it.♻️ Proposed tweak
const effectStyle: React.CSSProperties = { zIndex: 2, position: 'fixed', - width: '100vw', - height: '100vh', + inset: 0, pointerEvents: 'none', }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/seasonal/SeasonalEffect.tsx` around lines 11 - 22, Update the effectStyle dimensions used by getEffectStyle to avoid width: '100vw', which includes the scrollbar; use inset: 0 or width: '100%' with the existing fixed positioning while preserving the overlay’s full-viewport behavior.
81-94: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winBoth image sets are always created, even when the season doesn't need them.
springImagesandautumnImagesboth run on every mount, triggering four image requests regardless ofeffectTheme. Gate theuseMemoon the resolved theme, or return an empty array when unused.♻️ Proposed refactor
- const springImages = React.useMemo( - () => [ - createImage('/image/flower_pink.png'), - createImage('/image/flower_bpink.png'), - ], - [], - ) - const autumnImages = React.useMemo( - () => [ - createImage('/image/autumn_orange.svg'), - createImage('/image/autumn_brown.svg'), - ], - [], - ) + const images = React.useMemo(() => { + if (effectTheme === THEME.SPRING) { + return [ + createImage('/image/flower_pink.png'), + createImage('/image/flower_bpink.png'), + ] + } + if (effectTheme === THEME.AUTUMN) { + return [ + createImage('/image/autumn_orange.svg'), + createImage('/image/autumn_brown.svg'), + ] + } + + return [] + }, [effectTheme])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/seasonal/SeasonalEffect.tsx` around lines 81 - 94, Update the springImages and autumnImages useMemo definitions in SeasonalEffect so each image set is created only when its corresponding resolved effectTheme is active; otherwise return an empty array. Preserve the existing image lists and memoization behavior for the selected season.src/app/components/fab/fab.tsx (2)
247-249: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHoist the repeated icon-theme expression.
metadata.dataTheme === 'light' ? 'light' : 'inverted'is duplicated at lines 248, 267, 286, 305, 323, and 341.♻️ Proposed refactor
+ const iconTheme = metadata.dataTheme === 'light' ? 'light' : 'inverted' + return ( <>then
<Icons data-theme={iconTheme}>at each site.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/fab/fab.tsx` around lines 247 - 249, Hoist the repeated metadata.dataTheme conditional into a single iconTheme value within the relevant fab component scope, then replace the duplicated data-theme expressions on each Icons instance with that value. Preserve the existing light versus inverted behavior.
144-202: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the 7-branch chain with a
Record<THEME, …>map.This compares raw strings rather than the now-exported
THEMEenum, and every new theme requires anotherelse if. A lookup table (likethemeBackgroundsinsrc/app/components/useDarkMode.ts) makes the mapping exhaustive at compile time.♻️ Sketch
const fabMetadata: Record<THEME, Record<string, string>> = { [THEME.DARK]: { changeText: 'light', changeColor: '`#374151`', iconColor: 'white', dataTheme: 'dark', imgIcon: LightImg }, [THEME.CHRISTMAS]: { changeText: 'dark', changeColor: 'var(--color-theme-main)', iconColor: 'white', dataTheme: 'christmas', imgIcon: DarkImg }, // spring / summer / autumn / winter / light … } React.useLayoutEffect(() => { const entry = fabMetadata[theme] ?? fabMetadata[THEME.LIGHT] setMetadata({ ...entry, changeText: t(entry.changeText) }) }, [t, theme])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/fab/fab.tsx` around lines 144 - 202, Replace the theme-based else-if chain in the metadata React.useLayoutEffect with an exhaustive Record<THEME, ...> lookup keyed by the exported THEME enum. Define entries for all supported themes, preserve each existing color, icon, dataTheme, and image mapping, and translate the stored changeText through t before calling setMetadata; retain THEME.LIGHT as the fallback for unsupported values.src/app/context/ThemeContext.tsx (2)
145-157: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider memoizing the context value.
The value object is recreated on every provider render, so all consumers (
SeasonalEffect,App,Fabs,FullTime) re-render even when nothing they read changed.React.useMemoover the eight fields would cut that.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/context/ThemeContext.tsx` around lines 145 - 157, Memoize the context value object passed to ThemeContext.Provider using React.useMemo with all eight exposed fields—theme, setTheme, seasonalThemeEnabled, setSeasonalThemeEnabled, manualSeasonalTheme, setManualSeasonalTheme, seasonalThemePreview, and setSeasonalThemePreview—as dependencies, then pass the memoized result to the provider.
126-143: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMove initialization into lazy state initializers.
Lines 126–136 run on every provider render: three
localStoragereads plus anIntl.DateTimeFormatconstruction andformatToPartsinsidegetResolvedTheme. The results are only used as initial state.♻️ Proposed refactor
- const storedTheme = normalizeTheme(window.localStorage.getItem('theme')) - const storedSeasonalThemeEnabled = getStoredSeasonalThemeEnabled(storedTheme) - const storedManualSeasonalTheme = getStoredManualSeasonalTheme( - storedTheme, - storedSeasonalThemeEnabled, - ) - const themeName = getResolvedTheme( - storedTheme, - storedSeasonalThemeEnabled, - storedManualSeasonalTheme, - ) - const [theme, setTheme] = React.useState<THEME>(themeName) - const [seasonalThemeEnabled, setSeasonalThemeEnabled] = - React.useState<boolean>(storedSeasonalThemeEnabled) - const [manualSeasonalTheme, setManualSeasonalTheme] = - React.useState<THEME | null>(storedManualSeasonalTheme) + const [initialState] = React.useState(() => { + const storedTheme = normalizeTheme(window.localStorage.getItem('theme')) + const enabled = getStoredSeasonalThemeEnabled(storedTheme) + const manual = getStoredManualSeasonalTheme(storedTheme, enabled) + + return { + theme: getResolvedTheme(storedTheme, enabled, manual), + enabled, + manual, + } + }) + const [theme, setTheme] = React.useState<THEME>(initialState.theme) + const [seasonalThemeEnabled, setSeasonalThemeEnabled] = + React.useState<boolean>(initialState.enabled) + const [manualSeasonalTheme, setManualSeasonalTheme] = + React.useState<THEME | null>(initialState.manual)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/context/ThemeContext.tsx` around lines 126 - 143, Move the stored-theme reads and derived initialization currently preceding the state declarations into lazy React.useState initializers within the Theme provider. Ensure localStorage access and getResolvedTheme (including its Intl.DateTimeFormat work) execute only during initial state creation, while preserving the existing initial values for theme, seasonalThemeEnabled, manualSeasonalTheme, and seasonalThemePreview.src/app/components/useDarkMode.ts (1)
203-245: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftThis one-time bootstrap runs once per
useDarkMode()consumer, not once per app.
useDarkModeis instantiated insrc/App.tsx,src/app/components/fab/fab.tsx, andsrc/app/components/fulltime/FullTime.tsx. Since the dep list is all-stable, each of those mounts re-readslocalStorage, rewrites thetheme/seasonalThemeEnabled/manualSeasonalThemekeys, re-applies body classes and rewrites the_themecookie. It is idempotent today, but a component mounting after a theme change re-derives state from storage rather than from context, which is fragile.Consider moving this bootstrap into
DarkmodeContextProvider(which already computes the same resolved values) and keepinguseDarkModeas a pure action hook.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/useDarkMode.ts` around lines 203 - 245, Move the one-time theme bootstrap currently in the useDarkMode useLayoutEffect into DarkmodeContextProvider, so it runs once per provider rather than once per hook consumer. Keep the existing normalization, seasonal/manual preference persistence, theme application, state updates, and _theme cookie behavior in the provider’s initialization flow, and leave useDarkMode responsible only for consuming context and exposing actions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.tsx`:
- Around line 280-303: Update dismissModal to store its 300ms timeout in a ref,
clearing any existing timeout before scheduling a new dismissal so stale
callbacks cannot alter a later modal cycle. Add unmount cleanup that clears the
pending timeout and resets the ref, using the component’s existing effect/ref
patterns.
In `@src/app/components/seasonal/SeasonalEffect.tsx`:
- Around line 79-96: Update SeasonalEffect so it returns early when
seasonalThemeEnabled and seasonalThemePreview are both false, preventing effect
canvases and animation loops from mounting. Move the existing useMemo hooks for
springImages and autumnImages above the guard, or apply the guard at render
sites, to preserve stable hook ordering.
---
Nitpick comments:
In `@src/app/components/fab/fab.tsx`:
- Around line 247-249: Hoist the repeated metadata.dataTheme conditional into a
single iconTheme value within the relevant fab component scope, then replace the
duplicated data-theme expressions on each Icons instance with that value.
Preserve the existing light versus inverted behavior.
- Around line 144-202: Replace the theme-based else-if chain in the metadata
React.useLayoutEffect with an exhaustive Record<THEME, ...> lookup keyed by the
exported THEME enum. Define entries for all supported themes, preserve each
existing color, icon, dataTheme, and image mapping, and translate the stored
changeText through t before calling setMetadata; retain THEME.LIGHT as the
fallback for unsupported values.
In `@src/app/components/seasonal/SeasonalEffect.tsx`:
- Around line 11-22: Update the effectStyle dimensions used by getEffectStyle to
avoid width: '100vw', which includes the scrollbar; use inset: 0 or width:
'100%' with the existing fixed positioning while preserving the overlay’s
full-viewport behavior.
- Around line 81-94: Update the springImages and autumnImages useMemo
definitions in SeasonalEffect so each image set is created only when its
corresponding resolved effectTheme is active; otherwise return an empty array.
Preserve the existing image lists and memoization behavior for the selected
season.
In `@src/app/components/useDarkMode.ts`:
- Around line 203-245: Move the one-time theme bootstrap currently in the
useDarkMode useLayoutEffect into DarkmodeContextProvider, so it runs once per
provider rather than once per hook consumer. Keep the existing normalization,
seasonal/manual preference persistence, theme application, state updates, and
_theme cookie behavior in the provider’s initialization flow, and leave
useDarkMode responsible only for consuming context and exposing actions.
In `@src/app/context/ThemeContext.tsx`:
- Around line 145-157: Memoize the context value object passed to
ThemeContext.Provider using React.useMemo with all eight exposed fields—theme,
setTheme, seasonalThemeEnabled, setSeasonalThemeEnabled, manualSeasonalTheme,
setManualSeasonalTheme, seasonalThemePreview, and setSeasonalThemePreview—as
dependencies, then pass the memoized result to the provider.
- Around line 126-143: Move the stored-theme reads and derived initialization
currently preceding the state declarations into lazy React.useState initializers
within the Theme provider. Ensure localStorage access and getResolvedTheme
(including its Intl.DateTimeFormat work) execute only during initial state
creation, while preserving the existing initial values for theme,
seasonalThemeEnabled, manualSeasonalTheme, and seasonalThemePreview.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1f1456a1-4240-44ee-b51c-f1a5103885a1
⛔ Files ignored due to path filters (16)
.yarn/cache/hamt_plus-npm-1.0.2-67a52ee1df-3680a1820b.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/react-icons-npm-5.6.0-797edf502d-571ac91977.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/recoil-npm-0.7.7-4452f58b67-4ac9dfeddd.zipis excluded by!**/.yarn/**,!**/*.zippublic/image/apple-touch-icon-120x120.pngis excluded by!**/*.pngpublic/image/apple-touch-icon-152x152.pngis excluded by!**/*.pngpublic/image/apple-touch-icon-180x180.pngis excluded by!**/*.pngpublic/image/apple-touch-icon-60x60.pngis excluded by!**/*.pngpublic/image/apple-touch-icon-76x76.pngis excluded by!**/*.pngpublic/image/autumn_brown.svgis excluded by!**/*.svgpublic/image/autumn_orange.svgis excluded by!**/*.svgpublic/image/christmas_mode_black_48dp.svgis excluded by!**/*.svgpublic/image/github_light.pngis excluded by!**/*.pngpublic/image/og-image.pngis excluded by!**/*.pngpublic/image/selected.svgis excluded by!**/*.svgpublic/image/snowflake.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
.pnp.cjseslint.config.mjspackage.jsonsrc/App.tsxsrc/app/components/debug/ThemeDebugMenu.tsxsrc/app/components/fab/fab.tsxsrc/app/components/fulltime/FullTime.tsxsrc/app/components/index.tssrc/app/components/lang/lang.en.jsonsrc/app/components/lang/lang.ko.jsonsrc/app/components/modal/modal.tsxsrc/app/components/modal/modalOpen.tsxsrc/app/components/seasonal/SeasonalEffect.tsxsrc/app/components/useDarkMode.tssrc/app/context/ThemeContext.tsxsrc/index.csssrc/main.tsx
💤 Files with no reviewable changes (3)
- src/app/components/index.ts
- package.json
- .pnp.cjs
| const effectTheme = getEffectTheme(theme, manualSeasonalTheme) | ||
| const effectVisible = seasonalThemeEnabled || seasonalThemePreview | ||
| const springImages = React.useMemo( | ||
| () => [ | ||
| createImage('/image/flower_pink.png'), | ||
| createImage('/image/flower_bpink.png'), | ||
| ], | ||
| [], | ||
| ) | ||
| const autumnImages = React.useMemo( | ||
| () => [ | ||
| createImage('/image/autumn_orange.svg'), | ||
| createImage('/image/autumn_brown.svg'), | ||
| ], | ||
| [], | ||
| ) | ||
|
|
||
| const style = getEffectStyle(effectVisible) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Effects stay mounted and animating when the user has opted out.
When seasonalThemeEnabled is false and no preview is active, getEffectTheme still resolves light/dark to a season, so a <Snowfall> canvas is mounted and its requestAnimationFrame loop keeps running — only hidden via opacity: 0. That is continuous CPU/battery cost for users who explicitly disabled seasonal themes.
Bail out early instead of relying on opacity.
🐛 Proposed fix
const effectTheme = getEffectTheme(theme, manualSeasonalTheme)
const effectVisible = seasonalThemeEnabled || seasonalThemePreview
+
+ if (!effectVisible) return nullNote the hooks below must be moved above this early return (or the guard applied at the render sites) to keep hook order stable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/components/seasonal/SeasonalEffect.tsx` around lines 79 - 96, Update
SeasonalEffect so it returns early when seasonalThemeEnabled and
seasonalThemePreview are both false, preventing effect canvases and animation
loops from mounting. Move the existing useMemo hooks for springImages and
autumnImages above the guard, or apply the guard at render sites, to preserve
stable hook ordering.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f26852ab1b
ℹ️ 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 (options?.storeTheme === false || forceAutomaticSeason) { | ||
| window.localStorage.removeItem('theme') |
There was a problem hiding this comment.
When seasonal themes are enabled in production, this removes theme from local storage, but the pre-React bootstrap in index.html still derives the initial body and status-bar colors solely from that key and defaults to white. Consequently every reload starts with a white background until the bundle applies the automatic theme, producing a conspicuous flash for themes such as Christmas; persist enough state for bootstrap or update that script to resolve the automatic theme from seasonalThemeEnabled.
Useful? React with 👍 / 👎.
| const effectTheme = getEffectTheme(theme, manualSeasonalTheme) | ||
| const effectVisible = seasonalThemeEnabled || seasonalThemePreview |
There was a problem hiding this comment.
When the user chooses to keep seasonal themes off or disables them from the FAB, effectVisible only changes the layer opacity; effectTheme still resolves to the current automatic season and the corresponding effect remains mounted. react-snowfall continues its requestAnimationFrame drawing loop while transparent, and the summer drops retain their infinite CSS animations, so the supposedly disabled feature still consumes CPU and battery; return null or explicitly pause the effect when neither enablement nor preview is active.
Useful? React with 👍 / 👎.
| ) | ||
| ? (window.localStorage.getItem('theme') as THEME) | ||
| : THEME.LIGHT | ||
| const [theme, setTheme] = React.useState<THEME>(themeName) |
There was a problem hiding this comment.
For automatic seasonal mode, the resolved date-based theme is used only as the initial value of this state and there is no timer or visibility/resume handler that updates it later. If the installed PWA remains mounted across a Seoul-time season boundary, such as May 5 or August 7, it continues displaying the previous season indefinitely until the user reloads or toggles a theme; recompute the automatic theme at the next day boundary and when the app becomes visible again.
Useful? React with 👍 / 👎.
Taewan-P
left a comment
Taewan-P
left a comment
There was a problem hiding this comment.
For the summer theme, why is it always raining? 😂 The color harmony of the theme is stunning, but in my personal opinion, if it always rain it might feel gloomy (Personal Opinion)
Maybe a sunshine glowing effect or something summer-ish might be good. Weather based might be a great idea for phase 2.
https://youtube.com/shorts/2zpPJ9qCjN8 something like this? is a sunshine glowing effect, but more faster I guess? Not a requirement. Just a suggestion.
Add the custom WebGL2 optical flare with persistent top traversal and remove the superseded R3F/Three/postprocessing stack.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/components/seasonal/SeasonalEffect.tsx (1)
71-118: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winThe opt-out guard covers only the summer branch.
Line 87 returns
nullwheneffectVisibleis false, but only forTHEME.SUMMER. The spring, autumn, winter, and christmas branches still mount<Snowfall>and keep itsrequestAnimationFrameloop running. The canvas is only hidden through the style fromgetEffectStyle(effectVisible). Users who disabled seasonal themes still pay the CPU and battery cost.All hooks are declared above line 71, so a single early return here is hook-safe.
🐛 Proposed fix
const style = getEffectStyle(effectVisible) + if (!effectVisible) return null + if (effectTheme === THEME.SPRING) {Then remove the now-redundant guard in the summer branch:
if (effectTheme === THEME.SUMMER) { - if (!effectVisible) return null - return (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/seasonal/SeasonalEffect.tsx` around lines 71 - 118, Move the effectVisible opt-out check above the theme-specific branches in SeasonalEffect, returning null before any Spring, Summer, Autumn, Winter, or Christmas effect mounts. Remove the redundant guard inside the THEME.SUMMER branch while preserving the existing hook ordering and effect rendering when enabled.
🧹 Nitpick comments (4)
src/app/components/seasonal/lens-flare/OpticalLensFlare.tsx (1)
434-441: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winThe
resolveOptionsmemo never hits, becausepropschanges identity on every render.Line 434 uses
[props]as the dependency list. React creates a newpropsobject on every render of the parent, soresolveOptionsre-runs each time andresolvedOptionsis always a new object. The layout effect at lines 436-441 therefore also runs on every render and callsrequestRenderRef.current?.(), which schedules an extra frame.
SummerLensFlareEffectpasses inline object literals forghosts,motion,source, and the other options, so this happens on every parent render.Depend on the individual props instead, or hoist the option objects in the caller to stable module-level constants.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/seasonal/lens-flare/OpticalLensFlare.tsx` around lines 434 - 441, Update the resolved-options memoization in OpticalLensFlare to depend on the individual option props rather than the unstable props object, and ensure the caller’s inline option objects such as ghosts, motion, and source are stable where needed. Keep the layout effect dependent on the resulting resolved options and error/status callbacks so requestRenderRef.current?.() only runs when actual options or callbacks change.src/app/components/seasonal/godlights/SummerGodlightRays.tsx (2)
375-437: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftNine simultaneously animated GodLights layers can be costly on low-end devices.
The component mounts one canvas per ray plus one source canvas, so
raySpecs.length + 1= 9 canvases. Each canvas renders up toMAXIMUM_RENDER_PIXELS_PER_LAYER(600,000) pixels, and each ray layer animates opacity withrepeat: InfinityandmixBlendMode: 'screen'. The compositor must blend nine full-viewport layers on every frame.Consider reducing the layer count on small viewports, or lowering
MAXIMUM_RENDER_PIXELS_PER_LAYERwhen the device pixel budget is tight. Measure on a mid-range mobile device before release.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/seasonal/godlights/SummerGodlightRays.tsx` around lines 375 - 437, Reduce the rendering cost in the SummerGodlightRays component by lowering the layer count or render pixel budget for small viewports and constrained devices. Update the ray-spec generation or MAXIMUM_RENDER_PIXELS_PER_LAYER logic used by the GodLights layers, while preserving the existing source layer and animation behavior on larger or capable displays.
300-308: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
getTimelineDelayis evaluated during render, so the animation restarts on every re-render.Line 426 calls
getTimelineDelay(spec)inside the JSX. The function readsperformance.now(), so the transition object changes on every render. Each viewport resize therefore produces a newdelayand re-seeds the looping timeline, which produces a visible opacity jump.Compute the delay once per ray, for example in a
React.useRefor aReact.useMemowith an empty dependency list, and reuse it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/seasonal/godlights/SummerGodlightRays.tsx` around lines 300 - 308, Update the ray rendering flow around getTimelineDelay so each ray computes its delay only once and reuses that stable value across re-renders. Store the result with a per-ray React useRef or empty-dependency useMemo, then pass the stored delay into the transition object instead of calling getTimelineDelay during JSX evaluation.src/index.css (1)
134-135: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused summer rain CSS.
summer-rain-falland.summer-rain-dropare no longer applied by any component; the summer effect now rendersSummerLensFlareEffectonly, so the rules betweensrc/index.csslines 220 and 328 are dead code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/index.css` around lines 134 - 135, Remove the unused summer-rain-fall and .summer-rain-drop CSS rules from src/index.css, including the dead-code block between the referenced lines. Leave the route color variables and other unrelated styles unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/app/components/seasonal/SeasonalEffect.tsx`:
- Around line 71-118: Move the effectVisible opt-out check above the
theme-specific branches in SeasonalEffect, returning null before any Spring,
Summer, Autumn, Winter, or Christmas effect mounts. Remove the redundant guard
inside the THEME.SUMMER branch while preserving the existing hook ordering and
effect rendering when enabled.
---
Nitpick comments:
In `@src/app/components/seasonal/godlights/SummerGodlightRays.tsx`:
- Around line 375-437: Reduce the rendering cost in the SummerGodlightRays
component by lowering the layer count or render pixel budget for small viewports
and constrained devices. Update the ray-spec generation or
MAXIMUM_RENDER_PIXELS_PER_LAYER logic used by the GodLights layers, while
preserving the existing source layer and animation behavior on larger or capable
displays.
- Around line 300-308: Update the ray rendering flow around getTimelineDelay so
each ray computes its delay only once and reuses that stable value across
re-renders. Store the result with a per-ray React useRef or empty-dependency
useMemo, then pass the stored delay into the transition object instead of
calling getTimelineDelay during JSX evaluation.
In `@src/app/components/seasonal/lens-flare/OpticalLensFlare.tsx`:
- Around line 434-441: Update the resolved-options memoization in
OpticalLensFlare to depend on the individual option props rather than the
unstable props object, and ensure the caller’s inline option objects such as
ghosts, motion, and source are stable where needed. Keep the layout effect
dependent on the resulting resolved options and error/status callbacks so
requestRenderRef.current?.() only runs when actual options or callbacks change.
In `@src/index.css`:
- Around line 134-135: Remove the unused summer-rain-fall and .summer-rain-drop
CSS rules from src/index.css, including the dead-code block between the
referenced lines. Leave the route color variables and other unrelated styles
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7fa29399-3907-4403-8106-6eac40faa051
⛔ Files ignored due to path filters (8)
.yarn/cache/@emotion-is-prop-valid-npm-1.4.0-36d89399d2-6fbec4d5cd.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/@emotion-memoize-npm-0.9.0-ccd80906b3-0381323593.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/framer-motion-npm-12.43.0-b47c004d75-c8013cae9f.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/godlights-npm-1.0.0-bd5649270a-7450c01836.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/motion-dom-npm-12.43.0-5ed81b11fa-ac5f7164b7.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/motion-npm-12.43.0-9cd05b659d-3c37060bd3.zipis excluded by!**/.yarn/**,!**/*.zip.yarn/cache/motion-utils-npm-12.39.0-59d768d874-5aa2972bf3.zipis excluded by!**/.yarn/**,!**/*.zipyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
.pnp.cjseslint.config.mjsindex.htmlpackage.jsonsrc/App.tsxsrc/app/components/routemap/RouteVisual.tsxsrc/app/components/seasonal/SeasonalEffect.tsxsrc/app/components/seasonal/SummerLensFlareEffect.tsxsrc/app/components/seasonal/godlights/SummerGodlightRays.tsxsrc/app/components/seasonal/lens-flare/OpticalLensFlare.tsxsrc/app/components/seasonal/lens-flare/opticalLensFlareShaders.tssrc/app/components/useDarkMode.tssrc/app/context/ThemeContext.tsxsrc/index.css
🚧 Files skipped from review as they are similar to previous changes (1)
- eslint.config.mjs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 46 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- .pnp.cjs: Generated file
Suppressed comments (6)
src/app/components/seasonal/SeasonalEffect.tsx:74
- When seasonal themes are disabled (and not in preview), the Spring effect still mounts
Snowfallwith opacity 0. That keeps the animation loop running even though the user opted out.
src/app/components/seasonal/SeasonalEffect.tsx:97 - When seasonal themes are disabled (and not in preview), the Autumn effect still mounts
Snowfallwith opacity 0, which wastes CPU/GPU while invisible.
src/app/components/seasonal/SeasonalEffect.tsx:110 - When seasonal themes are disabled (and not in preview), the Winter/Christmas
Snowfalleffect still mounts with opacity 0, so the effect keeps running even though it's not meant to be active.
index.html:103 '%DEV%'is not replaced by Vite inindex.html(no HTML injection plugin is configured), so the bootstrap theme-color/background logic will never applymanualSeasonalThemeon reload in development. This undermines the PR goal of preserving manual theme selection across reloads inyarn dev.
const manual = '%DEV%' === 'true' && enabled
? normalize(window.localStorage.getItem('manualSeasonalTheme')) || storedSeason
: null
eslint.config.mjs:70
- This file-specific
react/no-unknown-propertyignore list allowsdispose/object, but those props aren't used anywhere undersrc/app/components/seasonalright now. Keeping this override can mask real typos in JSX props.
rules: {
'react/no-unknown-property': [
'error',
{
ignore: ['dispose', 'object'],
src/app/components/fab/fab.tsx:131
handleSeasonalThemeOnClickreturns a Promise that never resolves/rejects. Ifreact-tiny-fabawaits the returned promise, this can stall its internal state handling (same pattern exists elsewhere, but this instance is newly introduced).
const handleSeasonalThemeOnClick = (): Promise<React.FC> => {
return new Promise(() => {
toggleSeasonalTheme()
})
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 46 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- .pnp.cjs: Generated file
Suppressed comments (2)
src/app/components/fab/fab.tsx:131
- This handler returns a Promise that never resolves/rejects (the executor never calls
resolve). Ifreact-tiny-fabawaits the promise to decide when to close animations/menus, this can leave it waiting indefinitely and also creates unnecessary allocations.
Return a resolved promise after toggling (or make the handler synchronous if the library allows).
const handleSeasonalThemeOnClick = (): Promise<React.FC> => {
return new Promise(() => {
toggleSeasonalTheme()
})
}
index.html:105
%DEV%is not a Vite index.html replacement token in this repo (vite.config.ts doesn’t transform index.html), so this condition will always be false and the pre-hydration theme will never respect the stored dev-onlymanualSeasonalTheme. That makes the initial theme-color/background incorrect on reload in dev and can cause a visible flash before React applies the manual theme.
Use a real dev signal in this inline script (e.g., hostname check) so the initial paint matches the React-side dev behavior.
const enabled = preference === 'true' ||
(preference !== 'false' && storedSeason !== null)
const manual = '%DEV%' === 'true' && enabled
? normalize(window.localStorage.getItem('manualSeasonalTheme')) || storedSeason
: null
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 52 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- .pnp.cjs: Generated file
Suppressed comments (2)
src/app/components/debug/ThemeDebugMenu.tsx:155
ThemeDebugMenumixes i18n-driven labels (e.g.,t('theme_auto'),t('dark')) with hard-coded Korean strings for visible text and ARIA labels (e.g.,aria-label="테마 설정", panel title/description). This makes the debug UI partially untranslated when the app is in English and can be confusing for screen readers. Recommend moving these strings into i18next resources (or at least deriving them fromt(...)) for consistency.
return (
<ThemeDebugRoot ref={rootRef}>
<ThemeDebugTrigger
type="button"
aria-label="테마 설정"
aria-controls="theme-debug-panel"
aria-expanded={isOpen}
onClick={() => setIsOpen((open) => !open)}
>
테마<span className="hsm:hidden"> 설정</span>
</ThemeDebugTrigger>
{isOpen && (
<ThemeDebugPanel id="theme-debug-panel" aria-label="테마 설정 디버그">
<ThemeDebugHeader>
<div>
<ThemeDebugTitle>테마 미리보기</ThemeDebugTitle>
<ThemeDebugDescription>
개발 모드에서만 표시되는 선택 메뉴입니다.
</ThemeDebugDescription>
</div>
<ThemeDebugClose
type="button"
aria-label="테마 설정 닫기"
onClick={() => setIsOpen(false)}
>
×
</ThemeDebugClose>
</ThemeDebugHeader>
src/app/components/seasonal/SeasonalEffect.tsx:69
SeasonalEffecteagerly createsHTMLImageElements (and triggers asset fetches) for Spring/Autumn on every mount, even when seasonal themes are disabled. Also, whenseasonalThemeEnabledis false it can still renderSnowfall(withopacity: 0), so the animation work continues while invisible. Consider short-circuiting wheneffectVisibleis false and only creating images when the corresponding effect is actually active.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/app/components/seasonal/godlights/staticGodLights.client.ts (1)
41-46: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDelay worker termination to avoid boot churn on resize.
releaseIdleWorkerterminates the worker the moment the last render resolves.SummerGodlightRays.tsxmounts 9StaticGodLightsinstances, and every settled viewport resize regenerates all 9 scenes. Each burst therefore pays a full worker boot: module fetch, module evaluation, and thesupportsRequiredCanvasFeaturesprobe.Keep the worker for a short idle window instead.
♻️ Proposed change
+const WORKER_IDLE_TIMEOUT = 5000 +let idleTimer: ReturnType<typeof setTimeout> | null = null + +const clearIdleTimer = () => { + if (idleTimer === null) return + + clearTimeout(idleTimer) + idleTimer = null +} + const releaseIdleWorker = () => { if (pendingRenders.size > 0 || !renderWorker) return - renderWorker.terminate() - renderWorker = null + clearIdleTimer() + idleTimer = setTimeout(() => { + idleTimer = null + if (pendingRenders.size > 0 || !renderWorker) return + + renderWorker.terminate() + renderWorker = null + }, WORKER_IDLE_TIMEOUT) }Call
clearIdleTimer()at the start ofdisableWorkerand inrenderStaticGodLightsbefore the worker is reused.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/seasonal/godlights/staticGodLights.client.ts` around lines 41 - 46, Update releaseIdleWorker and the worker lifecycle around disableWorker and renderStaticGodLights to defer termination through a short idle timer instead of terminating immediately when pendingRenders becomes empty. Clear the existing idle timer at the start of disableWorker and before reusing the worker in renderStaticGodLights, preserving immediate termination only after the idle window expires.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/components/seasonal/godlights/StaticGodLights.tsx`:
- Around line 35-40: In the StaticGodLights render flow, validate
canvasRef.current before invoking renderStaticGodLights(scene). Move the
renderStaticGodLights call after the canvas null check, preserving the existing
fallback return, so no worker request is created when the canvas is unavailable.
In `@src/app/components/seasonal/lens-flare/OpticalLensFlare.tsx`:
- Around line 268-281: Remove the false terminal status emitted during context
restoration: in src/app/components/seasonal/lens-flare/OpticalLensFlare.tsx
lines 268-281, remove callbacks.onStatus('destroyed') from
handleContextRestored; in
src/app/components/seasonal/lens-flare/opticalLensFlare.worker.ts lines 98-113,
remove postStatus('destroyed') from the corresponding restoration handler. Keep
the restored, ready, and recovery flow unchanged.
In `@src/app/components/seasonal/lens-flare/opticalLensFlareRenderer.ts`:
- Around line 160-198: Update the ghostData lookup in getUniformLocations to
request the base uniform name uGhostData instead of the indexed name
uGhostData[0], while keeping getUniform’s strict null validation and all other
uniform lookups unchanged.
---
Nitpick comments:
In `@src/app/components/seasonal/godlights/staticGodLights.client.ts`:
- Around line 41-46: Update releaseIdleWorker and the worker lifecycle around
disableWorker and renderStaticGodLights to defer termination through a short
idle timer instead of terminating immediately when pendingRenders becomes empty.
Clear the existing idle timer at the start of disableWorker and before reusing
the worker in renderStaticGodLights, preserving immediate termination only after
the idle window expires.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 23437397-060b-4a66-b6a4-1da01ce97412
📒 Files selected for processing (8)
src/app/components/seasonal/godlights/StaticGodLights.tsxsrc/app/components/seasonal/godlights/SummerGodlightRays.tsxsrc/app/components/seasonal/godlights/staticGodLights.client.tssrc/app/components/seasonal/godlights/staticGodLights.worker.tssrc/app/components/seasonal/lens-flare/OpticalLensFlare.tsxsrc/app/components/seasonal/lens-flare/opticalLensFlare.worker.tssrc/app/components/seasonal/lens-flare/opticalLensFlareRenderer.tssrc/app/components/seasonal/lens-flare/opticalLensFlareTypes.ts
| const canvas = canvasRef.current | ||
| const render = renderStaticGodLights(scene) | ||
| if (!canvas || !render) { | ||
| setUseMainThreadFallback(true) | ||
| return | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Start the render only after the canvas check.
Line 36 starts the worker render before line 37 validates canvas. If canvas is null and render is non-null, the early return abandons the request. The entry stays in the module-level pendingRenders map in staticGodLights.client.ts. Two consequences follow:
releaseIdleWorkernever terminates the worker, because it requirespendingRenders.size === 0.- The worker still resolves the request. The returned
ImageBitmapis never drawn and never closed.
Check the canvas first, then start the render.
🐛 Proposed fix
const canvas = canvasRef.current
- const render = renderStaticGodLights(scene)
- if (!canvas || !render) {
+ if (!canvas) {
+ setUseMainThreadFallback(true)
+ return
+ }
+
+ const render = renderStaticGodLights(scene)
+ if (!render) {
setUseMainThreadFallback(true)
return
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const canvas = canvasRef.current | |
| const render = renderStaticGodLights(scene) | |
| if (!canvas || !render) { | |
| setUseMainThreadFallback(true) | |
| return | |
| } | |
| const canvas = canvasRef.current | |
| if (!canvas) { | |
| setUseMainThreadFallback(true) | |
| return | |
| } | |
| const render = renderStaticGodLights(scene) | |
| if (!render) { | |
| setUseMainThreadFallback(true) | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/components/seasonal/godlights/StaticGodLights.tsx` around lines 35 -
40, In the StaticGodLights render flow, validate canvasRef.current before
invoking renderStaticGodLights(scene). Move the renderStaticGodLights call after
the canvas null check, preserving the existing fallback return, so no worker
request is created when the canvas is unavailable.
| const handleContextRestored = (): void => { | ||
| callbacks.onStatus('restored') | ||
|
|
||
| try { | ||
| renderer.restore() | ||
| renderer.resetLastFrameTime(toAbsoluteLensFlareTime(performance.now())) | ||
| contextLost = false | ||
| callbacks.onStatus('destroyed') | ||
| callbacks.onStatus('ready') | ||
| requestFrame() | ||
| } catch (error) { | ||
| callbacks.onError(error) | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Both lens flare backends publish a false 'destroyed' status during context restoration. Each handleContextRestored emits 'restored', then 'destroyed', then 'ready', but the renderer is alive after renderer.restore() succeeds. 'destroyed' is the terminal status used by the effect cleanup, so a consumer of onStatusChange stops tracking a flare that recovered.
src/app/components/seasonal/lens-flare/OpticalLensFlare.tsx#L268-L281: remove thecallbacks.onStatus('destroyed')call between thecontextLost = falseassignment andcallbacks.onStatus('ready').src/app/components/seasonal/lens-flare/opticalLensFlare.worker.ts#L98-L113: remove thepostStatus('destroyed')call between thecontextLost = falseassignment andpostStatus('ready').
📍 Affects 2 files
src/app/components/seasonal/lens-flare/OpticalLensFlare.tsx#L268-L281(this comment)src/app/components/seasonal/lens-flare/opticalLensFlare.worker.ts#L98-L113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/components/seasonal/lens-flare/OpticalLensFlare.tsx` around lines 268
- 281, Remove the false terminal status emitted during context restoration: in
src/app/components/seasonal/lens-flare/OpticalLensFlare.tsx lines 268-281,
remove callbacks.onStatus('destroyed') from handleContextRestored; in
src/app/components/seasonal/lens-flare/opticalLensFlare.worker.ts lines 98-113,
remove postStatus('destroyed') from the corresponding restoration handler. Keep
the restored, ready, and recovery flow unchanged.
| const getUniform = ( | ||
| gl: WebGL2RenderingContext, | ||
| program: WebGLProgram, | ||
| name: string, | ||
| ): WebGLUniformLocation => { | ||
| const location = gl.getUniformLocation(program, name) | ||
|
|
||
| if (location === null) { | ||
| throw new Error(`Lens flare uniform is unavailable: ${name}`) | ||
| } | ||
|
|
||
| return location | ||
| } | ||
|
|
||
| const getUniformLocations = ( | ||
| gl: WebGL2RenderingContext, | ||
| program: WebGLProgram, | ||
| ): UniformLocations => ({ | ||
| resolution: getUniform(gl, program, 'uResolution'), | ||
| time: getUniform(gl, program, 'uTime'), | ||
| intensity: getUniform(gl, program, 'uIntensity'), | ||
| source: getUniform(gl, program, 'uSource'), | ||
| opticalCenter: getUniform(gl, program, 'uOpticalCenter'), | ||
| coreColor: getUniform(gl, program, 'uCoreColor'), | ||
| haloColor: getUniform(gl, program, 'uHaloColor'), | ||
| rayColor: getUniform(gl, program, 'uRayColor'), | ||
| streakColor: getUniform(gl, program, 'uStreakColor'), | ||
| ghostColorA: getUniform(gl, program, 'uGhostColorA'), | ||
| ghostColorB: getUniform(gl, program, 'uGhostColorB'), | ||
| sourceStyle: getUniform(gl, program, 'uSourceStyle'), | ||
| rayStyle: getUniform(gl, program, 'uRayStyle'), | ||
| rayAngle: getUniform(gl, program, 'uRayAngle'), | ||
| streakStyle: getUniform(gl, program, 'uStreakStyle'), | ||
| ghostStyle: getUniform(gl, program, 'uGhostStyle'), | ||
| ghostAppearance: getUniform(gl, program, 'uGhostAppearance'), | ||
| ghostRingIntensity: getUniform(gl, program, 'uGhostRingIntensity'), | ||
| ghostMotion: getUniform(gl, program, 'uGhostMotion'), | ||
| ghostData: getUniform(gl, program, 'uGhostData[0]'), | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: List uniform declarations and their reference counts in the lens flare shaders.
set -euo pipefail
fd -t f 'opticalLensFlareShaders.ts' | while IFS= read -r file; do
echo "== $file =="
rg -n 'uniform\s+\w+\s+u\w+' "$file"
echo "-- reference counts --"
rg -o 'u[A-Z]\w*' "$file" | sort | uniq -c | sort -rn
doneRepository: BusHanyang/hybus-genesis
Length of output: 1257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
fd -t f 'opticalLensFlareRenderer.ts|opticalLensFlareShaders.ts|opticalLensFlare' .
echo "== outline renderer =="
ast-grep outline src/app/components/seasonal/lens-flare/opticalLensFlareRenderer.ts --view expanded || true
echo "== renderer relevant sections =="
wc -l src/app/components/seasonal/lens-flare/opticalLensFlareRenderer.ts
sed -n '1,260p' src/app/components/seasonal/lens-flare/opticalLensFlareRenderer.ts
echo "== shader relevant sections =="
sed -n '1,140p' src/app/components/seasonal/lens-flare/opticalLensFlareShaders.tsRepository: BusHanyang/hybus-genesis
Length of output: 13773
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Static probe: compare declarations in shader source with getUniformLocations keys and references.
python3 - <<'PY'
from pathlib import Path
import re
src = Path('src/app/components/seasonal/lens-flare/opticalLensFlareShaders.ts').read_text()
renderer = Path('src/app/components/seasonal/lens-flare/opticalLensFlareRenderer.ts').read_text()
decls = sorted(re.findall(r'(?m)^(\s*)uniform\s+(\S+)\s+(u\w+)(?:\[[^\]]+\])?\s*;', src))
refs = sorted(re.findall(r'\bu[A-Z]\w*\b', src))
print("decl_count:", len(decls))
decl_names = [name for _, _, name in decls]
print("decl_names:", decl_names)
print("reference_counts:")
from collections import Counter
for name, count in Counter(refs).most_common():
print(f" {name}: {count}")
print("all_decl_names_referenced:", all(name in refs for name in decl_names))
# Extract UniformLocations fields and getUniformLocations calls.
uni_type = re.findall(r"type UniformLocations = Readonly<\{.*?)\n\}$", renderer, re.S)
print("UniformLocations match:", bool(uni_type))
if uni_type:
fields = re.findall(r"^\s*(\w+): .*?$", uni_type[0], re.M)
print("UniformLocations fields:", fields)
locations_fn = re.search(r"const getUniformLocations[\s\S]*?=>[\s\S]*?\)", renderer)
print(locations_fn_found:", bool(locations_fn))
if locations_fn:
print(locations_fn group count:", len(locations_fn.groups()))
pyRepository: BusHanyang/hybus-genesis
Length of output: 385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== draw and update paths =="
sed -n '332,465p' src/app/components/seasonal/lens-flare/opticalLensFlareRenderer.ts
echo "== program creation errors =="
rg -n "LensFlareShader|program linking failed|Unable to allocate" src/app/components/seasonal/lens-flare/opticalLensFlareRenderer.ts
echo "== worker resources usage =="
sed -n '1,260p' src/app/components/seasonal/lens-flare/opticalLensFlare.worker.tsRepository: BusHanyang/hybus-genesis
Length of output: 10371
🌐 Web query:
WebGL getUniformLocation returns null inactive uniform gl.uniform accepts null location
💡 Result:
In WebGL, the getUniformLocation method returns null if a uniform variable is not found [1]. This most commonly occurs if the uniform is "inactive"—meaning the shader compiler and linker determined that the variable has no observable effect on the shader's output and subsequently optimized it out [2][3]. When you attempt to pass a null value (the result of getUniformLocation for an inactive or non-existent uniform) into any gl.uniform* function, the WebGL implementation will silently ignore the call [4][5][6]. No uniform variables will be updated, and no error will be generated [4][5][7]. This behavior is a deliberate design choice in WebGL to simplify application code [4][5]. It allows developers to attempt to update a collection of uniforms without needing to manually verify if each one exists in the linked shader program, as the API gracefully handles the null cases [5][6].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/WebGLRenderingContext/getUniformLocation
- 2: https://stackoverflow.com/questions/75578339/glsl-vertex-shader-glgetuniformlocation-return-null
- 3: https://stackoverflow.com/questions/56883308/glgetuniformlocation-returns-1-even-though-i-used-the-variable-in-shader
- 4: https://www.khronos.org/webgl/public-mailing-list/public_webgl/1012/msg00056.php
- 5: https://www.khronos.org/webgl/public-mailing-list/public_webgl/1005/msg00009.php
- 6: https://greggman.github.io/webgl-lint/
- 7: https://searchfox.org/firefox-main/source/dom/canvas/test/webgl-conf/checkout/conformance/uniforms/null-uniform-location.html
Avoid a null uniform location at uGhostData[0].
All active declarations are referenced, but array indices can be driver-sensitive; using the base name uGhostData avoids getUniformLocation returning null for the lookup while keeping other uniform lookups strict.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/components/seasonal/lens-flare/opticalLensFlareRenderer.ts` around
lines 160 - 198, Update the ghostData lookup in getUniformLocations to request
the base uniform name uGhostData instead of the indexed name uGhostData[0],
while keeping getUniform’s strict null validation and all other uniform lookups
unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/components/shuttle/Shuttle.tsx (1)
218-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive
timetableAliveduring render instead of storing it in state.The value depends only on render inputs. The effect adds an extra render after data arrives, and it re-runs every second because
upcomingTimetablegets a new reference on each tick.♻️ Proposed refactor
const [touched, setTouched] = useState<boolean>(false) const [infoClosed, setInfoClosed] = useState<boolean>( window.localStorage.getItem('touch_info') === 'closed', ) - const [timetableAlive, setTimetableAlive] = useState<boolean>(true) - - // For info card to not show when error or no shuttle available - useEffect(() => { - if ( - timetable.data?.length === 0 || - timetable.status !== 'success' || - upcomingTimetable.length === 0 - ) { - setTimetableAlive(false) - } else { - setTimetableAlive(true) - } - }, [timetable.data, timetable.status, upcomingTimetable]) + + // For info card to not show when error or no shuttle available + const timetableAlive = + timetable.status === 'success' && + timetable.data.length > 0 && + upcomingTimetable.length > 0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/shuttle/Shuttle.tsx` around lines 218 - 235, Replace the timetableAlive state and synchronization effect with a render-time derived boolean based on timetable.data, timetable.status, and upcomingTimetable. Preserve the current condition: it should be false for empty or unsuccessful timetables or when no upcoming entries exist, and true otherwise; update all consumers to use the derived value.src/app/components/routemap/RouteMap.tsx (1)
69-70: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winReplace the unchecked
as StopLocationcast with a runtime check.
props.tabis a stored string fromwindow.localStorage, andRouteMappasses it intostopAPIviauseShuttleTimetable. A stale local-storage value bypassesstopLocationKeys, causing the timetable query key to include invalid string data instead of a declared stop location. Narrow the value againststopLocationKeysbefore the cast and use an empty value when it does not match.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/components/routemap/RouteMap.tsx` around lines 69 - 70, In RouteMap, validate props.tab against stopLocationKeys before passing it to useShuttleTimetable; cast only the validated matching value to StopLocation and supply the established empty value for stale or invalid local-storage data. Keep useDotAnimation aligned with the resulting routeTimetable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/components/shuttle/useShuttleTimetable.ts`:
- Around line 129-148: Update the useShuttleTimetable derivation around
upcomingTimetable and routeTimetable to key filtering on the number of schedules
whose times have departed, rather than currentTime, so both array references
remain stable between timetable-boundary changes while preserving the one-second
countdown tick elsewhere. First verify the timetable is sorted ascending by time
using the existing shuttleAPI/sorting sources; if not, establish the required
ordering before deriving the departed-count key. Keep the current filtering and
route-pair behavior unchanged, and do not alter the independent ticker ownership
unless implementing a shared context or store is explicitly required.
---
Nitpick comments:
In `@src/app/components/routemap/RouteMap.tsx`:
- Around line 69-70: In RouteMap, validate props.tab against stopLocationKeys
before passing it to useShuttleTimetable; cast only the validated matching value
to StopLocation and supply the established empty value for stale or invalid
local-storage data. Keep useDotAnimation aligned with the resulting
routeTimetable.
In `@src/app/components/shuttle/Shuttle.tsx`:
- Around line 218-235: Replace the timetableAlive state and synchronization
effect with a render-time derived boolean based on timetable.data,
timetable.status, and upcomingTimetable. Preserve the current condition: it
should be false for empty or unsuccessful timetables or when no upcoming entries
exist, and true otherwise; update all consumers to use the derived value.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c86ac5f8-dbfc-447e-8f85-5f229e5ab3b5
📒 Files selected for processing (10)
src/app/components/routemap/DotAnimation.tsxsrc/app/components/routemap/RouteMap.tsxsrc/app/components/routemap/RouteVisual.tsxsrc/app/components/seasonal/SummerLensFlareEffect.tsxsrc/app/components/seasonal/godlights/StaticGodLights.tsxsrc/app/components/seasonal/lens-flare/opticalLensFlareShaders.tssrc/app/components/shuttle/Shuttle.tsxsrc/app/components/shuttle/useShuttleTimetable.tssrc/app/context/TimeTableContext.tsxsrc/main.tsx
💤 Files with no reviewable changes (1)
- src/app/context/TimeTableContext.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/components/seasonal/SummerLensFlareEffect.tsx
- src/app/components/seasonal/lens-flare/opticalLensFlareShaders.ts
|
Check out your Lighthouse Report: https://lighthouse.hybus.app/app/projects/bushanyang-production/dashboard |
Describe your Pull Request
Add automatic seasonal themes while preserving the existing light and dark theme behavior.
Main changes
yarn dev.Verification
yarn install --immutable --immutable-cacheyarn validateyarn exec tsc --noEmit --noUnusedLocals --noUnusedParameters --pretty falseyarn buildAdditional content
frozenpreferences are normalized to the Winter theme.yarn devto access the theme debugging controls.Summary by CodeRabbit
New Features
Improvements