Fix: Minor UI Enhancement in questionnaire #10987
Conversation
WalkthroughThe pull request introduces modifications to four components. In the Changes
Possibly related PRs
Suggested labels
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)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🪧 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. |
|
@nihal467 @rithviknishad @amjithtitus09
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
662-665: Fixed issue with clearing the dosage fieldThis change resolves the original issue where users couldn't completely remove dosage values. Now when a user clears the input, it will properly set the value to
undefinedinstead of defaulting to 0.The type casting approach
(undefined as unknown as number)works but is a bit unconventional. Consider using an optional type in the interface instead of forcing undefined into a number type.A more type-safe approach might be:
-value: value - ? Number(value) - : (undefined as unknown as number), +value: value ? Number(value) : undefined, +// And update the type definition to allow undefined for the value propertyThis would require updating the type definition for
bounds_durationto allowundefinedfor thevalueproperty, but would be more type-safe.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx(2 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (3)
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx (2)
32-32: Import of Hospitalization type looks goodThe import of the
Hospitalizationtype is correctly added to support the implementation of resetting hospitalization data.
118-124: Great addition to clear hospitalization data for non-inpatient encountersThis change properly addresses the requirement to reset hospitalization data when switching to a non-hospitalization encounter class. When the user changes the encounter class to ambulatory ("amb"), virtual ("vr"), or home health ("hh"), the hospitalization data is correctly reset to an empty object.
This fix will prevent outdated hospitalization data from persisting after changing encounter types, which aligns with the PR objective.
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
649-652: Good fix for dosage input renderingUsing the nullish coalescing operator is an appropriate way to handle potentially undefined values. This ensures the input field displays properly when there is no value instead of showing "0".
src/components/Questionnaire/QuestionTypes/EncounterQuestion.tsx
Outdated
Show resolved
Hide resolved
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx
Outdated
Show resolved
Hide resolved
@AdityaJ2305 Why not clear these doubts in the issue itself? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (3)
202-233: Responsive desktop table layout.
This block cleanly displays diagnoses in a tabular format for larger screens. The structure is correct, but be aware of potential duplication with the mobile layout below. Consider abstracting shared logic if the code grows further.
235-420: Mobile-friendly layout.
This layout replicates the desktop features for smaller screens. The result is readable, but there is visible duplication between desktop and mobile views. Should you need to maintain more logic or additional fields, consider a unified or shared component to reduce repetition.
470-544: Main row structure and status fields.
Using a table row with separate cells for date, clinical status, and verification status is organized. Attempt to use a unique, stable key for each rendered row. Currently, the list useskey={index}, which could cause React to misinterpret row updates.- <DiagnosisTableRow - key={index} - ... + <DiagnosisTableRow + key={diagnosis.id || index} + ...
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (6)
src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (6)
2-3: No concerns with the icon imports.
They are used correctly for marking statuses and actions.
11-11: Ensure all side effects are intended.
TheuseEffecthook is fetchingpatientDiagnosesand updating the questionnaire response preemptively. Ensure no re-render loops or unintended side effects occur ifpatientDiagnoseschanges repeatedly.
30-37: New table component imports look correct.
The imports match the usage of<Table>,<TableBody>,<TableCell>,<TableHead>,<TableHeader>, and<TableRow>.
441-446: Introduction ofDiagnosisTableRow.
Splitting out a dedicated component for table rows improves readability and maintainability. This separation makes sense, especially due to the more complex UI controls.
546-573: Dropdown menu actions.
Marking a diagnosis as active, inactive, or resolved from this menu is clear. The icons are well chosen, and the destructuring approach to updating fields is consistent and concise. No issues found.
585-601: Conditional notes row.
In-table notes are a neat UX addition. Ensure to update your test coverage or manual QA to verify the notes row is displayed and updated correctly in all scenarios (active, inactive, resolved, etc.).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx (3)
255-282: Consider reusing the existing SymptomActionsMenu component instead of duplicating code.The dropdown menu implementation here duplicates much of the functionality already available in the
SymptomActionsMenucomponent (defined on lines 104-148), leading to code duplication.- <TableCell className="min-w-[35px] py-1 px-0"> - <DropdownMenu> - <DropdownMenuTrigger asChild> - <Button - variant="ghost" - size="icon" - disabled={disabled} - className="h-7 w-6 px-0" - > - <DotsVerticalIcon className="h-3.5 w-3.5" /> - </Button> - </DropdownMenuTrigger> - <DropdownMenuContent align="end"> - <DropdownMenuItem onClick={handleToggleNotes}> - <Pencil2Icon className="h-4 w-4 mr-2" /> - {showNotes ? t("hide_notes") : t("add_notes")} - </DropdownMenuItem> - <DropdownMenuSeparator /> - <DropdownMenuItem - className="text-destructive focus:text-destructive" - onClick={handleRemove} - > - <MinusCircledIcon className="h-4 w-4 mr-2" /> - {t("remove_symptom")} - </DropdownMenuItem> - </DropdownMenuContent> - </DropdownMenu> + <TableCell className="min-w-[35px] py-1 px-0"> + <SymptomActionsMenu + showNotes={showNotes} + verificationStatus={symptom.verification_status} + disabled={disabled} + onToggleNotes={handleToggleNotes} + onRemove={handleRemove} + /> </TableCell>
422-438: Consider adding ARIA attributes for improved accessibility.While the mobile view implementation is excellent, consider enhancing it with additional ARIA attributes for better screen reader support.
- <div className="md:hidden divide-y divide-gray-200"> + <div className="md:hidden divide-y divide-gray-200" role="table" aria-label={t("symptoms_list")}> {symptoms.map((symptom, index) => ( - <div key={index} className="p-3 space-y-3"> + <div key={index} className="p-3 space-y-3" role="row"> - <div className="flex items-center justify-between"> + <div className="flex items-center justify-between" role="cell"> <div className="font-medium">{symptom.code.display}</div> <SymptomActionsMenu showNotes={Boolean(symptom.note)} verificationStatus={symptom.verification_status} disabled={disabled} onToggleNotes={() => handleUpdateSymptom(index, { note: symptom.note ? undefined : "", }) } onRemove={() => handleRemoveSymptom(index)} /> </div>
518-534: Consider conditionally handling empty notes.The current implementation renders the notes section if
symptom.note !== undefined, but it might still show an empty input field ifsymptom.noteis an empty string. Consider adjusting the condition to also check for empty strings.- {symptom.note !== undefined && ( + {symptom.note !== undefined && ( <div> <Label className="text-xs text-gray-500"> {t("notes")} </Label> <Input type="text" placeholder={t("add_notes_about_symptom")} value={symptom.note ?? ""} onChange={(e) => handleUpdateSymptom(index, { note: e.target.value }) } disabled={disabled} className="mt-1" /> </div> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (5)
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx (5)
22-22: Good addition of new UI components for better structure.These imports support the migration from a div-based layout to a more semantic table-based layout, along with proper form labeling.
Also applies to: 30-37
192-299: Excellent restructuring with semantic table layout.The restructuring of the
SymptomRowcomponent from divs to a table-based layout significantly improves the semantic structure, readability, and accessibility of the UI. This approach:
- Uses proper table semantics with
TableRowandTableCell- Maintains good responsive styling with appropriate class names
- Correctly implements the notes section as a full-width row when displayed
- Preserves all the existing functionality while improving structure
391-421: Well-implemented desktop view with proper table semantics.The implementation of the table structure with proper
TableHeader,TableHead, and related components provides good semantic structure and improves accessibility. The column layout is well organized with appropriate widths for each type of content.
422-537: Excellent responsive design implementation for mobile view.The mobile-specific view is well implemented with:
- Clear separation using the
md:hiddenclass- Proper layout with appropriate spacing and formatting
- Good use of
Labelcomponents for improved accessibility- Descriptive section organization with appropriate grid structure
The implementation properly handles the responsive design requirements while maintaining all functionality.
395-405:❓ Verification inconclusive
Verify if the width classes are defined correctly in your CSS system.
The table header uses width classes like
w-55,w-25,w-21, etc., which are not standard Tailwind CSS classes. Ensure these custom width classes are properly defined in your CSS system.
🏁 Script executed:
#!/bin/bash # Check for custom width classes definition in CSS files or Tailwind config echo "Searching for custom width class definitions..." rg -t css -t js -t ts -t jsx -t tsx "w-55|w-25|w-21|w-9" --no-ignoreLength of output: 203
Action Required: Verify Custom Width Class Definitions
Please double-check that the custom width classes (w-55,w-25,w-21, andw-9) used insrc/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx(lines 395–405) are properly defined in your CSS system or Tailwind configuration. The initial search command encountered an error due to including the unsupported JSX file type. The revised search (above) should help confirm whether these custom classes are declared; if no definitions are found, please update your configuration accordingly.
🏁 Script executed:
#!/bin/bash echo "Searching for custom width class definitions (excluding JSX files)..." rg -t css -t js -t ts -t tsx "w-55|w-25|w-21|w-9" --no-ignoreLength of output: 362
Ensure Custom Width Classes Are Defined in Your CSS/Tailwind Config
Please double-check that the custom width classes (w-55,w-25,w-21,w-9) used insrc/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx(lines 395–405) are properly defined. Our initial search efforts ran into file type issues with JSX/TSX files. The revised script above adds custom type definitions for JSX and TSX. If this new search still yields no results, manually verify whether these classes are defined in your CSS or in your Tailwind configuration (e.g.,tailwind.config.js). If they’re missing, please update your config accordingly.
🏁 Script executed:
#!/bin/bash echo "Searching for custom width class definitions with added type definitions (including JSX/TSX)..." rg --type-add 'jsx:*.jsx' --type-add 'tsx:*.tsx' -t css -t js -t ts -t tsx -t jsx "w-55|w-25|w-21|w-9" --no-ignoreLength of output: 65976
Action: Confirm Custom Width Class Definitions
The table header in
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx(lines 395–405) uses custom width classes (w-55,w-25,w-21,w-9), which aren’t standard Tailwind classes. An automated search (excluding files from node_modules) did not conclusively find definitions for these classes. Please manually verify whether these classes are defined in your CSS or Tailwind configuration (e.g., in yourtailwind.config.jsor a related CSS file). If they’re missing or unintended, update your CSS configuration or consider replacing them with standard Tailwind width utilities.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx (2)
403-518: Comprehensive mobile-friendly implementation.The mobile view implementation shows careful attention to user experience on smaller screens, with proper labeling and stacked layout. This addresses the "clipping issue in the questionnaire" mentioned in the PR objectives.
However, there's some code duplication between desktop and mobile views that could be refactored.
Consider extracting common logic for symptom manipulation into shared helper functions instead of duplicating them in both views. For example, the input change handlers could be shared:
// Add this shared function +const createSymptomUpdateHandler = (index, field, handleUpdateSymptom) => (value) => { + const update = field === 'onset' + ? { onset: { onset_datetime: typeof value === 'string' ? value : value.target.value } } + : { [field]: value }; + handleUpdateSymptom(index, update); +}; // Then in the mobile view, replace inline handlers: -onChange={(e) => - handleUpdateSymptom(index, { - onset: { onset_datetime: e.target.value }, - }) -} +onChange={createSymptomUpdateHandler(index, 'onset', handleUpdateSymptom)}
209-209: UI consistency improvement for input heights.The desktop view uses shorter inputs (h-7) while the mobile view uses taller inputs (h-8). Consider standardizing input heights across different viewports for better visual consistency.
-className="h-7 text-sm px-1" +className="h-8 text-sm px-1"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: Test
- GitHub Check: cypress-run (1)
🔇 Additional comments (6)
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx (6)
22-22: Improved UI structure with semantic table components.Good addition of UI components that enhance the semantic structure of the questionnaire, making it more accessible and organized. The Table components provide better structure than div-based layouts previously used.
Also applies to: 30-37
193-264: Excellent transition to table-based layout for improved organization.The restructuring from div-based layout to TableRow and TableCell components creates a better semantic structure, improving accessibility and visual organization of symptom data. The table cells are properly aligned with appropriate minimum widths and styling.
266-279: Good pattern for expandable notes within the table structure.Using a separate TableRow with colSpan maintains the table structure while allowing for expandable notes. This is a good UI pattern for complex forms.
372-402: Well-implemented responsive desktop view.The desktop view properly uses TableHeader and TableBody components with appropriate column headings. This creates a clean, professional presentation of symptom data that aligns with the PR objective of addressing UI clipping issues.
423-435: Enhanced accessibility with proper labeling in mobile view.Good use of the Label component for form fields in the mobile view. This improves accessibility and follows best practices for form design.
376-387:❓ Verification inconclusive
Verify column alignment and width constraints.
The table header widths are set to fixed values (w-55, w-25, etc.) which may not be standard Tailwind classes. Ensure that these classes are properly defined in your tailwind configuration, or consider using standard Tailwind width classes.
🏁 Script executed:
#!/bin/bash # Look for custom width classes in the tailwind config grep -r "w-55\|w-25\|w-21\|w-9" --include="*.js" --include="*.ts" --include="*.tsx" .Length of output: 65624
Custom Tailwind width classes need verification
It appears that no locally defined custom classes (e.g. “w-55”, “w-25”, “w-21”, “w-9”) were found in the repository’s configuration. Please double-check your Tailwind configuration to confirm that these width classes are intentionally defined. If they are missing or unintended, consider switching to standard Tailwind width classes (or using Tailwind’s arbitrary value syntax) to ensure consistent column alignment.
- Verify your tailwind.config.js (or equivalent) includes extensions for these custom classes.
- If not defined, update the widths to standard Tailwind classes or define custom settings accordingly.
|
@AdityaJ2305 Date field looks very weird |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style