Ft/account switching in the switching panel#171
Conversation
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds multi-account support across drafting and settings flows: new account summary/device-account hooks, account-switch UI, account-limit enforcement, DB lookup helpers, and sign-in network error wrapping. ChangesMulti-account switching feature
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/services/networkError.ts (1)
1-12: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
Error.causedirectly
@react-native/typescript-config@0.85.3already exposesError.cause, so the explicitcauseproperty and manual assignment are redundant. Pass{ cause }tosuper(...)and drop the field.🤖 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/services/networkError.ts` around lines 1 - 12, The NetworkError class is duplicating the built-in Error.cause support by declaring its own cause field and assigning it manually. Update the NetworkError constructor to pass the optional cause through to super(...) using the native Error.cause mechanism, and remove the explicit cause property and its assignment so the class relies on the standard Error behavior.src/app/screens/DraftingScreen.tsx (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove commented-out dead code.
Two commented-out remnants should be cleaned up:
- Line 24:
// useDraftingContext,— commented-out import.- Line 193:
{/* <DraftingPlayerBar verses={verses} /> */}— commented-out component JSX.If
DraftingPlayerBarremoval is intentional, remove the comment entirely. If it's temporarily disabled, consider a feature flag or document the rationale inline. TheuseDraftingContextimport should simply be deleted if no longer needed.♻️ Proposed cleanup
import { DraftingProvider, - // useDraftingContext, } from '../../drafting/DraftingContext';- {/* <DraftingPlayerBar verses={verses} /> */} - <DraftingTabBar activeTab={activeTab} onTabChange={setActiveTab} />Also applies to: 193-193
🤖 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/screens/DraftingScreen.tsx` at line 24, Remove the commented-out dead code in DraftingScreen by deleting the unused useDraftingContext import fragment and removing the commented DraftingPlayerBar JSX instead of leaving it in place. If DraftingPlayerBar is intentionally disabled, replace the comment with an explicit, documented mechanism; otherwise keep DraftingScreen clean by fully deleting the unused import and the unused JSX.src/components/ui/AccountSwitcherPanel.tsx (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the dynamic sheet padding out of JSX.
This inline style violates the React Native style rule for app code. Use a memoized style object instead. As per coding guidelines, “Avoid inline styles in React Native (
react-native/no-inline-stylesis a warning).”Proposed fix
-import React, { useCallback, useState } from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; const { accounts, hasAccountLimit, loading } = useDeviceAccounts(visible); const [switchingUserId, setSwitchingUserId] = useState<string | null>(null); const [switchError, setSwitchError] = useState<string | null>(null); + const sheetInsetsStyle = useMemo( + () => ({ paddingBottom: insets.bottom + theme.spacing.lg }), + [insets.bottom], + ); <Pressable onPress={event => event.stopPropagation()} - style={[ - styles.sheet, - { paddingBottom: insets.bottom + theme.spacing.lg }, - ]} + style={[styles.sheet, sheetInsetsStyle]} >Also applies to: 89-92
🤖 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/components/ui/AccountSwitcherPanel.tsx` at line 1, The dynamic sheet padding is currently being defined inline in JSX, which violates the React Native no-inline-styles guideline. Move the padding logic out of the rendered props in AccountSwitcherPanel and into a memoized style object created with useMemo (or an equivalent cached style helper), then apply that style through the component’s style prop so the JSX stays static.Source: Coding guidelines
🤖 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/components/ui/Accountswitcherpanel.test.tsx`:
- Around line 1-228: The AccountSwitcherPanel test file has formatting that
fails Prettier, so reformat the file to match the project style. Run the
formatter on the test and keep the existing test logic intact, especially the
AccountSwitcherPanel, setDeviceAccountsResult, and mocked navigation/service
setup.
In `@src/components/ui/AccountSwitcherPanel.tsx`:
- Line 131: There is trailing whitespace in the onPress handler for
AccountSwitcherPanel that is causing Prettier to fail. Remove the extra spaces
after the opening brace in the onPress callback and make sure the surrounding
JSX in AccountSwitcherPanel stays formatted consistently so the lint job passes.
In `@src/hooks/useDeviceAccounts.ts`:
- Around line 93-98: The limit check in useDeviceAccounts is based on
accounts.length, which is still empty during initial load and can briefly allow
adding another account. Update the hasAccountLimit logic to derive from the
stored known IDs source (for example KNOWN_USER_IDS or the persisted IDs used by
useDeviceAccounts) instead of the asynchronously loaded accounts array, and keep
the return shape in useDeviceAccounts consistent.
---
Nitpick comments:
In `@src/app/screens/DraftingScreen.tsx`:
- Line 24: Remove the commented-out dead code in DraftingScreen by deleting the
unused useDraftingContext import fragment and removing the commented
DraftingPlayerBar JSX instead of leaving it in place. If DraftingPlayerBar is
intentionally disabled, replace the comment with an explicit, documented
mechanism; otherwise keep DraftingScreen clean by fully deleting the unused
import and the unused JSX.
In `@src/components/ui/AccountSwitcherPanel.tsx`:
- Line 1: The dynamic sheet padding is currently being defined inline in JSX,
which violates the React Native no-inline-styles guideline. Move the padding
logic out of the rendered props in AccountSwitcherPanel and into a memoized
style object created with useMemo (or an equivalent cached style helper), then
apply that style through the component’s style prop so the JSX stays static.
In `@src/services/networkError.ts`:
- Around line 1-12: The NetworkError class is duplicating the built-in
Error.cause support by declaring its own cause field and assigning it manually.
Update the NetworkError constructor to pass the optional cause through to
super(...) using the native Error.cause mechanism, and remove the explicit cause
property and its assignment so the class relies on the standard Error behavior.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 85cf1beb-0c60-4316-9a7d-125dd7cd785c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (18)
src/app/appStyles.tssrc/app/screens/DraftingScreen.tsxsrc/app/screens/SettingsScreen.tsxsrc/components/layout/DraftingHeader.tsxsrc/components/ui/AccountInitialsButton.tsxsrc/components/ui/AccountSwitcherPanel.tsxsrc/components/ui/Accountswitcherpanel.test.tsxsrc/components/ui/SettingsListRow.tsxsrc/components/ui/UserSettingsMenu.tsxsrc/db/queries.tssrc/hooks/useActiveAccountSummary.tssrc/hooks/useAsyncRequestGuard.tssrc/hooks/useDeviceAccounts.tssrc/services/api.tssrc/services/networkError.tssrc/services/storage.tssrc/types/db/types.tssrc/utils/accountDisplay.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/utils/acccountDisplay.test.ts`:
- Around line 1-107: The test file name has a typo and does not match the module
naming, which makes the `getAccountDisplayName` and `getAccountInitials` tests
harder to locate. Rename the test file from the misspelled
`acccountDisplay.test.ts` to `accountDisplay.test.ts` so it aligns with the
`./accountDisplay` import and the source module naming.
🪄 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: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 32a499ea-4828-4f6f-93d1-586420988d47
📒 Files selected for processing (6)
src/components/ui/AccountSwitcherPanel.tsxsrc/components/ui/Accountswitcherpanel.test.tsxsrc/components/ui/UserSettingsMenu.test.tsxsrc/hooks/useDeviceAccounts.tssrc/services/storage.test.tssrc/utils/acccountDisplay.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/ui/Accountswitcherpanel.test.tsx
- src/components/ui/AccountSwitcherPanel.tsx
- src/hooks/useDeviceAccounts.ts
2 participants
Uh oh!
There was an error while loading. Please reload this page.