Questionnaire List & Edit UI Redesign #10678
Conversation
…quest-ui-redesign
Deploying care-fe with
|
| Latest commit: |
4461967
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a3f7f285.care-fe.pages.dev |
| Branch Preview URL: | https://quest-ui-redesign.care-fe.pages.dev |
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Issue created for Questionnaire Preview Redesign #10679 |
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
quest-ui-redesign
|
| Run status |
|
| Run duration | 06m 54s |
| Commit |
|
| Committer | Amjith Titus |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
11
|
| View all changes introduced in this branch ↗︎ | |
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request implements multiple enhancements across the localization and questionnaire modules. New keys are added to the localization JSON file to support questionnaire creation, navigation, and search functions. Several components have been updated to include an Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CB as Clone Button
participant M as cloneQuestionnaire Mutation
participant EH as Error Handler
participant T as Toast
U->>CB: Click "Clone" button
CB->>M: Trigger cloneQuestionnaire mutation
alt Mutation Fails
M-->>EH: Return error object with details
EH->>T: Iterate over error messages
T-->>U: Display each error message
else Mutation Succeeds
M-->>CB: Successful clone response
end
sequenceDiagram
participant Q as Question Component
participant PC as Preview Checker (isPreview)
participant UQ as useQuery Hook
participant DF as Data Fetch
Q->>PC: Check if ID equals "preview"
alt isPreview is true
PC-->>UQ: Disable query (enabled: false)
UQ-->>Q: Skip data fetching (preview mode)
else isPreview is false
PC-->>UQ: Enable query
UQ->>DF: Fetch data from API
DF-->>UQ: Return data
UQ-->>Q: Display fetched data
end
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (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 (
|
![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 comments (6)
src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (2)
127-139: 🛠️ Refactor suggestionAdd error handling for API failures.
The
useEffecthook should handle potential API errors to prevent silent failures and provide feedback to users.const { data: patientAllergies } = useQuery({ + onError: (error) => { + // Handle error appropriately (e.g., show toast notification) + console.error('Failed to fetch allergies:', error); + }, queryKey: ["allergies", patientId], queryFn: query(allergyIntoleranceApi.getAllergy, {
411-415: 🛠️ Refactor suggestionAdd date format validation.
The
last_occurrencedate should be validated before updating to prevent invalid dates.onChange={(e) => + const date = new Date(e.target.value); + if (!isNaN(date.getTime())) { handleUpdateAllergy(index, { last_occurrence: e.target.value, }) + } }src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (3)
108-120:⚠️ Potential issueAdd missing dependencies to useEffect.
The effect uses
updateQuestionnaireResponseCBandquestionnaireResponse.question_idbut they're not included in the dependency array. This could lead to stale closures.useEffect(() => { if (patientDiagnoses?.results) { updateQuestionnaireResponseCB( [ { type: "diagnosis", value: patientDiagnoses.results.map(convertToDiagnosisRequest), }, ], questionnaireResponse.question_id, ); } -}, [patientDiagnoses]); +}, [patientDiagnoses, updateQuestionnaireResponseCB, questionnaireResponse.question_id]);
287-298: 🛠️ Refactor suggestionAdd date validation to prevent invalid inputs.
The date input lacks validation constraints which could allow invalid dates to be entered.
<Input type="date" value={diagnosis.onset?.onset_datetime || ""} + min="1900-01-01" + max={new Date().toISOString().split('T')[0]} onChange={(e) => onUpdate?.({ onset: { onset_datetime: e.target.value }, }) } disabled={disabled || !!diagnosis.id} className="h-8 md:h-9" />
96-106: 🛠️ Refactor suggestionAdd error and loading states for better user experience.
The query implementation should handle and communicate error and loading states to users.
-const { data: patientDiagnoses } = useQuery({ +const { data: patientDiagnoses, isLoading, error } = useQuery({ queryKey: ["diagnoses", patientId], queryFn: query(diagnosisApi.listDiagnosis, { pathParams: { patientId }, queryParams: { encounter: encounterId, limit: 100, }, }), enabled: encounterId !== "preview", }); + +if (isLoading) { + return <div className="text-sm text-gray-500">{t("loading_diagnoses")}</div>; +} + +if (error) { + return <div className="text-sm text-red-500">{t("error_loading_diagnoses")}</div>; +}src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (1)
121-128:⚠️ Potential issueAdd null safety checks for patientMedications.
The current implementation assumes patientMedications is always defined. Add null safety checks to prevent potential runtime errors.
useEffect(() => { - if (patientMedications?.results) { + if (patientMedications?.results?.length) { updateQuestionnaireResponseCB( [{ type: "medication_statement", value: patientMedications.results }], questionnaireResponse.question_id, ); + } else if (patientMedications?.results) { + // Handle empty results case + updateQuestionnaireResponseCB( + [{ type: "medication_statement", value: [] }], + questionnaireResponse.question_id, + ); } }, [patientMedications]);
🧹 Nitpick comments (20)
src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx (1)
290-300: Good optimization, but consider handling preview mode explicitly.The addition of the
enabledoption to prevent unnecessary API calls in preview mode is a good optimization. However, consider the following improvements:
- Document the preview mode behavior in the component's JSDoc.
- Consider explicitly handling the preview mode case in the
useEffecthook.Add JSDoc comments to document the behavior:
+/** + * SymptomQuestion component for managing symptoms in a questionnaire. + * @remarks + * - In preview mode (encounterId === "preview"), the symptoms query is disabled + * and the component will show an empty initial state. + */ export function SymptomQuestion({Handle preview mode explicitly in the effect:
useEffect(() => { + if (encounterId === "preview") { + return; // Exit early for preview mode + } if (patientSymptoms?.results) { updateQuestionnaireResponseCB(src/components/Questionnaire/QuestionRenderer.tsx (1)
82-82: Consider improving the preview mode implementation.While the change correctly implements the preview mode behavior, there are a few improvements to consider:
- Handle the case when
encounterIdis undefined to avoid unexpected behavior.- Use a constant for the "preview" value to prevent typos and make refactoring easier.
- Consider using a dedicated boolean prop for preview mode instead of overloading
encounterId.Here's a suggested implementation:
+const PREVIEW_ENCOUNTER_ID = "preview"; + export function QuestionRenderer({ questions, responses, onResponseChange, errors, clearError, disabled, activeGroupId, - encounterId, + encounterId = "", facilityId, patientId, }: QuestionRendererProps) { // ... <QuestionGroup // ... - disabled={disabled || encounterId === "preview"} + disabled={disabled || encounterId === PREVIEW_ENCOUNTER_ID} // ... /> // ... }Alternatively, consider using a dedicated prop:
interface QuestionRendererProps { // ... encounterId?: string; + isPreview?: boolean; // ... } export function QuestionRenderer({ // ... encounterId, + isPreview = false, // ... }: QuestionRendererProps) { // ... <QuestionGroup // ... - disabled={disabled || encounterId === "preview"} + disabled={disabled || isPreview} // ... /> // ... }src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (3)
106-113: Consider handling the preview mode in the useEffect.The useEffect should account for the preview mode to prevent unnecessary state updates.
useEffect(() => { - if (patientMedications?.results) { + if (patientMedications?.results && encounterId !== "preview") { updateQuestionnaireResponseCB( [{ type: "medication_request", value: patientMedications.results }], questionnaireResponse.question_id, ); } -}, [patientMedications]); +}, [patientMedications, encounterId]);
171-183: Consider using immutable state updates.The current implementation directly mutates the medication object. Consider using a more immutable approach to prevent potential state-related issues.
const handleUpdateMedication = ( index: number, updates: Partial<MedicationRequest>, ) => { - const newMedications = medications.map((medication, i) => - i === index ? { ...medication, ...updates } : medication, - ); + const newMedications = medications.map((medication, i) => + i === index + ? { ...medication, ...updates, updatedAt: new Date().toISOString() } + : medication, + ); updateQuestionnaireResponseCB( [{ type: "medication_request", value: newMedications }], questionnaireResponse.question_id, ); };
828-840: Enhance date picker accessibility.The date picker could benefit from additional accessibility attributes and better error handling.
<DateTimePicker value={ medication.authored_on ? new Date(medication.authored_on) : undefined } onChange={(date) => { if (!date) return; onUpdate?.({ authored_on: date.toISOString() }); }} disabled={disabled || isReadOnly} + aria-label={t("authored_on")} + aria-required="true" + onError={(error) => { + console.error('Date validation error:', error); + }} />src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (2)
408-419: Enhance date input user experience.Consider adding explicit focus handling to invoke the native date picker when the input receives focus.
<Input type="date" value={allergy.last_occurrence || ""} onChange={(e) => handleUpdateAllergy(index, { last_occurrence: e.target.value, }) } + onFocus={(e) => e.target.showPicker?.()} disabled={disabled} className="h-8 mt-1" />
467-475: Improve accessibility of status indicators.Using only opacity for status indicators may not provide sufficient contrast. Consider adding additional visual cues.
const rowClassName = `group ${ allergy.verification_status === "entered_in_error" - ? "opacity-40 pointer-events-none" + ? "opacity-40 pointer-events-none bg-gray-50" : allergy.clinical_status === "inactive" - ? "opacity-60" + ? "opacity-60 bg-gray-50" : allergy.clinical_status === "resolved" - ? "line-through" + ? "line-through bg-green-50" : "" }`;src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx (1)
96-106: LGTM! Good performance optimization.The addition of the
enabledproperty to prevent unnecessary API calls during preview mode is a good optimization. This aligns well with similar changes in other questionnaire components.Consider extracting the "preview" string as a constant to maintain consistency across components:
+const PREVIEW_MODE = "preview"; + const { data: patientDiagnoses } = useQuery({ queryKey: ["diagnoses", patientId], queryFn: query(diagnosisApi.listDiagnosis, { pathParams: { patientId }, queryParams: { encounter: encounterId, limit: 100, }, }), - enabled: encounterId !== "preview", + enabled: encounterId !== PREVIEW_MODE, });src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx (4)
109-119: Good optimization for preview mode, but consider error handling.The
enabledcondition is a good optimization to prevent unnecessary API calls in preview mode. However, consider adding error handling and pagination for better reliability.const { data: patientMedications } = useQuery({ queryKey: ["medication_statements", patientId], queryFn: query(medicationStatementApi.list, { pathParams: { patientId }, queryParams: { limit: 100, encounter: encounterId, }, }), enabled: encounterId !== "preview", + retry: 3, + onError: (error) => { + // Handle error appropriately + console.error('Failed to fetch medications:', error); + } });
130-140: Add validation for medication code.The
handleAddMedicationfunction should validate the medication code before adding it to prevent invalid data.const handleAddMedication = (medication: Code) => { + if (!medication?.code || !medication?.display) { + console.warn('Invalid medication code:', medication); + return; + } const newMedications: MedicationStatementRequest[] = [ ...medications, { ...MEDICATION_STATEMENT_INITIAL_VALUE, medication }, ]; updateQuestionnaireResponseCB( [{ type: "medication_statement", value: newMedications }], questionnaireResponse.question_id, ); setExpandedMedicationIndex(newMedications.length - 1); };
225-248: Enhance table header accessibility.The table header could benefit from improved accessibility attributes for better screen reader support.
-<div className="hidden lg:grid grid-cols-[300px,180px,170px,250px,260px,190px,200px,48px] bg-gray-50 border-b text-sm font-medium text-gray-500"> +<div + role="row" + className="hidden lg:grid grid-cols-[300px,180px,170px,250px,260px,190px,200px,48px] bg-gray-50 border-b text-sm font-medium text-gray-500"> - <div className="font-semibold text-gray-600 p-3 border-r"> + <div role="columnheader" className="font-semibold text-gray-600 p-3 border-r"> {t("medicine")} </div> // Apply similar changes to other header cells </div>
378-384: Optimize component rendering with React.memo.The
MedicationStatementGridRowcomponent could benefit from memoization to prevent unnecessary re-renders.-const MedicationStatementGridRow: React.FC<MedicationStatementGridRowProps> = ({ +const MedicationStatementGridRow = React.memo<MedicationStatementGridRowProps>(({ medication, disabled, onUpdate, onRemove, index, -}) => { +}) => { // Component implementation -}; +});src/components/Questionnaire/QuestionnaireEditor.tsx (6)
67-67: UnusedObservationTypestate variable.
const [observation, setObservation] = useState<ObservationType | undefined>();is declared but never used. Consider removing it if it's not part of a planned future feature.Also applies to: 614-614
103-147: New interfaces and props typings.
These interfaces (Organization,StylingMetadata,OrganizationResponse,TagResponse,QuestionnairePropertiesProps) look well-structured. However, theQuestionnairePropertiesPropsis quite large; consider splitting it into smaller, domain-specific props if it grows further.
344-446:TagSelectorlogic.
Mirrors the approach ofOrganizationSelector. Consider refactoring common code to avoid duplication (e.g., shared “Selector” component).
772-779:handleToggleTagfunction.
Identical tohandleToggleOrganization; potential duplication. You could unify toggling logic with a single helper to reduce repetition.
808-815: Tab triggers for “edit” and “preview.”
The icons and labels look correct. Consider adding i18n translations for "Edit form" / "Preview form" if they're user-facing.
894-917: DuplicatedQuestionnairePropertiesin mobile vs. desktop.
The approach of rendering properties differently for mobile (lg:hidden) vs. desktop (hidden lg:block) works, but if the logic grows, consider refactoring into a single responsive layout to avoid duplication.Also applies to: 1064-1088
src/components/Questionnaire/QuestionnaireList.tsx (1)
130-156: Consider adding loading state for description popover.While the popover implementation is good, consider showing a loading state when fetching detailed content to improve user experience.
<PopoverContent className="w-80"> <div className="space-y-2"> + {isLoading ? ( + <div className="flex items-center justify-center py-4"> + <Loader2 className="h-4 w-4 animate-spin" /> + </div> + ) : ( <h4 className="font-medium">{questionnaire.title}</h4> <p className="text-sm text-gray-600"> {questionnaire.description} </p> + )} </div> </PopoverContent>src/components/Questionnaire/QuestionnaireForm.tsx (1)
466-527: Consider extracting form actions into a separate component.The form actions section (search and buttons) is quite large. Consider extracting it into a separate component for better maintainability.
+interface FormActionsProps { + questionnaireForms: QuestionnaireFormState[]; + subjectType?: string; + onSelect: (selected: QuestionnaireDetail) => void; + onCancel?: () => void; + onSubmit: () => void; + isPending: boolean; + hasErrors: boolean; +} + +function FormActions({ + questionnaireForms, + subjectType, + onSelect, + onCancel, + onSubmit, + isPending, + hasErrors, +}: FormActionsProps) { + return ( + <> + <div className="flex gap-4 items-center m-4 max-w-4xl"> + <QuestionnaireSearch + subjectType={subjectType} + onSelect={onSelect} + disabled={isPending} + /> + </div> + {questionnaireForms.length > 0 && ( + <div className="flex justify-end gap-4 mx-4 mt-4 max-w-4xl"> + <Button + type="button" + variant="outline" + onClick={onCancel} + disabled={isPending} + > + {t("cancel")} + </Button> + <Button + type="submit" + onClick={onSubmit} + disabled={isPending || hasErrors} + className="relative" + > + {isPending ? ( + <> + <span className="opacity-0">{t("submit")}</span> + <div className="absolute inset-0 flex items-center justify-center"> + <div className="h-5 w-5 animate-spin rounded-full border-b-2 border-white" /> + </div> + </> + ) : ( + t("submit") + )} + </Button> + </div> + )} + </> + ); +}Then use it in the main component:
-{encounterId !== "preview" && ( - <> - <div className="flex gap-4 items-center m-4 max-w-4xl"> - ... - </div> - {questionnaireForms.length > 0 && ( - <div className="flex justify-end gap-4 mx-4 mt-4 max-w-4xl"> - ... - </div> - )} - </> -)} +{encounterId !== "preview" && ( + <FormActions + questionnaireForms={questionnaireForms} + subjectType={subjectType} + onSelect={(selected) => { + if (questionnaireForms.some((form) => form.questionnaire.id === selected.id)) { + return; + } + setQuestionnaireForms((prev) => [ + ...prev, + { + questionnaire: selected, + responses: initializeResponses(selected.questions), + errors: [], + }, + ]); + }} + onCancel={onCancel} + onSubmit={handleSubmit} + isPending={isPending} + hasErrors={hasErrors} + /> +)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
eslint.config.mjs(1 hunks)public/locale/en.json(8 hunks)src/components/Questionnaire/CloneQuestionnaireSheet.tsx(3 hunks)src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx(3 hunks)src/components/Questionnaire/QuestionRenderer.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/DiagnosisQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/MedicationStatementQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionTypes/SymptomQuestion.tsx(1 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(21 hunks)src/components/Questionnaire/QuestionnaireForm.tsx(1 hunks)src/components/Questionnaire/QuestionnaireList.tsx(3 hunks)src/components/Questionnaire/show.tsx(3 hunks)src/components/ui/input.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/Questionnaire/show.tsx
🔇 Additional comments (31)
src/components/Questionnaire/QuestionTypes/MedicationRequestQuestion.tsx (1)
94-104: LGTM! Good optimization to prevent unnecessary API calls.The addition of the
enabledproperty to prevent API calls during preview mode is a good performance optimization.src/components/Questionnaire/QuestionTypes/AllergyQuestion.tsx (1)
116-125: LGTM! Efficient handling of preview mode.The addition of the
enabledproperty to prevent unnecessary API calls during preview mode is a good optimization.src/components/Questionnaire/QuestionnaireEditor.tsx (16)
3-11: New icon imports from lucide-react.
The newly introduced icons appear properly used in this file (e.g.,SquarePenIconfor "Preview form"). No issues noted.
17-17:cnimport usage.
Thecnhelper is used consistently to conditionally merge class names. No concerns here.
79-79: Import ofQuestionnaireTagModel.
The import is utilized in newly introduced tag-related selectors. This inclusion is valid.
82-85: Imports for sheets.
CloneQuestionnaireSheetandManageQuestionnaireTagsSheetare referenced later. All good.
149-185:StatusSelectorimplementation.
Nicely encapsulates radio button logic for questionnaire status (active,draft,retired). The code is clear, with proper translation for each label.
187-229:SubjectTypeSelectorimplementation.
Similar toStatusSelector, this well-structured radio button group is easy to extend in the future (e.g., more subject types).
231-342:OrganizationSelectorlogic.
Provides a read-only view whenidis specified (edit mode), and a dynamic selection view when creating a new questionnaire. This is a clean approach.
448-513:QuestionnairePropertiescomponent.
Good breakdown of different property selectors (status, subject type, organizations, tags). The 'version' field is disabled, so if versioning is changed in the future, ensure there's a path for updates.
515-603:LAYOUT_OPTIONSandLayoutOptionCard.
Defines multiple grid layouts for grouping sub-questions. This is robust and user-friendly.
611-613: States for tags and tag search query.
Like organization states, these are well-defined. No immediate concerns.
628-656: Data fetching hooks for organizations and tags.
The react-query usage is consistent with the rest of the codebase. Consider advanced caching strategies if data grows large.
819-820: Styling changes for the editor layout.
Adopting a responsive split (flex-col md:flex-row) is a good approach for multi-device support.
1006-1008: Styling updates and newindexprop usage forQuestionEditor.
Allows better control over question ordering and highlighting. Implementation looks sensible.Also applies to: 1009-1058
1197-1220: Collapsible header and expanded state changes.
Clear toggling behavior usingChevronsDownUpvs.ChevronsUpDown. No issues.
1466-1505: New group layout options.
Encapsulated in aRadioGroupto setcontainerClasses. This fosters flexible nested layouts and is well done.
1645-1732: Sub-question management under type "group".
Matches the top-level question approach, allowing indefinite nesting. Implementation is straightforward and consistent.src/components/ui/input.tsx (1)
16-21: Automatic date picker invocation.
CallingshowPicker()is convenient for date inputs, but not all browsers support it. Consider a fallback for broader compatibility, or confirm that the target user base supports this feature.eslint.config.mjs (1)
69-70: Changed lint rules from "error" to "warn".
Lowering severity for@typescript-eslint/no-unused-varsand@typescript-eslint/no-unused-expressionsmay allow minor code smells to accumulate. Verify this is intentional for development flexibility.Also applies to: 77-78
src/components/Questionnaire/QuestionnaireList.tsx (2)
3-10: LGTM! Well-structured imports.The imports are well-organized, separating UI components and icons. The addition of lucide-react icons and UI components enhances the visual experience.
Also applies to: 14-21
58-90: Great implementation of filtering functionality!The implementation of tabs and search provides a clean and intuitive way to filter questionnaires. The use of flex layout ensures good responsiveness.
src/components/Questionnaire/CloneQuestionnaireSheet.tsx (1)
215-217: LGTM! Improved validation logic.The addition of organization selection validation ensures data integrity.
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx (2)
73-74: LGTM! Consistent query key naming.The update to use "questionnaireDetail" as the query key maintains consistency across components.
155-280: Great implementation of scrollable content!The ScrollArea implementation improves the user experience by handling overflow content elegantly while maintaining accessibility.
public/locale/en.json (6)
658-658: New Localization Entry for Creating a Questionnaire Added.
The new key"create_questionnaire": "Create Questionnaire"is clear, concise, and consistent with existing naming conventions for similar actions. Please verify that this key is correctly referenced in the questionnaire creation UI components.
1390-1390: New Localization Entry for Navigation Added.
The entry"navigation": "Navigation"is straightforward and fits well within the context of interface labels. Ensure that any updated navigation components in the redesign properly consume this key for internationalization.
1749-1749: New Localization Key for Properties Added.
The"properties": "Properties"entry follows the same style as other keys in the file. Please confirm that UI elements that display properties are updated to use this new key where appropriate.
1885-1885: New Localization Key for Retired Status Added.
The"retired": "Retired"entry is succinct and clear. Verify that any status indicators or filters that denote a “retired” state are now referring to this key to maintain consistency across the application.
1968-1968: New Localization Key for Searching Questionnaires Added.
The entry"search_questionnaires": "Search Questionnaires"is descriptive and aligned with other search-related keys. Please double-check that the questionnaire search functionality leverages this key to display accurate UI text.
2025-2025: New Localization Key for Subject Type Selection Added.
The"select_subject_type": "Select Subject Type"entry is clear and user-friendly. Ensure that any dropdowns or selection components in the questionnaire editing process are updated to use this key.
|
|
…quest-ui-redesign
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/components/Questionnaire/QuestionnaireEditor.tsx (2)
103-147: LGTM! Well-structured interfaces with clear type definitions.The interfaces are well-organized and provide good type safety. The
QuestionnairePropertiesPropsinterface effectively encapsulates all the necessary properties for organization and tag management.Consider splitting the large
QuestionnairePropertiesPropsinterface into smaller, focused interfaces for better maintainability:+interface SelectionConfig<T> { + selectedIds: string[]; + onToggle: (id: string) => void; + searchQuery: string; + setSearchQuery: (query: string) => void; + available?: T; + isLoading?: boolean; +} + interface QuestionnairePropertiesProps { questionnaire: QuestionnaireDetail; updateQuestionnaireField: <K extends keyof QuestionnaireDetail>( field: K, value: QuestionnaireDetail[K], ) => void; id?: string; organizations?: OrganizationResponse; - organizationSelection: { - selectedIds: string[]; - onToggle: (orgId: string) => void; - searchQuery: string; - setSearchQuery: (query: string) => void; - available?: OrganizationResponse; - isLoading?: boolean; - }; + organizationSelection: SelectionConfig<OrganizationResponse>; tags?: QuestionnaireTagModel[]; - tagSelection: { - selectedIds: string[]; - onToggle: (tagId: string) => void; - searchQuery: string; - setSearchQuery: (query: string) => void; - available?: TagResponse; - isLoading?: boolean; - }; + tagSelection: SelectionConfig<TagResponse>; }
231-446: Reduce code duplication in selector components.The
OrganizationSelectorandTagSelectorcomponents share very similar code patterns, especially in their Command components and Badge rendering logic.Consider creating a reusable
EntitySelectorcomponent:interface EntitySelectorProps<T> { id?: string; entities?: T[]; selection: { selectedIds: string[]; onToggle: (id: string) => void; searchQuery: string; setSearchQuery: (query: string) => void; available?: { results: T[] }; isLoading?: boolean; }; labels: { none: string; search: string; notFound: string; manage: string; }; renderBadge: (entity: T) => React.ReactNode; renderItem: (entity: T) => React.ReactNode; ManageSheet: React.ComponentType<any>; }This would reduce duplication and make the code more maintainable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Questionnaire/QuestionnaireEditor.tsx(21 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (2)
src/components/Questionnaire/QuestionnaireEditor.tsx (2)
819-1089: Well-implemented responsive layout system.The three-column layout with proper responsive breakpoints is well-implemented. The navigation and properties panels are appropriately hidden/shown based on screen size.
1466-1504: Excellent implementation of group layout options.The layout options for group questions are well-implemented with:
- Clear visual previews
- Responsive grid layout
- Consistent styling with the design system
- Good use of RadioGroup for option selection
|
@nihal467 Done |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/components/Questionnaire/QuestionnaireEditor.tsx (3)
809-817: Improve icon and text alignment in tab triggers.The icon and text alignment in tab triggers could be improved for better visual consistency.
- <TabsTrigger value="edit"> - <ViewIcon className="w-4 h-4 mr-2" /> - Edit form - </TabsTrigger> - <TabsTrigger value="preview"> - <SquarePenIcon className="w-4 h-4 mr-2" /> - Preview form - </TabsTrigger> + <TabsTrigger value="edit" className="flex items-center"> + <ViewIcon className="w-4 h-4 mr-2" /> + <span>Edit form</span> + </TabsTrigger> + <TabsTrigger value="preview" className="flex items-center"> + <SquarePenIcon className="w-4 h-4 mr-2" /> + <span>Preview form</span> + </TabsTrigger>
846-847: Enhance navigation button accessibility.The navigation button could benefit from improved accessibility.
- className={`w-full text-left px-3 py-2 text-sm rounded-md hover:bg-gray-200 flex items-center gap-2 ${ + className={`w-full text-left px-3 py-2 text-sm rounded-md hover:bg-gray-200 focus:ring-2 focus:ring-primary-500 focus:outline-none flex items-center gap-2 ${
1681-1685: Enhance sub-question container styling.The sub-question container styling is well-structured but could benefit from visual hierarchy improvements.
- <div - key={subQuestion.id} - id={`question-${subQuestion.id}`} - className="relative bg-white rounded-lg shadow-md" - > + <div + key={subQuestion.id} + id={`question-${subQuestion.id}`} + className="relative bg-white rounded-lg shadow-md transition-shadow hover:shadow-lg" + >public/locale/en.json (4)
658-658: New Localization Key for "Create Questionnaire"
The key"create_questionnaire": "Create Questionnaire"is added to support the new questionnaire creation UI. The wording is clear and aligns with existing naming conventions.
1749-1749: New "Properties" Entry
The new key"properties": "Properties"provides a label for properties. Ensure that the term “Properties” is contextually clear to users (e.g., whether referring to application settings, questionnaire properties, etc.).
2026-2026: New "Select Subject Type" Prompt
The entry"select_subject_type": "Select Subject Type"is introduced to facilitate user prompts when picking a subject type. Please verify that the usage distinguishes it from any non-prompt labels and that an asterisk or other indicator is applied where required by design.
2135-2135: New "Subject Type" Label
The key"subject_type": "Subject Type"is added. There is potential overlap with"select_subject_type"; please double-check that they are used in distinct contexts (e.g., one for a dropdown prompt vs. a plain label) to avoid redundancy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(8 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(21 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (8)
src/components/Questionnaire/QuestionnaireEditor.tsx (3)
740-740: Remove console.log statement.Remove debugging console.log statement from production code.
- console.log("Update Questionnaire", questionnaire);
821-823: Improve responsive layout structure.The layout structure has been improved with responsive classes, but there's room for better organization.
The use of
md:flex-rowandmd:w-60for the navigation sidebar, along with the transparent card styling, creates a clean and responsive layout. The header alignment withflex-row items-center justify-betweenis also well implemented.Also applies to: 968-969
1468-1506: Well-implemented group layout options.The group layout options implementation is clean and user-friendly.
The use of radio group with visual previews for layout options provides excellent UX. The implementation includes:
- Clear visual feedback with preview
- Proper styling for selected state
- Responsive grid layout
- Accessible form controls
public/locale/en.json (5)
1390-1390: Addition of "Navigation" Label
The entry"navigation": "Navigation"is introduced. Verify that its usage in the UI aligns with the intended static label for the navigation section as mentioned in the redesign feedback.
1885-1885: Addition of "Retired" Label
The entry"retired": "Retired"has been added. This should be consistent with similar status labels in the system. Confirm its usage across components that indicate a retired state.
1966-1966: New "Search Organizations" Localization Key
The key"search_organizations": "Search Organizations"is added. This entry complements other search functionality keys and is worded clearly.Likely an incorrect or invalid review comment.
1969-1969: New "Search Questionnaires" Entry
The key"search_questionnaires": "Search Questionnaires"is now available. Ensure that its styling and usage in the UI are consistent with similar search keys for entities like patients and facilities.
2372-2372: Addition of "Version" Label
The entry"version": "Version"is provided. This key will likely support display of the application's version info (for example, in a footer or about section). Its simplicity is appropriate.
There was a problem hiding this comment.
@amjithtitus09, it’s not redirecting back after a successful edit. Should it?
Line 675
…quest-ui-redesign
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/components/Questionnaire/QuestionnaireEditor.tsx (1)
671-681:⚠️ Potential issueAdd navigation after successful update.
Based on the past review comment, the code doesn't redirect after a successful edit. Consider adding navigation similar to the create flow.
const { mutate: updateQuestionnaire, isPending: isUpdating } = useMutation({ mutationFn: mutate(questionnaireApi.update, { pathParams: { id: id! }, }), onSuccess: () => { toast.success("Questionnaire updated successfully"); + navigate(`/admin/questionnaire/${id}`); }, onError: (_error) => { toast.error("Failed to update questionnaire"); }, });
🧹 Nitpick comments (4)
src/components/Questionnaire/QuestionnaireEditor.tsx (4)
103-107: Consider making the organization name more descriptive.The Organization interface could benefit from additional validation or constraints on the name field, such as minimum length or format requirements, to ensure meaningful organization names.
interface Organization { id: string; - name: string; + name: string; // @minLength(2) @pattern(^[a-zA-Z0-9\s-]+$) description?: string; }
473-482: Add aria-required attribute for better accessibility.The organization selector is marked as required with an asterisk, but missing the aria-required attribute for screen readers.
<Label> {t("organizations")} <span className="text-red-500">*</span> </Label> <OrganizationSelector + aria-required="true" id={id} organizations={organizations} selection={organizationSelection} />
1479-1481: Add type safety for styling metadata.The styling metadata access could be more type-safe by providing a default value and type checking.
-question.styling_metadata?.containerClasses || -LAYOUT_OPTIONS[0].value +question.styling_metadata?.containerClasses ?? +LAYOUT_OPTIONS[0].value as string
967-1063: Consider extracting question list into a separate component.The question list rendering logic is complex and could be extracted into a separate component for better maintainability and reusability.
Consider creating a
QuestionListcomponent to handle the rendering and management of questions, which would help reduce the complexity of the main component.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json(8 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(20 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (7)
public/locale/en.json (7)
659-659: New i18n entry: "create_questionnaire"
The key"create_questionnaire": "Create Questionnaire"is clear and concise. Please ensure that the corresponding UI elements now use this key for their label.
1391-1391: New i18n entry: "navigation"
The entry"navigation": "Navigation"fits well for labeling the navigation section. Verify that its usage is consistent with other similar UI elements in your application.
1750-1750: New i18n entry: "properties"
The key"properties": "Properties"adds a useful label for property sections. Ensure that it is referenced wherever a properties field or panel is displayed to maintain consistency across the UI.
1886-1886: New i18n entry: "retired"
The introduction of"retired": "Retired"appears well-named for indicating a retired status. Please double-check that it aligns with the overall status terminology used in the app.
1967-1967: New i18n entry: "search_organizations"
The new key"search_organizations": "Search Organizations"provides a clear prompt for searching organizations. Confirm that all UI components utilizing organization search reference this key to ensure a unified experience.
1970-1970: New i18n entry: "search_questionnaires"
The key"search_questionnaires": "Search Questionnaires"is appropriate for the redesigned questionnaire listing functionality. Make sure that this new entry is used in the search input or filter related to questionnaires.
2027-2027: New i18n entry: "select_subject_type"
The added entry"select_subject_type": "Select Subject Type"is self-explanatory and will aid in user form interactions. Verify that wherever the subject type selection is required in the UI, this key is properly implemented.
Deploying care-fe with
|
| Latest commit: |
70a93a2
|
| Status: | ⚡️ Build in progress... |
Deploying care-fe with
|
| Latest commit: |
70a93a2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d7d14c4a.care-fe.pages.dev |
| Branch Preview URL: | https://quest-ui-redesign.care-fe.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx (2)
155-155: Consider using a more flexible height calculation.The current height calculation using
h-[calc(100vh-11rem)]might not work well across all screen sizes and devices. Consider using a more flexible approach or adding responsive breakpoints.- <ScrollArea className="h-[calc(100vh-11rem)]"> + <ScrollArea className="h-[calc(100vh-11rem)] sm:h-[calc(100vh-13rem)] lg:h-[calc(100vh-15rem)]">
283-284: Improve footer styling and positioning.The footer styling can be improved:
- The gray background color makes the border less visible
- With ScrollArea, some positioning classes might be redundant
- <SheetFooter className=" bottom-0 left-0 right-0 p-4 border-t"> - <div className="flex w-full justify-end gap-4 bg-gray-100"> + <SheetFooter className="p-4 border-t bg-white"> + <div className="flex w-full justify-end gap-4">src/components/Questionnaire/QuestionnaireEditor.tsx (3)
102-141: Add JSDoc documentation to interfaces.The new interfaces are well-structured but would benefit from JSDoc documentation to improve code maintainability and IDE support.
Add documentation like this:
+/** + * Represents an organization entity + * @interface Organization + * @property {string} id - Unique identifier for the organization + * @property {string} name - Name of the organization + * @property {string} [description] - Optional description of the organization + */ interface Organization { id: string; name: string; description?: string; }
467-469: Add aria-required attribute for better accessibility.The organization field is marked as required with an asterisk, but should also include the aria-required attribute for better accessibility.
- <Label> + <Label aria-required="true"> {t("organizations")} <span className="text-red-500">*</span> </Label>
510-561: Memoize layout options for better performance.The LAYOUT_OPTIONS constant includes JSX elements in the preview property. These could be memoized to prevent unnecessary re-renders.
Consider using useMemo for the preview components:
+import { useMemo } from 'react'; -const LAYOUT_OPTIONS = [ +const LAYOUT_OPTIONS = useMemo(() => [ { id: "full-width", value: "grid grid-cols-1", label: "Full Width", preview: ( <div className="space-y-1 w-full"> <div className="h-2 w-full bg-gray-200 rounded" /> <div className="h-2 w-full bg-gray-200 rounded" /> </div> ), }, // ... other options -] as const; +], []) as const;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/components/Questionnaire/CloneQuestionnaireSheet.tsx(4 hunks)src/components/Questionnaire/ManageQuestionnaireOrganizationsSheet.tsx(1 hunks)src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx(3 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx(19 hunks)src/components/Resource/ResourceForm.tsx(1 hunks)src/pages/Encounters/tabs/EncounterNotesTab.tsx(3 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/components/Questionnaire/ManageQuestionnaireOrganizationsSheet.tsx
- src/pages/Encounters/tabs/EncounterNotesTab.tsx
- src/components/Resource/ResourceForm.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Questionnaire/CloneQuestionnaireSheet.tsx
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: Cloudflare Pages: care-fe
🔇 Additional comments (3)
src/components/Questionnaire/ManageQuestionnaireTagsSheet.tsx (2)
26-26: LGTM!The ScrollArea import is correctly placed and will help manage overflow content while maintaining a fixed footer.
73-73: LGTM!The query key change to "questionnaireDetail" maintains consistency with similar changes across the codebase.
src/components/Questionnaire/QuestionnaireEditor.tsx (1)
652-661: Add redirection after successful questionnaire creation.Based on the past review comment, the component doesn't redirect back after a successful edit. Consider adding navigation after successful updates.
Add navigation after successful update:
const { mutate: createQuestionnaire, isPending: isCreating } = useMutation({ mutationFn: mutate(questionnaireApi.create), onSuccess: (data: QuestionnaireDetail) => { toast.success("Questionnaire created successfully"); navigate(`/admin/questionnaire/${data.slug}`); }, onError: (_error) => { toast.error("Failed to create questionnaire"); }, }); const { mutate: updateQuestionnaire, isPending: isUpdating } = useMutation({ mutationFn: mutate(questionnaireApi.update, { pathParams: { id: id! }, }), onSuccess: () => { toast.success("Questionnaire updated successfully"); + navigate(`/admin/questionnaire/${id}`); }, onError: (_error) => { toast.error("Failed to update questionnaire"); }, });
| }); | ||
|
|
||
| const { data: availableTags, isLoading: isLoadingAvailableTags } = useQuery({ | ||
| queryKey: ["tags", tagSearchQuery], |
There was a problem hiding this comment.
let's keep the key more verbose. this could be mixed up with other tags (maybe patient tags when it comes up)
| queryKey: ["tags", tagSearchQuery], | |
| queryKey: ["questinnaire_tags", tagSearchQuery], |
There was a problem hiding this comment.
|
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
…quest-ui-redesign
Deploying care-fe with
|
| Latest commit: |
0394f0f
|
| Status: | ⚡️ Build in progress... |
|
@amjithtitus09 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! 🙌 |
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Style