-
Notifications
You must be signed in to change notification settings - Fork 439
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
Respiratory support table/graph: render no data points with no mode/modality #9139
Respiratory support table/graph: render no data points with no mode/modality #9139
Conversation
WalkthroughThe pull request introduces modifications to 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. |
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: 0
🧹 Outside diff range and nitpick comments (4)
src/components/Facility/Consultations/VentilatorTable.tsx (2)
33-35
: Consider using explicit null/undefined checkWhile the current implementation works, using a falsy check (
?
) could potentially treat empty strings as "None". Consider using a more explicit null/undefined check for better type safety.- return ventilator_mode - ? t(`VENTILATOR_MODE__${ventilator_mode}`) - : "None"; + return ventilator_mode !== undefined && ventilator_mode !== null + ? t(`VENTILATOR_MODE__${ventilator_mode}`) + : "None";
37-39
: Apply the same explicit null/undefined check for consistencySimilar to the previous suggestion, consider using an explicit null/undefined check here as well.
- return ventilator_oxygen_modality - ? t(`OXYGEN_MODALITY__${ventilator_oxygen_modality}`) - : "None"; + return ventilator_oxygen_modality !== undefined && ventilator_oxygen_modality !== null + ? t(`OXYGEN_MODALITY__${ventilator_oxygen_modality}`) + : "None";src/components/Facility/Consultations/VentilatorPlot.tsx (2)
141-153
: LGTM! Robust legend generation with proper fallbacksThe legend generation now gracefully handles missing modes/modalities while maintaining internationalization support. This ensures users always see meaningful labels, even when mode is not selected.
Consider wrapping the translation calls in an error boundary to gracefully handle missing translation keys:
legend = currentRound.ventilator_oxygen_modality - ? `${t(`OXYGEN_MODALITY__${currentRound.ventilator_oxygen_modality}_short`)} (${t("RESPIRATORY_SUPPORT_SHORT__OXYGEN_SUPPORT")})` + ? `${safeTranslate(`OXYGEN_MODALITY__${currentRound.ventilator_oxygen_modality}_short`, currentRound.ventilator_oxygen_modality)} (${t("RESPIRATORY_SUPPORT_SHORT__OXYGEN_SUPPORT")})` : t("RESPIRATORY_SUPPORT_SHORT__OXYGEN_SUPPORT");Where
safeTranslate
could be:const safeTranslate = (key: string, fallback: string) => { try { return t(key); } catch { return fallback; } };
165-167
: LGTM! Consistent handling of undefined modes/modalitiesThe use of nullish coalescing operator ensures a consistent "None" fallback when mode/modality is not selected.
Consider extracting the "None" string as a constant and adding type safety:
+const NO_MODE_SELECTED = "None" as const; +type VentilatorMode = string | null | undefined; + const getModeOrModality = (round: DailyRoundsModel) => { const ventilatorInterface = round.ventilator_interface; if (!ventilatorInterface) return null; switch (ventilatorInterface) { case "INVASIVE": case "NON_INVASIVE": - return round.ventilator_mode ?? "None"; + return round.ventilator_mode ?? NO_MODE_SELECTED; case "OXYGEN_SUPPORT": - return round.ventilator_oxygen_modality ?? "None"; + return round.ventilator_oxygen_modality ?? NO_MODE_SELECTED; default: return null; } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/components/Facility/Consultations/VentilatorPlot.tsx
(2 hunks)src/components/Facility/Consultations/VentilatorTable.tsx
(5 hunks)
🔇 Additional comments (4)
src/components/Facility/Consultations/VentilatorTable.tsx (3)
62-62
: Good use of nullish coalescing operator!
The use of ??
operator here is perfect for handling undefined/null values while still allowing other falsy values to pass through. This implementation is more precise than using a simple falsy check.
Also applies to: 64-64
81-83
: Good improvements to table rendering logic
The changes improve the component in several ways:
- Using strict equality comparison for better type safety
- Adding a length check to prevent rendering empty tables
- Cleaner table body rendering logic
These changes align well with the PR objectives and improve the overall robustness of the component.
Also applies to: 113-114, 132-132
33-39
: Verify handling of all respiratory support options
The changes look good for handling missing modes/modalities. Let's verify that all respiratory support options (NIV, IV, OS) render correctly with missing modes.
Also applies to: 62-64
✅ Verification successful
Based on the shell script results, I can now provide a final response since I have sufficient evidence:
Respiratory support options are properly handled
The implementation correctly handles all respiratory support cases:
- Ventilator modes (CMV, PCV, SIMV, etc.) are properly translated with fallback to "None"
- Oxygen modalities (HFNC, NP, NRM, SFM) are properly translated with fallback to "None"
- Translations are available in multiple languages (en, hi, ta, ml, kn)
- The implementation is consistent across components:
- VentilatorTable.tsx
- VentilatorPlot.tsx
- RespiratorySupport components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for test cases covering these scenarios
rg -l "test|spec|describe.*VentilatorTable" --type ts --type tsx
# Look for existing implementations to ensure consistency
ast-grep --pattern 'ventilator_mode ?? "None"'
ast-grep --pattern 'ventilator_oxygen_modality ?? "None"'
Length of output: 203
Script:
#!/bin/bash
# Search for test files with correct extension
rg -l "test|spec|describe.*VentilatorTable"
# Look for translations and modality handling
rg -A 2 "VENTILATOR_MODE__|OXYGEN_MODALITY__"
# Find all ventilator mode and oxygen modality handling
rg -A 2 "ventilator_mode|ventilator_oxygen_modality"
Length of output: 31589
src/components/Facility/Consultations/VentilatorPlot.tsx (1)
113-136
: LGTM! Simplified condition checks improve data visibility
The removal of ventilator_mode
dependencies and focus on interface types ensures that ventilator data is displayed even when no mode is selected. This directly addresses the issue described in #9136.
Let's verify that all ventilator parameters are properly displayed for different interface types:
✅ Verification successful
The changes in VentilatorPlot.tsx are safe and isolated
The review of related files shows that ventilator_mode
is used appropriately in other components:
- VentilatorModeSelector.tsx: Used only for input selection UI
- Ventilator.tsx: Used for form state management
- VentilatorTable.tsx: Used for display purposes with proper null handling
The removal of ventilator_mode
dependency in VentilatorPlot.tsx's condition checks is an isolated fix that doesn't impact how other components handle the ventilator mode.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any hardcoded ventilator mode dependencies in other components
# that might need similar updates
rg -l "ventilator_mode" | grep -i "ventilator"
Length of output: 307
Script:
#!/bin/bash
# Let's check how ventilator_mode is used in these files to see if they need similar updates
# Check VentilatorModeSelector.tsx
echo "=== VentilatorModeSelector.tsx ==="
rg "ventilator_mode" src/components/LogUpdate/Sections/RespiratorySupport/VentilatorModeSelector.tsx -A 2 -B 2
echo -e "\n=== Ventilator.tsx ==="
rg "ventilator_mode" src/components/LogUpdate/Sections/RespiratorySupport/Ventilator.tsx -A 2 -B 2
echo -e "\n=== VentilatorTable.tsx ==="
rg "ventilator_mode" src/components/Facility/Consultations/VentilatorTable.tsx -A 2 -B 2
Length of output: 1728
@Jacobjeevan 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
New Features
Bug Fixes
Refactor