Fix navigation bar highlighting to select only the active section #9252
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 Full review |
✅ Actions performedFull review 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
🧹 Outside diff range and nitpick comments (2)
src/components/Patient/PatientDetailsTab/Demography.tsx (2)
47-47: Consider lowering the intersection thresholdThe current threshold of 0.6 requires sections to be 60% visible before triggering the observer. This might lead to sections not being highlighted until significantly scrolled into view.
- { threshold: 0.6 }, + { threshold: 0.3 },
Line range hint
68-73: Consider adding a way to reset clicked sectionCurrently, once a section is clicked, the scroll-based highlighting won't work until page refresh. Consider adding a way to reset the clicked state, perhaps after a certain time delay or when clicking the same section again.
const scrollToSection = (sectionId: string) => { - setClickedSection(sectionId); + setClickedSection((prev) => prev === sectionId ? null : sectionId); setActiveSection(sectionId); const section = document.getElementById(sectionId); if (section) { section.scrollIntoView({ behavior: "smooth" }); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Patient/PatientDetailsTab/Demography.tsx(2 hunks)
🔇 Additional comments (2)
src/components/Patient/PatientDetailsTab/Demography.tsx (2)
34-34: LGTM: Good state management approach
The introduction of clickedSection state effectively separates user-initiated section selection from scroll-based activation.
41-45: Verify the fix prevents multiple section highlighting
The implementation looks correct, but let's verify that clicking "Volunteer Contact" no longer causes "Insurance Details" to be highlighted simultaneously.
Also applies to: 68-69
✅ Verification successful
Let me gather more information about the implementation to verify the fix thoroughly.
After reviewing the complete implementation, I can now provide the final response:
Fix for multiple section highlighting is correctly implemented
The implementation effectively prevents simultaneous highlighting through:
- The
clickedSectionstate that blocks the IntersectionObserver from updatingactiveSectionwhen a section is clicked - The
scrollToSectionfunction that properly manages both states - A single source of truth for section highlighting through the
activeSectionstate
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any other instances where multiple sections might be highlighted
# Look for other places in the codebase where section highlighting is managed
rg -l "setActiveSection|scrollToSection" src/
Length of output: 103
Script:
#!/bin/bash
# Check the complete implementation in Demography.tsx
cat src/components/Patient/PatientDetailsTab/Demography.tsx
Length of output: 15192
Jacobjeevan
requested review from
vigneshhari
and removed request for
vigneshhari
|
Waiting for backend changes. |
|
the phone number is not being fetched to the frontend even with backend |
I raised a PR in backend so it should be merged to get the patient contact number. now i have changed the alt_phone_number to phone_number in frontend |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Patient/PatientDetailsTab/Demography.tsx(3 hunks)src/components/Patient/models.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Patient/PatientDetailsTab/Demography.tsx
rithviknishad
added
needs testing
needs review
and removed
test failed
waiting for backend
Deploy-Failed
|
LGTM |
|
@Jacobjeevan I will update it by tomorrow. |
|
👋 Hi, @Rishith25, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
|
no longer relevant in new architecture |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Bug Fixes
Refactor