Repository navigation
feat(ui): animate loading bar with traveling wave - #332
Conversation
Replace the static pixel-art battery SVG with a smooth traveling wave animation. Each of the 6 segments oscillates between 30-100% fill height using eased sine waves with phase offsets, creating a fluid wave effect. - Use requestAnimationFrame + refs for 60fps without React re-renders - Apply ease-in-out-cubic to raw sine for natural motion - Respect prefers-reduced-motion via useReducedMotion hook - Add cloudflare tunnel host to vite allowedHosts for mobile testing Files: src/icons/LoadingBar.tsx, vite.config.ts
Deploying wallet-bitcoin with
|
| Latest commit: |
a22fc35
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2192e9f6.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://animate-loading-bar.wallet-bitcoin.pages.dev |
WalkthroughAdds a data-driven, animated LoadingBar React SVG component using per-segment RAF animation with reduced-motion support; updates Vite dev server to allow hosts matching Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 wallet-mutinynet with
|
| Latest commit: |
a22fc35
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b83d789d.arkade-wallet.pages.dev |
| Branch Preview URL: | https://animate-loading-bar.arkade-wallet.pages.dev |
… hook framer-motion is not a project dependency. Replace the import with a lightweight inline hook using window.matchMedia to detect the prefers-reduced-motion preference.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/icons/LoadingBar.tsx (1)
45-45: UseSEGMENTS.lengthinstead of hard-coded6.Line 45 should derive loop bounds from the data source to prevent drift during future edits.
♻️ Proposed change
- for (let i = 0; i < 6; i++) { + for (let i = 0; i < SEGMENTS.length; i++) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/icons/LoadingBar.tsx` at line 45, The loop in LoadingBar that currently uses a hard-coded bound (for (let i = 0; i < 6; i++)) should use the SEGMENTS data source length to avoid drift; update the loop to iterate up to SEGMENTS.length (reference the SEGMENTS constant and the loop inside the LoadingBar component) so additions/removals to SEGMENTS automatically adjust the loop count.vite.config.ts (1)
20-20: Scope tunnel host allowlisting to explicit dev sessions.Line 20 permanently allows any
*.trycloudflare.comhost while line 19 exposes the dev server on all interfaces. Consider env-scoping the tunnel host allowlist to make it opt-in rather than always active, following the pattern used on line 15 withprocess.env.HTTPS.♻️ Suggested refactor
server: { port: 3002, host: true, - allowedHosts: ['.trycloudflare.com'], + allowedHosts: process.env.CF_TUNNEL_HOST ? [process.env.CF_TUNNEL_HOST] : [], },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@vite.config.ts` at line 20, Scope the hardcoded allowedHosts entry so the trycloudflare tunnel is opt-in: instead of always setting allowedHosts: ['.trycloudflare.com'], guard it behind an environment check (similar to the existing process.env.HTTPS pattern) and only return the array when the env var (e.g. process.env.USE_TUNNEL or process.env.DEV_TUNNEL) is truthy; otherwise omit or set allowedHosts to undefined/[] — also ensure this check pairs with the dev server host setting (server.host) that exposes all interfaces so the tunnel is only permitted for explicit dev sessions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/icons/LoadingBar.tsx`:
- Around line 39-42: When shouldReduce flips to true inside the LoadingBar
animation block, the current code cancels the RAF but leaves segment
heights/opacities mid-animation; update the if (shouldReduce) branch in
LoadingBar.tsx to (1) cancelAnimationFrame(rafRef.current), (2) clear
rafRef.current, and (3) reset all battery segment elements/state (e.g., via
existing segment refs or a helper like resetSegmentsToFullBattery) to full
height and full opacity so the static full-battery appearance is restored.
Ensure this runs synchronously before returning so no partial-filled visuals
remain.
- Around line 1-2: The file imports useReducedMotion from 'framer-motion' which
causes a CI unresolved-import; remove that external dependency by replacing the
framer-motion call with a small native hook (e.g., implement
usePrefersReducedMotion using window.matchMedia('(prefers-reduced-motion:
reduce)') and subscribing to its change events) and use that hook in the
LoadingBar component instead of useReducedMotion; update any references to
useReducedMotion in this file (and keep existing useEffect/useRef usage) so
behavior is identical while avoiding adding framer-motion to package.json.
---
Nitpick comments:
In `@src/icons/LoadingBar.tsx`:
- Line 45: The loop in LoadingBar that currently uses a hard-coded bound (for
(let i = 0; i < 6; i++)) should use the SEGMENTS data source length to avoid
drift; update the loop to iterate up to SEGMENTS.length (reference the SEGMENTS
constant and the loop inside the LoadingBar component) so additions/removals to
SEGMENTS automatically adjust the loop count.
In `@vite.config.ts`:
- Line 20: Scope the hardcoded allowedHosts entry so the trycloudflare tunnel is
opt-in: instead of always setting allowedHosts: ['.trycloudflare.com'], guard it
behind an environment check (similar to the existing process.env.HTTPS pattern)
and only return the array when the env var (e.g. process.env.USE_TUNNEL or
process.env.DEV_TUNNEL) is truthy; otherwise omit or set allowedHosts to
undefined/[] — also ensure this check pairs with the dev server host setting
(server.host) that exposes all interfaces so the tunnel is only permitted for
explicit dev sessions.
| if (shouldReduce) { | ||
| cancelAnimationFrame(rafRef.current) | ||
| return | ||
| } |
There was a problem hiding this comment.
Reduced-motion toggle can leave a partially filled battery.
When shouldReduce becomes true after animation has started, the loop stops but current segment heights/opacities are not restored. This breaks the “static full battery” behavior.
✅ Proposed fix
useEffect(() => {
if (shouldReduce) {
cancelAnimationFrame(rafRef.current)
+ for (let i = 0; i < SEGMENTS.length; i++) {
+ const bodyEl = rectsRef.current[i]
+ const shadowEl = shadowsRef.current[i]
+ if (!bodyEl) continue
+ bodyEl.setAttribute('height', String(BODY_H))
+ bodyEl.setAttribute('y', String(BODY_Y))
+ if (shadowEl) shadowEl.style.opacity = '1'
+ }
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.
| if (shouldReduce) { | |
| cancelAnimationFrame(rafRef.current) | |
| return | |
| } | |
| if (shouldReduce) { | |
| cancelAnimationFrame(rafRef.current) | |
| for (let i = 0; i < SEGMENTS.length; i++) { | |
| const bodyEl = rectsRef.current[i] | |
| const shadowEl = shadowsRef.current[i] | |
| if (!bodyEl) continue | |
| bodyEl.setAttribute('height', String(BODY_H)) | |
| bodyEl.setAttribute('y', String(BODY_Y)) | |
| if (shadowEl) shadowEl.style.opacity = '1' | |
| } | |
| return | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/icons/LoadingBar.tsx` around lines 39 - 42, When shouldReduce flips to
true inside the LoadingBar animation block, the current code cancels the RAF but
leaves segment heights/opacities mid-animation; update the if (shouldReduce)
branch in LoadingBar.tsx to (1) cancelAnimationFrame(rafRef.current), (2) clear
rafRef.current, and (3) reset all battery segment elements/state (e.g., via
existing segment refs or a helper like resetSegmentsToFullBattery) to full
height and full opacity so the static full-battery appearance is restored.
Ensure this runs synchronously before returning so no partial-filled visuals
remain.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/icons/LoadingBar.tsx (1)
51-54:⚠️ Potential issue | 🟠 MajorReduced-motion toggle still leaves segments in a partial-fill state.
When
shouldReduceflips totruemid-animation the RAF is cancelled but the body rects'height/yand shadows'opacityare left at whatever values the last frame wrote. The "static full battery" behaviour described in the PR is therefore broken whenever reduced motion is toggled on after the animation has started.🛠️ Proposed fix — reset all segments synchronously before returning
if (shouldReduce) { cancelAnimationFrame(rafRef.current) + for (let i = 0; i < SEGMENTS.length; i++) { + const bodyEl = rectsRef.current[i] + const shadowEl = shadowsRef.current[i] + if (!bodyEl) continue + bodyEl.setAttribute('height', String(BODY_H)) + bodyEl.setAttribute('y', String(BODY_Y)) + if (shadowEl) shadowEl.style.opacity = '1' + } return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/icons/LoadingBar.tsx` around lines 51 - 54, When shouldReduce becomes true we currently cancel rafRef.current but leave SVG rects/shadows at mid-frame values; update the shouldReduce branch inside the LoadingBar animation logic to synchronously reset all segment elements to the "static full battery" state before returning: cancel the RAF (rafRef.current), then iterate the same segment collections used by the animator (e.g., bodyRectsRef / shadowRectsRef or the arrays used in applyFrame/animateFrame) and set each body rect's height/y and each shadow's opacity to the final static values the reduced-motion UI expects, then return. Ensure you reference and mutate the same element refs/arrays used by the animation so the DOM is deterministically reset when reduced motion is toggled.
🧹 Nitpick comments (1)
src/icons/LoadingBar.tsx (1)
57-57: UseSEGMENTS.lengthinstead of the magic number6.The loop bound is hardcoded to
6, which will silently go out of sync if theSEGMENTSarray is ever extended or trimmed.♻️ Proposed change
- for (let i = 0; i < 6; i++) { + for (let i = 0; i < SEGMENTS.length; i++) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/icons/LoadingBar.tsx` at line 57, Replace the hardcoded loop bound 6 with SEGMENTS.length in the for loop inside LoadingBar.tsx so the iteration uses the actual SEGMENTS array length; update the loop that currently reads "for (let i = 0; i < 6; i++)" to reference SEGMENTS.length to prevent it getting out of sync when SEGMENTS is changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/icons/LoadingBar.tsx`:
- Around line 51-54: When shouldReduce becomes true we currently cancel
rafRef.current but leave SVG rects/shadows at mid-frame values; update the
shouldReduce branch inside the LoadingBar animation logic to synchronously reset
all segment elements to the "static full battery" state before returning: cancel
the RAF (rafRef.current), then iterate the same segment collections used by the
animator (e.g., bodyRectsRef / shadowRectsRef or the arrays used in
applyFrame/animateFrame) and set each body rect's height/y and each shadow's
opacity to the final static values the reduced-motion UI expects, then return.
Ensure you reference and mutate the same element refs/arrays used by the
animation so the DOM is deterministically reset when reduced motion is toggled.
---
Nitpick comments:
In `@src/icons/LoadingBar.tsx`:
- Line 57: Replace the hardcoded loop bound 6 with SEGMENTS.length in the for
loop inside LoadingBar.tsx so the iteration uses the actual SEGMENTS array
length; update the loop that currently reads "for (let i = 0; i < 6; i++)" to
reference SEGMENTS.length to prevent it getting out of sync when SEGMENTS is
changed.
Summary
requestAnimationFrame+ refs for 60fps animation without React re-renders per frameprefers-reduced-motionvia framer-motion'suseReducedMotionhookallowedHostsfor mobile HTTPS testingDetails
The animation applies
ease-in-out-cubiceasing to a raw sine wave for each segment, with staggered phase offsets (0.45 per segment) creating the traveling wave. Shadow opacity tracks fill level for depth. No new dependencies — uses existingframer-motionfor reduced-motion detection only.Test plan
pnpm start— loading screen shows animated battery waveFiles changed
src/icons/LoadingBar.tsxvite.config.ts.trycloudflare.comtoallowedHostssrc/index.csssrc/index.tsxSummary by CodeRabbit
New Features
Chores