-
Notifications
You must be signed in to change notification settings - Fork 516
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed User Default View in Resource and Shifting #9306
Fixed User Default View in Resource and Shifting #9306
Conversation
WalkthroughThe pull request introduces changes to four components: Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
src/components/Shifting/ShiftingBoard.tsx (1)
Line range hint
1-1
: Consider a centralized view preference management systemThe current implementation has several systemic issues across components. Consider implementing a centralized solution:
- Create a custom hook for view preference management
- Implement a context provider for sharing view state
- Add proper TypeScript types for view preferences
- Add migration logic for handling legacy preferences
Would you like me to provide a detailed implementation plan for this architectural improvement?
src/components/Resource/ResourceList.tsx (2)
Line range hint
35-36
: Consider persisting view preference on navigationThe board view navigation doesn't update the stored preference, which could lead to inconsistency with the actual view state.
const onBoardViewBtnClick = () => { + try { + localStorage.setItem("defaultResourceView", "board"); + } catch (error) { + console.error("Failed to save view preference:", error); + } navigate("/resource/board", { query: qParams }); };
26-26
: Consider adding unit tests for view persistenceTo ensure the view persistence feature works reliably, add unit tests covering:
- Initial view selection
- View switching
- localStorage error handling
- Navigation with stored preferences
Would you like me to help create unit tests for the view persistence functionality?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/Resource/ResourceBoard.tsx
(1 hunks)src/components/Resource/ResourceList.tsx
(1 hunks)src/components/Shifting/ShiftingBoard.tsx
(1 hunks)src/components/Shifting/ShiftingList.tsx
(1 hunks)
🔇 Additional comments (1)
src/components/Resource/ResourceList.tsx (1)
26-26
: Verify the view persistence implementation across components
The localStorage implementation needs to be consistent across all view components to ensure proper view persistence.
✅ Verification successful
Based on the results, I can now generate the final response as we have sufficient information about the localStorage implementation across components.
View persistence implementation is consistent across components
The localStorage implementation for view preferences is consistently implemented:
- Resource views correctly set "defaultResourceView" to "list"/"board" in both ResourceList and ResourceBoard
- Shifting views follow the same pattern with "defaultShiftView"
- Navigation between views is properly handled with query parameter preservation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent localStorage implementation across view components
# Check for localStorage usage in resource views
echo "Checking resource view components..."
rg -A 2 'localStorage\.setItem.*defaultResourceView'
# Check for localStorage usage in shifting views
echo "Checking shifting view components..."
rg -A 2 'localStorage\.setItem.*defaultShiftView'
# Check for proper view preference handling in navigation
echo "Checking navigation handling..."
rg -A 5 'navigate\(.*/resource/(list|board)'
Length of output: 2427
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/Resource/ResourceBoard.tsx
(2 hunks)src/components/Resource/ResourceList.tsx
(3 hunks)src/components/Shifting/ShiftingBoard.tsx
(3 hunks)src/components/Shifting/ShiftingList.tsx
(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Shifting/ShiftingList.tsx
- src/components/Resource/ResourceList.tsx
- src/components/Resource/ResourceBoard.tsx
🔇 Additional comments (3)
src/components/Shifting/ShiftingBoard.tsx (3)
9-10
: LGTM: Clean import of new Button component
The migration from ButtonV2 to the new Button component aligns with UI standardization.
127-130
: LGTM: Clean Button implementation
The Button component implementation follows best practices:
- Uses proper variant
- Includes icon with label for accessibility
- Properly integrates with translation system
79-82
: Verify consistent implementation across components
Let's verify that the view preference handling is consistent across all components.
✅ Verification successful
Let me correct the file type flags and try again with a more precise search pattern.
Let me try one more time with the correct file type flags.
Based on the search results, I can now provide a final response as we have sufficient evidence:
View preference handling is consistent across components
The implementation is consistent across both Shifting and Resource components:
- Both ShiftingBoard and ShiftingList components use "defaultShiftView"
- Both ResourceBoard and ResourceList components use "defaultResourceView"
- Each component correctly sets the opposite view type when switching views
- Navigation patterns are consistent, using the same structure for both resources
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent view preference implementation across components
# Search for localStorage usage with view preferences
echo "Checking localStorage usage for view preferences:"
rg -A 2 "localStorage\.setItem\(.*View.*\)" --type ts --type tsx
# Search for view navigation patterns
echo "\nChecking view navigation patterns:"
rg -A 2 "navigate\(.*/(list|board)" --type ts --type tsx
# Search for similar button implementations
echo "\nChecking button implementations:"
ast-grep --pattern 'Button $props {
$$$
onClick={$_}
$$$
}'
Length of output: 522
Script:
#!/bin/bash
# Search for localStorage usage with view preferences
echo "Checking localStorage usage for view preferences:"
rg "localStorage\.setItem.*View" -t typescript -t tsx
# Search for view navigation patterns
echo -e "\nChecking view navigation patterns:"
rg "navigate.*/(list|board)" -t typescript -t tsx
# Search for view-related button click handlers
echo -e "\nChecking view-related click handlers:"
rg "on(List|Board|Grid).*Click" -t typescript -t tsx
Length of output: 552
Script:
#!/bin/bash
# Search for localStorage usage with view preferences
echo "Checking localStorage usage for view preferences:"
rg "localStorage\.setItem.*View"
# Search for view navigation patterns
echo -e "\nChecking view navigation patterns:"
rg "navigate.*/(list|board)"
# Search for view-related button click handlers
echo -e "\nChecking view-related click handlers:"
rg "on(List|Board|Grid).*Click"
# Check the file extension pattern in the codebase
echo -e "\nChecking available file extensions:"
fd -e ts -e tsx
Length of output: 27695
LGTM |
@nithish1018 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
Screen Recording:
fixresourceview.mp4
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Style