Repository navigation
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
WalkthroughThis PR introduces a new dismissible banner component with configuration-based dismissal tracking, updates AlertBox to use lucide-react icons as the default export, enhances LoadingBar with reduced-motion-aware RAF animation and easing functions, and extends the Config type to persist dismissed banner IDs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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-bitcoin with
|
| Latest commit: |
ae41708
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0ced92f3.wallet-bitcoin.pages.dev |
| Branch Preview URL: | https://wt-dismissible-banner-202602.wallet-bitcoin.pages.dev |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/icons/LoadingBar.tsx (1)
57-57: Avoid hard-coding segment count in the animation loop.Line 57 ties behavior to a magic number (
6) while rendering is driven bySEGMENTS. IfSEGMENTSchanges, animation and rendering can diverge.♻️ Proposed fix
- 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, The animation loop in LoadingBar.tsx uses a hard-coded 6 (for (let i = 0; i < 6; i++)) which can diverge from the rendering segments; change that loop to use the SEGMENTS constant (e.g., for (let i = 0; i < SEGMENTS; i++)) so animation iteration count always matches rendering, and update any related index math inside the loop that assumed 6 to reference SEGMENTS instead.src/components/DismissibleBanner.tsx (1)
16-17: Static reduced-motion check won't react to preference changes mid-session.This is a common and acceptable pattern, but if a user toggles their reduced-motion preference while the app is open, it won't be detected until next module load. For a banner that's dismissed once, this is unlikely to matter in practice.
If you want reactive behavior later, consider moving this check inside the component or using a custom hook with
matchMedia.addEventListener('change', ...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DismissibleBanner.tsx` around lines 16 - 17, The module-level prefersReducedMotion constant is evaluated once and won't update if the user changes their OS reduced-motion setting mid-session; move the media-query logic into the DismissibleBanner component (or a small hook like usePrefersReducedMotion) so it uses window.matchMedia('(prefers-reduced-motion: reduce)') inside useEffect, initialize component state from matchMedia.matches, add a 'change' event listener to update that state, and clean up the listener on unmount; update any usage of prefersReducedMotion in the component to read from the new state/hook.
🤖 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/components/AlertBox.tsx`:
- Around line 85-103: The dismiss control is a non-focusable div with
role='button' so replace that element with a native <button> element (preserve
aria-label='Dismiss', onClick handler, and all inline style properties) to make
it keyboard-focusable and support Enter/Space automatically; remove
role='button' and ensure you set type="button" on the button, keep the X icon
usage (X size={18} color='var(--black)') and flex layout, and retain touchAction
and cursor styles so behavior and appearance remain unchanged.
---
Nitpick comments:
In `@src/components/DismissibleBanner.tsx`:
- Around line 16-17: The module-level prefersReducedMotion constant is evaluated
once and won't update if the user changes their OS reduced-motion setting
mid-session; move the media-query logic into the DismissibleBanner component (or
a small hook like usePrefersReducedMotion) so it uses
window.matchMedia('(prefers-reduced-motion: reduce)') inside useEffect,
initialize component state from matchMedia.matches, add a 'change' event
listener to update that state, and clean up the listener on unmount; update any
usage of prefersReducedMotion in the component to read from the new state/hook.
In `@src/icons/LoadingBar.tsx`:
- Line 57: The animation loop in LoadingBar.tsx uses a hard-coded 6 (for (let i
= 0; i < 6; i++)) which can diverge from the rendering segments; change that
loop to use the SEGMENTS constant (e.g., for (let i = 0; i < SEGMENTS; i++)) so
animation iteration count always matches rendering, and update any related index
math inside the loop that assumed 6 to reference SEGMENTS instead.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
package.jsonsrc/components/AlertBox.tsxsrc/components/DismissibleBanner.tsxsrc/icons/LoadingBar.tsxsrc/index.tsxsrc/lib/types.tssrc/providers/config.tsxsrc/screens/Wallet/Index.tsxvite.config.ts
Replace the non-dismissible PSA banner with a reusable DismissibleBanner
component. The support banner ("Go to Settings > Support for help and bug
reports") can be closed via an X button, and the dismissal persists in
localStorage via a new `bannersDismissed` field on Config.
- New DismissibleBanner component wraps AlertBox with persistence logic
- Dismiss animation: fade out + slide down + collapse (300ms ease-out-quint)
- Haptic feedback on dismiss, respects prefers-reduced-motion
- Close button uses Lucide X icon with 44px tap target for iOS
- Close button uses var(--black) for proper light/dark mode contrast
- PSA system kept as higher-priority emergency fallback
- Tapping the banner navigates to Settings > Support
039a771 to
3284e41
Compare
Deploying wallet-mutinynet with
|
| Latest commit: |
ae41708
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3fa7a8bb.arkade-wallet.pages.dev |
| Branch Preview URL: | https://wt-dismissible-banner-202602.arkade-wallet.pages.dev |
Replaces div[role=button] with a native <button> element for the banner close icon so it is keyboard-focusable and handles Enter/Space automatically. Addresses CodeRabbit review feedback on PR #337.
bordalix
left a comment
bordalix
left a comment
There was a problem hiding this comment.
- rebase master to fix conflicts
- don't use lucide-react, there are known problems with expo, check src/icons for icons in SVG
- currently we would never see the new banner since psaMessage is defined in the running environment and it supersedes the new banner: TODO remove psaMessage from running environment
|
closing since we have #375 |
Summary
DismissibleBannercomponent on the homepagebannersDismissed: string[]field onConfigXicon with 44px iOS tap target, proper light/dark mode contrastprefers-reduced-motion— skips animation and persists immediatelyVITE_PSA_MESSAGE) kept as higher-priority emergency fallbackFiles changed
src/components/DismissibleBanner.tsxsrc/components/AlertBox.tsxsrc/lib/types.tsbannersDismissed: string[]to Config typesrc/providers/config.tsxbannersDismissed: []to defaultConfigsrc/screens/Wallet/Index.tsxpackage.jsonlucide-reactdependencyTest plan
bannersDismissed: ["support-banner"])VITE_PSA_MESSAGEis set, PSA takes priority over support bannerSummary by CodeRabbit
New Features
Chores