-
Notifications
You must be signed in to change notification settings - Fork 561
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
bugfix:Remove arrow icon from cancel button in Create Questionnaire #10773
base: develop
Are you sure you want to change the base?
bugfix:Remove arrow icon from cancel button in Create Questionnaire #10773
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (12)
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 (
|
6819c7d
to
0384bdc
Compare
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: 1
🧹 Nitpick comments (1)
public/locale/en.json (1)
1742-1742
: Added "preview" localization key.
The key"preview": "Preview"
provides a succinct label. However, note that similar keys (e.g."form_preview"
) exist—please verify if this might lead to duplicate or confusing UI references.Consider consolidating these similar keys if they serve the same purpose.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json
(11 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx
(25 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Questionnaire/QuestionnaireEditor.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: cypress-run (1)
🔇 Additional comments (9)
public/locale/en.json (9)
292-292
: Added localization key for "add_condition".
This new entry"add_condition": "Add Condition"
is clear, concise, and consistent with the style of similar action labels in the file.
310-310
: Added localization key for "add_option".
The entry"add_option": "Add Option"
is straightforward and matches the formatting of other similar keys.
318-318
: Added localization key for "add_question".
The value"Add Question"
is clear and appears to follow the same style as other keys; ensure that any UI component referencing this key is updated accordingly.
584-584
: Introduced "clone_questionnaire" localization key.
The added entry"clone_questionnaire": "Clone Questionnaire"
is intuitive and aligns with similar action phrases. Verify that the UI components correctly use this translatable string.
588-588
: Added localization key for "collect".
The new key"collect": "Collect"
is clear; please ensure that its use in the UI matches the intended action, as it follows the established label style.
814-814
: Added localization key for "edit_form".
The entry"edit_form": "Edit Form"
is concise and complies with the application's language consistency.
1097-1097
: Added localization key for "form_preview".
The new key"form_preview": "Preview form"
effectively captures the intended UI text for form preview functionality.
1773-1773
: Added localization key for "question".
The new key"question": "Question"
is self-explanatory and follows the pattern of similar terms in the file.
1837-1837
: Added localization key for "repeatable".
The entry"repeatable": "Repeatable"
is concise and clear. Make sure that its usage in the questionnaire components correctly reflects its intent.
e1ae475
to
9517093
Compare
Conflicts have been detected against the base branch. Please merge the base branch into your branch.
|
9517093
to
172d323
Compare
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently, please check "Code review limits" under "Moderation" settings.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/components/ValueSet/ValueSetForm.tsx (1)
104-116
: 🛠️ Refactor suggestionMissing internationalization in ConceptFields component.
While translations were added to the main form, the ConceptFields component still uses hardcoded strings.
Add translations for all user-facing strings:
<div className="flex items-center justify-between"> - <h4 className="text-sm font-medium">Concepts</h4> + <h4 className="text-sm font-medium">{t("concepts")}</h4> <Button type="button" variant="outline" size="sm" onClick={() => append({ code: "", display: "" })} > <PlusIcon className="h-4 w-4 mr-2" /> - Add Concept + {t("add_concept")} </Button> </div>
🧹 Nitpick comments (12)
src/components/Patient/symptoms/SymptomTable.tsx (1)
113-117
: Consider using a more consistent date formatThe implementation for displaying the onset date works well, but consider using a more standardized date formatting approach for consistency across the application.
- {symptom.onset?.onset_datetime - ? new Date(symptom.onset.onset_datetime).toLocaleDateString() - : "-"} + {symptom.onset?.onset_datetime + ? new Date(symptom.onset.onset_datetime).toLocaleDateString(undefined, { + year: 'numeric', + month: 'short', + day: 'numeric' + }) + : "-"}src/components/Common/AvatarEditModal.tsx (1)
236-240
: Good security practice for blob URL handlingSelectively sanitizing blob URLs while leaving backend-provided imageUrls untouched is a good approach based on trust boundaries. According to retrieved learnings, the
imageUrl
prop is fetched from the backend and considered safe.However, consider using optional chaining for better null/undefined safety:
-src={ - preview && preview.startsWith("blob:") - ? DOMPurify.sanitize(preview) - : imageUrl -} +src={ + preview?.startsWith("blob:") + ? DOMPurify.sanitize(preview) + : imageUrl +}🧰 Tools
🪛 Biome (1.9.4)
[error] 237-237: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
src/pages/Facility/settings/devices/UpdateDevice.tsx (1)
42-44
: Consider using React Router's navigation methods instead of window.history.Using
window.history.back()
works but isn't aligned with React Router's navigation model.- onSuccess={() => { - window.history.back(); - }} + onSuccess={() => { + navigate(-1); // Assuming you're using React Router's useNavigate hook + }}src/pages/Facility/settings/devices/DevicesList.tsx (2)
27-28
: Consider making the page limit configurable.The limit of 12 devices per page is hardcoded, which reduces flexibility.
Consider making this a configurable value, either through props, context, or user preferences:
- const limit = 12; + const limit = props.itemsPerPage || 12;
79-81
: Pagination component usage can be simplified.The onChange handler has an unused parameter.
<Pagination data={{ totalCount: data.count }} - onChange={(page, _) => setPage(page)} + onChange={(page) => setPage(page)} defaultPerPage={limit} cPage={page} />src/pages/Facility/settings/devices/DeviceDetail.tsx (1)
105-122
: Improve URL validation in contact link generation.The function doesn't validate URL formats when handling contacts of type 'url', which could lead to invalid links.
case "url": - return value; + // Basic URL validation + return value.startsWith('http://') || value.startsWith('https://') + ? value + : `https://${value}`;src/components/ValueSet/ValueSetForm.tsx (1)
344-396
: Consider using the hook-based translation approach in the schema.The schema is using the global
t
function directly, which might cause issues in certain contexts where the translation context isn't properly initialized.Consider creating the schema inside the component to use the hook-based approach:
-const valuesetFormSchema = z.object({...}); export function ValueSetForm({ initialData, onSubmit, isSubmitting, }: ValueSetFormProps) { const { t } = useTranslation(); + const valuesetFormSchema = z.object({ + name: z.string().min(1, t("field_required")), + // rest of the schema + }); const form = useForm<ValuesetFormType>({ resolver: zodResolver(valuesetFormSchema), // ... }); // ... }src/types/location/location.ts (1)
45-61
: Consider adding comments explaining the abbreviationsThe location form options use abbreviated codes ("si", "bu", "wi", etc.) that aren't self-explanatory. Consider adding comments explaining what each code represents to improve code readability and maintainability.
export const LocationFormOptions = [ - "si", - "bu", - "wi", - "wa", - "lvl", - "co", - "ro", - "bd", - "ve", - "ho", - "ca", - "rd", - "area", - "jdn", - "vi", + "si", // Site + "bu", // Building + "wi", // Wing + "wa", // Ward + "lvl", // Level + "co", // Corridor + "ro", // Room + "bd", // Bed + "ve", // Vehicle + "ho", // House + "ca", // Cabinet + "rd", // Road + "area", // Area + "jdn", // Junction + "vi", // Virtual ] as const;src/types/device/device.ts (1)
24-39
: Remove or uncomment commented codeThere's a commented-out field (
care_type
) in the DeviceBase interface. Either remove it completely or uncomment it if it's needed for future implementation.export interface DeviceBase { identifier?: string; status: DeviceStatus; availability_status: DeviceAvailabilityStatus; manufacturer?: string; manufacture_date?: string; // datetime expiration_date?: string; // datetime lot_number?: string; serial_number?: string; registered_name: string; user_friendly_name?: string; model_number?: string; part_number?: string; contact: ContactPoint[]; - // care_type: string | undefined; }
public/locale/en.json (3)
292-298
: Verify Consistency in "Add" Action Keys
New localization entries for"add_condition"
,"add_consultation"
,"add_consultation_update"
,"add_contact_point"
, and"add_device"
have been introduced. Please verify that the capitalization is consistent across these keys (e.g."Add consultation"
uses a lowercase “c” while"Add Condition"
uses uppercase “C”).
314-322
: Check Consistency for "Add Option/Question" Keys
The entries"add_option"
and"add_question"
have been added. Ensure these follow the same wording/format conventions used for similar “add_…” keys (for example, decide whether the action words should be uniformly capitalized) and that they reflect the intended UI copy.
2297-2299
: Review Deletion Confirmation Warning Messages
The keys:
"this_will_permanently_remove_the_exception_and_cannot_be_undone"
"this_will_permanently_remove_the_scheduled_template_and_cannot_be_undone"
"this_will_permanently_remove_the_session_and_cannot_be_undone"
are introduced to warn users about irreversible actions. While the messages are clear and explicit, consider whether their length is optimal for UI dialogs. If needed, consider a slight rephrasing for brevity without losing clarity.
🛑 Comments failed to post (2)
src/pages/Facility/settings/devices/DevicesList.tsx (1)
47-52:
⚠️ Potential issueRoute to create device should include facilityId.
The Link component's href is using a static path "/devices/create" which doesn't include the facilityId parameter. This could cause issues when navigating to the device creation page.
Consider updating the Link to include the facilityId parameter:
<Button variant="primary" asChild> - <Link href="/devices/create"> + <Link href={`/facility/${facilityId}/settings/devices/create`}> <CareIcon icon="l-plus" className="h-4 w-4 mr-2" /> {t("add_device")} </Link> </Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Button variant="primary" asChild> <Link href={`/facility/${facilityId}/settings/devices/create`}> <CareIcon icon="l-plus" className="h-4 w-4 mr-2" /> {t("add_device")} </Link> </Button>
src/pages/Facility/settings/devices/DeviceDetail.tsx (1)
147-148:
⚠️ Potential issueDevice edit link is missing facilityId parameter.
The Link to edit the device doesn't include the facilityId, which might cause routing issues.
-<Link href={`/devices/${deviceId}/edit`}> +<Link href={`/facility/${facilityId}/settings/devices/${deviceId}/edit`}> <Button variant="outline">{t("edit")}</Button> </Link>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.<Link href={`/facility/${facilityId}/settings/devices/${deviceId}/edit`}> <Button variant="outline">{t("edit")}</Button> </Link>
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
🧹 Nitpick comments (1)
public/locale/en.json (1)
604-604
: New Localization Key: "collect"
The key"collect": "Collect"
is added, presumably to label an action that gathers data or responses. While the label is concise, please verify that “Collect” unambiguously conveys the intended functionality in the context of your interface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
public/locale/en.json
(11 hunks)src/components/Questionnaire/QuestionnaireEditor.tsx
(25 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Questionnaire/QuestionnaireEditor.tsx
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Test
- GitHub Check: cypress-run (1)
🔇 Additional comments (10)
public/locale/en.json (10)
295-295
: New Localization Key: "add_condition"
The key"add_condition": "Add Condition"
is introduced to support UI functionality (e.g. adding a condition in a questionnaire). The naming convention and value are clear and consistent with similar keys.
314-314
: New Localization Key: "add_option"
The addition of"add_option": "Add Option"
follows the established pattern and provides a clear label for options within forms.
322-322
: New Localization Key: "add_question"
The key"add_question": "Add Question"
is added for enabling users to add questions in the questionnaire. Its phrasing is direct and aligns with the overall UI language.
599-599
: New Localization Key: "clone_questionnaire"
The key"clone_questionnaire": "Clone Questionnaire"
is provided to label an action that duplicates a questionnaire. Ensure that this term is used consistently in the UI and does not conflict with any similar action labels.
867-867
: New Localization Key: "edit_form"
The addition of"edit_form": "Edit Form"
is straightforward and matches the UI’s tone for form actions. It is consistent with similar keys already present in the file.
1164-1164
: New Localization Key: "form_preview"
The key"form_preview": "Preview form"
provides a clear label for the preview functionality. Ensure that its usage in the application is not confused with other similar terms (for example, review any potential redundancy with keys like"previous_form"
noted in past reviews).
1484-1484
: New Localization Key: "move_down"
The key"move_down": "Move Down"
has been added for UI interactions involving the reordering of elements. Its language is clear and fits with the application’s style.
1486-1486
: New Localization Key: "move_up"
Similarly,"move_up": "Move Up"
provides a label for upward movement actions. It is consistent with the “move_down” key and with the overall language of the platform.
1937-1937
: New Localization Key: "repeatable"
The key"repeatable": "Repeatable"
is introduced likely to indicate that a particular question or element can be repeated. Its wording is simple and fits well within the UI context.
1581-1581
: New Localization Key: "no_requested_questionnaires_found"
This key with the text"The requested questionnaire could not be found."
offers a clear and user-friendly error message when a questionnaire cannot be retrieved. It is well formulated for localization purposes.
172d323
to
f0ec2dd
Compare
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.
Clear merge conflict.
f0ec2dd
to
d6f192c
Compare
Proposed Changes
QuestionnaireEditor.tsx
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Style