Always show "online" for authenticated user profiles #9574
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Users/UserListAndCard.tsx (2)
137-138: Remove trailing spaces to satisfy prettier rules.
Both lines 137 and 138 appear to have trailing whitespace that's flagged by ESLint.- const authUser = useAuthUser(); - const isAuthUser = user.id === authUser.id; + const authUser = useAuthUser(); + const isAuthUser = user.id === authUser.id;🧰 Tools
🪛 eslint
[error] 137-137: Delete
·(prettier/prettier)
[error] 138-138: Delete
·(prettier/prettier)
238-238: Remove trailing space for consistency.
On line 238, ESLint suggests removing the extra space at the end of the line.- <div className="grid grid-cols-1 gap-4 @xl:grid-cols-3 @4xl:grid-cols-4 @6xl:grid-cols-5 lg:grid-cols-2"> + <div className="grid grid-cols-1 gap-4 @xl:grid-cols-3 @4xl:grid-cols-4 @6xl:grid-cols-5 lg:grid-cols-2">🧰 Tools
🪛 eslint
[error] 238-238: Delete
·(prettier/prettier)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Users/UserListAndCard.tsx(2 hunks)
🧰 Additional context used
🪛 eslint
src/components/Users/UserListAndCard.tsx
[error] 137-137: Delete ·
(prettier/prettier)
[error] 138-138: Delete ·
(prettier/prettier)
[error] 238-238: Delete ·
(prettier/prettier)
🔇 Additional comments (1)
src/components/Users/UserListAndCard.tsx (1)
139-139: Implementation correctly ensures the authenticated user is always shown as online.
This addresses the primary issue of displaying "Online now" for the user's own profile.
|
Do add proper title for the PR. Refer the following on getting started to contributing: |
rithviknishad
changed the title
Co-authored-by: Rithvik Nishad <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Users/UserListAndCard.tsx(1 hunks)
🧰 Additional context used
🪛 eslint
src/components/Users/UserListAndCard.tsx
[error] 139-139: 'isOnline' is assigned a value but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
🔇 Additional comments (2)
src/components/Users/UserListAndCard.tsx (2)
137-139: Follow consistent naming conventions
The variable name cur_online doesn't follow the common JavaScript/TypeScript naming convention for boolean variables. The new isOnline variable follows better naming practices.
Let's maintain consistency by using the isOnline naming convention throughout the codebase. This includes updating other components that use cur_online:
- Update in
getNameAndStatusCard:
const getNameAndStatusCard = (
user: UserModel | UserAssignedModel,
- cur_online: boolean,
+ isOnline: boolean,
showDetailsButton = false,
) => {
// ...
className={classNames(
"flex items-center gap-2 rounded-full px-3 py-1",
- cur_online ? "bg-green-100" : "bg-gray-100",
+ isOnline ? "bg-green-100" : "bg-gray-100",
)}
// ...
}- Update in
UserCard:
const UserCard = ({ user }: { user: UserModel | UserAssignedModel }) => {
- const userOnline = isUserOnline(user);
+ const isOnline = isUserOnline(user);
// ...
- {getNameAndStatusCard(user, userOnline)}
+ {getNameAndStatusCard(user, isOnline)}
// ...
}🧰 Tools
🪛 eslint
[error] 139-139: 'isOnline' is assigned a value but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
137-139: Verify the impact on user experience
The change to always show authenticated users as online might need additional UI/UX considerations:
- Should we visually distinguish between "actually online" and "self-view always online"?
- Should we show a tooltip or additional context when viewing our own status?
Let's check if there are any existing UI patterns for status indicators:
✅ Verification successful
Based on the code analysis, I can now provide a final response:
Current UI implementation already handles online status distinction appropriately
The current implementation already provides clear visual distinction between online statuses:
-
The
UserStatusIndicatorcomponent uses different visual styles:- A green background (
bg-green-100) for online users - A gray background (
bg-gray-100) for offline users - The status is shown in a rounded pill/badge format (
rounded-full)
- A green background (
-
The implementation is consistent across different views:
- Used in user lists (
UserListAndCard.tsx) - Used in user banners (
UserBanner.tsx) - Maintains consistent styling with padding variations for different contexts
- Used in user lists (
The existing implementation provides sufficient visual distinction between actual online status and self-view status, as it uses standard UI patterns (color-coded badges) that are intuitive for users. Additional tooltips or visual indicators would add unnecessary complexity to an already clear interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other status indicator patterns in the codebase
rg -l 'StatusIndicator|status.*indicator|online.*status' --type tsx --type ts
Length of output: 107
Script:
#!/bin/bash
# Search for other status indicator patterns in the codebase, adjusting file extensions
rg -l 'StatusIndicator|status.*indicator|online.*status' --type ts
Length of output: 145
Script:
#!/bin/bash
# Look for UI components or styles related to online status
rg -A 3 'isOnline|online.*status|user.*status' --type ts
Length of output: 6086
Script:
#!/bin/bash
# Search for tooltip or visual indicator implementations
rg -A 3 'tooltip|Tooltip|title=' --type ts
Length of output: 74922
Script:
#!/bin/bash
# Find files that might contain the status indicator component
fd -e ts -e tsx StatusIndicator
Length of output: 33
Script:
#!/bin/bash
# Search for online status implementation and visual indicators
rg -A 5 'isOnline|online.*status|user.*status|status.*indicator' src/components/Users/
Length of output: 1144
Script:
#!/bin/bash
# Look for any UI components that show user status
ast-grep --pattern 'const $status = $$$; return <$$$>{$status}$$$' src/components/Users/
Length of output: 90
Script:
#!/bash/shell
# Search for any existing tooltip implementations for user status
rg -A 3 'title=.*online|tooltip.*online|aria-label.*online' src/components/Users/
Length of output: 83
Script:
#!/bin/bash
# Look for the implementation of isUserOnline function
rg -A 5 'function isUserOnline|const isUserOnline' --type ts
Length of output: 347
Script:
#!/bin/bash
# Search for any visual indicators or styles related to online status
rg -A 3 'className.*online|style.*online|<.*online' src/components/Users/
Length of output: 75
Script:
#!/bin/bash
# Look for any existing user status display components
rg -A 5 '<.*Status.*>|status.*indicator|status.*badge' src/components/Users/
Length of output: 1354
Script:
#!/bin/bash
# Look for the UserStatusIndicator component implementation
rg -A 10 'UserStatusIndicator' src/components/Users/ --type ts
Length of output: 4056
Script:
#!/bin/bash
# Search for any existing visual indicators or status dots
rg -A 3 'rounded-full.*bg-|h-[1-2].*w-[1-2].*bg-' src/components/Users/
Length of output: 73
Script:
#!/bin/bash
# Look for any existing tooltip or title implementations in the status indicator
ast-grep --pattern 'const $$ = isUserOnline($user); return <div $$$>{$$$}</div>' src/components/Users/
Length of output: 104
Script:
#!/bin/bash
# Look for the actual implementation of the status indicator's JSX
rg -A 10 'const isOnline = isUserOnline\(user\).*\{' src/components/Users/
Length of output: 76
Script:
#!/bin/bash
# Search for any existing status dot implementations
rg -A 3 'div.*className.*h-.*w-.*rounded.*bg-' src/components/Users/
Length of output: 70
Script:
#!/bin/bash
# Look for any existing tooltip components used with online status
rg -A 5 'Tooltip.*online|title.*online' src/CAREUI/
Length of output: 53
Script:
#!/bin/bash
# Look for the actual implementation of the status indicator's render logic
rg -A 10 'isOnline = isUserOnline.*\|\| isAuthUser' src/components/Users/
Length of output: 843
Script:
#!/bin/bash
# Search for any existing status dot implementations in RecordMeta
rg -A 5 'isOnline.*rounded-full' src/CAREUI/display/RecordMeta.tsx
Length of output: 68
Script:
#!/bin/bash
# Look for any existing online status implementations in the codebase
rg -A 5 'const isOnline.*return.*div' --type ts
Length of output: 49
🧰 Tools
🪛 eslint
[error] 139-139: 'isOnline' is assigned a value but never used. Allowed unused vars must match /^_/u.
(@typescript-eslint/no-unused-vars)
rithviknishad
added
good to merge
and removed
needs testing
needs review
Deploy-Failed
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Users/UserListAndCard.tsx (1)
137-141: Implementation looks good, consider adding type safetyThe implementation correctly ensures that authenticated users are shown as online on their own profiles. However, consider adding type guards to handle potential undefined values.
const authUser = useAuthUser(); - const isAuthUser = user.id === authUser.id; + const isAuthUser = Boolean(user.id && authUser.id && user.id === authUser.id); const isOnline = isUserOnline(user) || isAuthUser;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Users/UserListAndCard.tsx(1 hunks)
🔇 Additional comments (1)
src/components/Users/UserListAndCard.tsx (1)
147-147: Verify consistent behavior across all component usages
The implementation correctly updates all instances where online status is used. The changes maintain consistency in the UI by updating:
- Background color classes
- Status indicator color
- Text color
- Status text display
Let's verify all usages of the UserStatusIndicator component:
Also applies to: 154-154, 160-160, 163-163
✅ Verification successful
Let me verify the implementation of UserStatusIndicator to ensure consistent behavior.
Let me check the full implementation of the component to verify the consistency.
Let me try one final check to see the complete implementation of the component.
Online status styling is consistently implemented across all usages
Based on the verification:
- The
UserStatusIndicatorcomponent is used in 3 locations:UserListAndCard.tsx(2 instances) andUserBanner.tsx(1 instance) - The component implementation shows consistent styling:
- Container:
bg-green-100for online,bg-gray-100for offline - Status dot:
bg-green-500for online,bg-gray-400for offline - Text shows either "online" or relative time since last login
- Container:
- All instances use the same component with identical styling behavior, passing only the required user prop and optional padding
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all instances where UserStatusIndicator is used
# to ensure the changes don't have unintended side effects
# Find direct component usage
rg -A 2 "<UserStatusIndicator"
# Find imports of the component
rg "import.*UserStatusIndicator"
Length of output: 831
Script:
#!/bin/bash
# Find the implementation of UserStatusIndicator component
ast-grep --pattern 'const UserStatusIndicator = ({ $$ }) => {
$$$
}'
# Also check for any other status-related styling
rg -A 3 'isOnline.*bg-'
Length of output: 605
Script:
#!/bin/bash
# Get the full component implementation with more context
rg -B 5 -A 15 'const UserStatusIndicator'
# Check for any other online status related styling patterns
rg 'online.*className' -A 2
Length of output: 1334
Script:
#!/bin/bash
# Get a larger context around the UserStatusIndicator component
rg -B 5 -A 30 'export const UserStatusIndicator'
# Also check for any status-related text
rg '"online"' -A 2
Length of output: 2600
|
@abhimanyurajeesh Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit