Add edit button to immunisation section; replaced buttons with shadcn buttons across PatientsDetail section #9245
Conversation
WalkthroughThe changes involve updating the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
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
🧹 Outside diff range and nitpick comments (1)
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx (1)
46-46: LGTM! Consider minor hover state improvements.The removal of the 'hidden' class properly fixes the visibility issue while maintaining all necessary functionality (permissions, translations, error handling).
However, the hover state classes could be more consistent:
hover:bg-secondary-200is used independentlygroup-hover:flexseems unnecessary as the button is always visible nowConsider simplifying the class list:
- className=" rounded border border-secondary-400 bg-white px-1 py-1 text-sm font-semibold text-green-800 hover:bg-secondary-200 group-hover:flex" + className="rounded border border-secondary-400 bg-white px-1 py-1 text-sm font-semibold text-green-800 hover:bg-secondary-200"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx(1 hunks)
🔇 Additional comments (2)
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx (2)
Line range hint 1-144: Overall implementation looks good!
The component demonstrates good practices:
- Proper permission handling
- Comprehensive i18n support
- Clean data display patterns
- Proper date formatting
Line range hint 46-58: Verify consistency with other sections
Let's ensure this implementation matches the styling and behavior of edit buttons in other sections of the patient details page.
✅ Verification successful
Button styling and edit icon implementation are consistent
The implementation in ImmunisationRecords.tsx follows the same patterns found across the patient details components:
- The button styling with
rounded border border-secondary-400 bg-white hover:bg-secondary-200matches the patterns used in other patient detail tabs (Demography.tsx and HealthProfileSummary.tsx) - The CareIcon implementation with
icon="l-edit-alt"and similar className styling (text-md mr-1 mt-1) is consistent with other patient detail components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar edit button implementations in other patient detail components
rg -l "PatientDetailsTab" | xargs rg -A 2 "className=\".*rounded.*border.*hover:bg-secondary-200.*\""
# Look for other uses of CareIcon with edit functionality
rg "CareIcon.*icon=\"l-edit-alt\"" --type typescript
Length of output: 187
Script:
#!/bin/bash
# Search for similar edit button implementations in patient detail components
rg -l "PatientDetailsTab"
# Then search for button styling patterns
rg "className=.*rounded.*border.*hover:bg"
# Look for CareIcon edit patterns
rg "CareIcon.*icon.*edit"
Length of output: 7187
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx (1)
Issue found with Edit button visibility in immunisation section
The immunisation section's Edit button has a
group-hover:flexclass but its parent div doesn't have the requiredgroupclass, which explains why the button remains invisible. The parent div only hasflex flex-row gap-x-4classes.
- In
src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx, add thegroupclass to the parent div that hasflex flex-row gap-x-4classes.🔗 Analysis chain
Line range hint
64-80: Verify changes in immunisation section.While this change looks good, the PR's main objective is to fix the invisible Edit button in the immunisation section. Let's verify the corresponding changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for ImmunisationRecords component and its Edit button implementation rg -l "ImmunisationRecords" | xargs rg -A 10 "ButtonV2" # Check for any hidden class usage in immunisation-related files rg -l "immunisation|Immunisation" | xargs rg "hidden"Length of output: 3682
Script:
#!/bin/bash # Let's check the diff of the PR to see the changes made to the immunisation section git diff origin/main -- src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx # Let's also check for any group-hover classes in the file as we see it in the ButtonV2 rg "group-hover:" src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx -A 2 -B 2Length of output: 539
Script:
#!/bin/bash # Let's check the parent div of the ButtonV2 to see if it has the group class ast-grep --pattern 'div { $$$ <ButtonV2 className="$_" $$$ }' src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx # Also check for any display-related classes in the parent elements rg "flex|hidden|invisible" src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx -B 5 -A 1Length of output: 779
src/components/Patient/PatientDetailsTab/Demography.tsx (1)
421-430: LGTM! The Edit button implementation aligns with PR objectives.The ButtonV2 implementation correctly addresses the visibility issue while maintaining:
- Proper permission checks via the
withPermissionCheckwrapper- Consistent styling with other sections using appropriate color classes
- Accessibility through clear button text and icon
However, consider adding a data-testid attribute for better testability:
<ButtonV2 + data-testid="edit-section-button" className="flex rounded border border-secondary-400 bg-white px-2 py-1 text-sm font-semibold text-green-800 hover:bg-secondary-200" disabled={!patientData.is_active} onClick={withPermissionCheck(() => handleEditClick(subtab.id), )} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/components/Patient/PatientDetailsTab/Demography.tsx(1 hunks)src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx(3 hunks)src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx
🔇 Additional comments (5)
src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx (4)
6-6: LGTM!
The ButtonV2 import follows the project's import conventions.
Line range hint 64-80: LGTM! ButtonV2 implementation maintains all necessary functionality.
The ButtonV2 component:
- Preserves accessibility with aria-label
- Maintains consistent styling
- Retains all permission checks and click handling
78-78: Verify icon spacing after margin removal.
The icon's class has been simplified to just text-md. Please verify that:
- The icon maintains proper spacing with the "edit" text
- The icon size remains consistent with other edit buttons across the application
✅ Verification successful
Icon styling appears consistent with similar edit buttons
Based on the search results, the text-md class is consistently used across similar edit buttons in the codebase, particularly in the Patient Details components:
ImmunisationRecords.tsx: usestext-mdDemography.tsx: uses bothtext-mdandtext-lgfor different edit buttons- Your component (
HealthProfileSummary.tsx): usestext-md
The spacing will be maintained as all these buttons follow the same pattern of icon followed by translated text within a ButtonV2 component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for consistent icon class usage across similar buttons
rg -l "CareIcon.*l-edit-alt" | xargs rg -A 2 "CareIcon.*l-edit-alt"
Length of output: 2161
Line range hint 1-138: Verify alignment with PR objectives.
While the changes in this file are well-implemented, they appear to be part of a broader refactor rather than directly addressing the invisible Edit button in the immunisation section (PR #9243). Could you please:
- Clarify if these changes are necessary for consistency across all edit buttons
- Ensure the main issue in the immunisation section is also being addressed
✅ Verification successful
The git diff shows that the changes in HealthProfileSummary.tsx are part of a consistent pattern of replacing <button> elements with <ButtonV2> components and standardizing the icon styling across multiple files, including ImmunisationRecords.tsx. This change directly addresses the invisible edit button issue in the immunisation section by removing the group-hover:flex class that was causing the visibility issue.
Changes align with PR objectives and fix the core issue.
The modifications in HealthProfileSummary.tsx are necessary as they're part of a systematic update to:
- Replace all
buttonelements with the standardizedButtonV2component - Normalize icon styling by removing inconsistent margin classes (
mr-1 mt-1) - Fix visibility issues by removing conditional display classes
These changes maintain consistency across the codebase while addressing the original issue in the immunisation section.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Look for the immunisation component
rg -l "immunisation|Immunisation" --type typescript --type typescript-react
# Check its recent changes
git diff HEAD~1 -- $(rg -l "immunisation|Immunisation" --type typescript --type typescript-react)
Length of output: 4881
src/components/Patient/PatientDetailsTab/Demography.tsx (1)
421-430: Verify consistent ButtonV2 usage across patient detail sections
Let's ensure this ButtonV2 implementation is consistent with other sections mentioned in the PR objectives (immunisation section).
✅ Verification successful
Let me gather more information about ButtonV2 usage to ensure we have a complete picture.
Let me try one more search to find all ButtonV2 components and their styling patterns across the patient components:
ButtonV2 implementation is consistent across patient detail sections
The verification shows consistent implementation of edit buttons across the patient detail sections:
-
The Immunisation section (
ImmunisationRecords.tsx) uses the same ButtonV2 pattern with matching:- Styling:
rounded border border-secondary-400 bg-white text-green-800 hover:bg-secondary-200 - Icon:
<CareIcon icon="l-edit-alt" - Disabled state:
disabled={!patientData.is_active}
- Styling:
-
This matches the implementation in:
- Demography section
- HealthProfileSummary section
- Other patient detail sections
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check ButtonV2 implementation consistency across patient detail sections
# Search for ButtonV2 usage in patient detail components
echo "Checking ButtonV2 implementation in patient detail components..."
rg -A 5 "ButtonV2.*edit" "src/components/Patient/PatientDetailsTab/"
# Search for similar button styling patterns
echo "Checking button styling consistency..."
rg "flex.*rounded.*border.*secondary-400.*bg-white.*text-green-800" "src/components/Patient/"
Length of output: 812
Script:
#!/bin/bash
# Search for all ButtonV2 components in patient detail sections
echo "Searching for all ButtonV2 components..."
ast-grep --pattern 'ButtonV2 $$$' src/components/Patient/PatientDetailsTab/
# Check for any edit buttons in immunisation section specifically
echo "Checking immunisation section..."
rg -A 10 "ButtonV2" "src/components/Patient/PatientDetailsTab/Immunisation.tsx" || echo "File not found"
# Look for other similar button patterns that might use different styling
echo "Checking other button patterns..."
rg "ButtonV2.*className.*text-green-800" "src/components/Patient/"
Length of output: 602
Script:
#!/bin/bash
# Find all files containing ButtonV2 in patient components
echo "Finding all files with ButtonV2..."
fd . src/components/Patient -t f -x rg -l "ButtonV2"
# Search for all ButtonV2 components with their full context
echo "Checking ButtonV2 implementations..."
rg -B 2 -A 5 "<ButtonV2" src/components/Patient/
# Look for edit buttons specifically
echo "Checking edit button patterns..."
rg -B 1 -A 5 'icon="l-edit-alt"' src/components/Patient/
Length of output: 25696
DonXavierdev
changed the title
There was a problem hiding this comment.
ButtonV2 is not shadcn's button.
Here's shadcn's button: https://github.com/ohcnetwork/care_fe/blob/develop/src/components/ui/button.tsx
Also do not apply classNames to style the button. Use variant prop there.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/components/ui/button.tsx (1)
25-26: Consider renaming the variant for consistencyThe variant name
rounded_secondarydoesn't follow the existing naming pattern. Other variants use descriptive names without shape attributes (e.g., 'default', 'destructive', 'outline', 'secondary').Consider renaming to something more semantic like:
-rounded_secondary: +secondary_outline:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
src/components/Patient/PatientDetailsTab/Demography.tsx(3 hunks)src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx(3 hunks)src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx(3 hunks)src/components/ui/button.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/components/Patient/PatientDetailsTab/Demography.tsx
- src/components/Patient/PatientDetailsTab/HealthProfileSummary.tsx
- src/components/Patient/PatientDetailsTab/ImmunisationRecords.tsx
rithviknishad
changed the title
|
@DonXavierdev 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! 🙌 |
… buttons across PatientsDetail section (ohcnetwork#9245)
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Screenshot
Before
After
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes