Conversation
📝 WalkthroughWalkthroughThe call sidebar component is enhanced with platform-specific behavior, including web-confirm dialogs and location alert consolidation. New helper functions check location data availability and centralize alert handling. Query logic is optimized to fetch calls only when the bottom sheet is open. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/sidebar/call-sidebar.tsx (1)
70-75:⚠️ Potential issue | 🟡 MinorFalsy check on
Latitude/Longitudetreats0as "no data".Coordinates of
0(equator/prime meridian) are valid but would fail the truthiness check on Line 72. If calls can realistically have(0, 0)or0on one axis, this would incorrectly hide the directions button.Suggested fix
const hasLocationData = (call: typeof activeCall) => { if (!call) return false; - const hasCoordinates = call.Latitude && call.Longitude; + const hasCoordinates = call.Latitude != null && call.Latitude !== '' && call.Longitude != null && call.Longitude !== ''; const hasAddress = call.Address && call.Address.trim() !== ''; return hasCoordinates || hasAddress; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/call-sidebar.tsx` around lines 70 - 75, hasLocationData currently treats numeric coordinates of 0 as absent due to truthiness checks; update the function (hasLocationData / the call parameter referencing Latitude and Longitude and Address on activeCall) to explicitly check for null/undefined for Latitude and Longitude (e.g., call.Latitude !== null && call.Latitude !== undefined) instead of truthiness, and keep the Address check as non-empty after trim (call.Address && call.Address.trim() !== '') so that valid 0 coordinates are considered present and the directions button is shown.
🧹 Nitpick comments (2)
src/components/sidebar/call-sidebar.tsx (2)
139-143: Use ternary operator instead of&&for conditional rendering.As per coding guidelines, use the conditional (ternary) operator
? :instead of&&for conditional rendering.Suggested fix
- {hasLocationData(activeCall) && ( - <Button variant="outline" className="flex-1" size="sm" action="primary" onPress={handleDirections}> - <ButtonIcon as={MapPin} /> - </Button> - )} + {hasLocationData(activeCall) ? ( + <Button variant="outline" className="flex-1" size="sm" action="primary" onPress={handleDirections}> + <ButtonIcon as={MapPin} /> + </Button> + ) : null}As per coding guidelines: "Use ternary operator
? :for conditional rendering instead of&&".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/call-sidebar.tsx` around lines 139 - 143, Replace the logical-AND JSX conditional in the CallSidebar render with an explicit ternary expression: where the code currently uses hasLocationData(activeCall) && (<Button ... onPress={handleDirections}>...), change it to hasLocationData(activeCall) ? (<Button ... onPress={handleDirections}><ButtonIcon as={MapPin} /></Button>) : null so the component returns null when the condition is false; keep the same Button, ButtonIcon, MapPin, and handleDirections identifiers and props unchanged.
31-40: Redundant guard insidequeryFn.When
enabled: isBottomSheetOpenisfalse, react-query won't invokequeryFnat all, making theif (!isBottomSheetOpen) return [];check on Line 35 dead code. Not harmful, but unnecessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/sidebar/call-sidebar.tsx` around lines 31 - 40, The queryFn passed to useQuery contains a redundant runtime guard (if (!isBottomSheetOpen) return []) because enabled: isBottomSheetOpen prevents queryFn from running when false; remove that check inside the queryFn and make queryFn simply call useCallsStore.getState().fetchCalls() then return useCallsStore.getState().calls so that useQuery({ queryKey: ['calls','open'], queryFn, enabled: isBottomSheetOpen }) is concise and correct.
🤖 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/sidebar/call-sidebar.tsx`:
- Around line 172-173: The className template literal for the call item has
trailing spaces and a missing line break; open the template on one line and
split conditional parts cleanly to avoid trailing spaces and ensure the closing
backtick and closing brace/parenthesis are on their own line. Locate the JSX
attribute using className on the call item (references: activeCall, CallId,
call, colorScheme) and reformat the multi-line template literal so each
conditional segment is concatenated without trailing whitespace and the final
closing `}` and `)` are placed on a new line to satisfy the linter.
---
Outside diff comments:
In `@src/components/sidebar/call-sidebar.tsx`:
- Around line 70-75: hasLocationData currently treats numeric coordinates of 0
as absent due to truthiness checks; update the function (hasLocationData / the
call parameter referencing Latitude and Longitude and Address on activeCall) to
explicitly check for null/undefined for Latitude and Longitude (e.g.,
call.Latitude !== null && call.Latitude !== undefined) instead of truthiness,
and keep the Address check as non-empty after trim (call.Address &&
call.Address.trim() !== '') so that valid 0 coordinates are considered present
and the directions button is shown.
---
Nitpick comments:
In `@src/components/sidebar/call-sidebar.tsx`:
- Around line 139-143: Replace the logical-AND JSX conditional in the
CallSidebar render with an explicit ternary expression: where the code currently
uses hasLocationData(activeCall) && (<Button ...
onPress={handleDirections}>...), change it to hasLocationData(activeCall) ?
(<Button ... onPress={handleDirections}><ButtonIcon as={MapPin} /></Button>) :
null so the component returns null when the condition is false; keep the same
Button, ButtonIcon, MapPin, and handleDirections identifiers and props
unchanged.
- Around line 31-40: The queryFn passed to useQuery contains a redundant runtime
guard (if (!isBottomSheetOpen) return []) because enabled: isBottomSheetOpen
prevents queryFn from running when false; remove that check inside the queryFn
and make queryFn simply call useCallsStore.getState().fetchCalls() then return
useCallsStore.getState().calls so that useQuery({ queryKey: ['calls','open'],
queryFn, enabled: isBottomSheetOpen }) is concise and correct.
Comment on lines
+172
to
+173
| className={`rounded-lg border p-4 ${colorScheme === 'dark' ? 'border-neutral-800 bg-neutral-800' : 'border-neutral-200 bg-neutral-50'} ${activeCall?.CallId === call.CallId ? (colorScheme === 'dark' ? 'bg-primary-900' : 'bg-primary-50') : '' | ||
| }`} |
There was a problem hiding this comment.
Fix formatting per linter.
Static analysis flagged whitespace issues on these lines. The multi-line template literal has trailing spaces and a missing line break.
Suggested fix
- className={`rounded-lg border p-4 ${colorScheme === 'dark' ? 'border-neutral-800 bg-neutral-800' : 'border-neutral-200 bg-neutral-50'} ${activeCall?.CallId === call.CallId ? (colorScheme === 'dark' ? 'bg-primary-900' : 'bg-primary-50') : ''
- }`}
+ className={`rounded-lg border p-4 ${colorScheme === 'dark' ? 'border-neutral-800 bg-neutral-800' : 'border-neutral-200 bg-neutral-50'} ${
+ activeCall?.CallId === call.CallId ? (colorScheme === 'dark' ? 'bg-primary-900' : 'bg-primary-50') : ''
+ }`}📝 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.
Suggested change
| className={`rounded-lg border p-4 ${colorScheme === 'dark' ? 'border-neutral-800 bg-neutral-800' : 'border-neutral-200 bg-neutral-50'} ${activeCall?.CallId === call.CallId ? (colorScheme === 'dark' ? 'bg-primary-900' : 'bg-primary-50') : '' | |
| }`} | |
| className={`rounded-lg border p-4 ${colorScheme === 'dark' ? 'border-neutral-800 bg-neutral-800' : 'border-neutral-200 bg-neutral-50'} ${ | |
| activeCall?.CallId === call.CallId ? (colorScheme === 'dark' ? 'bg-primary-900' : 'bg-primary-50') : '' | |
| }`} |
🧰 Tools
🪛 GitHub Check: test
[warning] 173-173:
Delete ··
[warning] 172-172:
Insert ⏎····················
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/sidebar/call-sidebar.tsx` around lines 172 - 173, The
className template literal for the call item has trailing spaces and a missing
line break; open the template on one line and split conditional parts cleanly to
avoid trailing spaces and ensure the closing backtick and closing
brace/parenthesis are on their own line. Locate the JSX attribute using
className on the call item (references: activeCall, CallId, call, colorScheme)
and reformat the multi-line template literal so each conditional segment is
concatenated without trailing whitespace and the final closing `}` and `)` are
placed on a new line to satisfy the linter.
Member
Author
|
Approve |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
Bug Fixes
Performance
Style