Feat: Display total no. of entities with heading in Organization #10092
Conversation
WalkthroughThis pull request introduces a consistent approach to displaying entity counts across various organization-related tabs. The changes involve modifying localization files and updating multiple components to include an Changes
Assessment against linked issues
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 (3)
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.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/pages/Organization/OrganizationView.tsx (2)
54-65: Maintain consistency with heading and translation keys.The heading shows "organizations" while viewing child organizations, which might be confusing. Consider changing it to "child organizations" or similar for clarity.
- <h2 className="text-lg font-semibold">{t("organizations")}</h2> + <h2 className="text-lg font-semibold">{t("child_organizations")}</h2>
57-58: Extract common Badge styling to a shared component.The Badge styling is duplicated across all organization pages. Consider creating a shared component to maintain consistency and reduce duplication.
// src/components/Common/EntityCountBadge.tsx export const EntityCountBadge = ({ count, entity }: { count: number; entity: string }) => ( <Badge className="bg-purple-50 text-purple-700 ml-2 text-sm font-medium rounded-xl px-3 m-3" variant="outline" > {t("entity_count", { count, entity })} </Badge> );Also applies to: 63-63
src/pages/Organization/OrganizationFacilities.tsx (1)
67-69: Standardize translation key usage.Use the common
entity_counttranslation key for consistency with other organization pages.- {t("facility_count", { - count: facilities?.count || 0, - })} + {t("entity_count", { + count: facilities?.count || 0, + entity: "Facility" + })}src/pages/Organization/OrganizationPatients.tsx (1)
102-110: Consider adding ARIA label for screen readers.The Badge component should have an aria-label for better accessibility.
<Badge className="bg-purple-50 text-purple-700 ml-2 text-sm font-medium rounded-xl px-3 m-3" variant="outline" + aria-label={t("entity_count", { + count: patients?.count || 0, + entity: "Patient" + })} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json(2 hunks)src/components/Facility/FacilityUsers.tsx(1 hunks)src/pages/Organization/OrganizationFacilities.tsx(2 hunks)src/pages/Organization/OrganizationPatients.tsx(2 hunks)src/pages/Organization/OrganizationUsers.tsx(2 hunks)src/pages/Organization/OrganizationView.tsx(1 hunks)
👮 Files not reviewed due to content moderation or server errors (2)
- src/components/Facility/FacilityUsers.tsx
- public/locale/en.json
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
public/locale/en.json (1)
987-988: Consider handling irregular plurals.The current pluralization simply adds 's' to the entity name, which works for regular English plurals but might not handle irregular cases (e.g., "person" → "people", "facility" → "facilities"). Consider using a more robust pluralization mechanism if irregular plurals are needed.
Example enhancement:
- "entity_count_other": "{{count}} {{entity}}s", + "entity_count_other": "{{count}} {{entityPlural}}"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
public/locale/en.json(2 hunks)src/components/Facility/FacilityUsers.tsx(1 hunks)src/pages/Organization/OrganizationFacilities.tsx(2 hunks)src/pages/Organization/OrganizationPatients.tsx(2 hunks)src/pages/Organization/OrganizationUsers.tsx(2 hunks)src/pages/Organization/OrganizationView.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/pages/Organization/OrganizationFacilities.tsx
- src/pages/Organization/OrganizationUsers.tsx
- src/components/Facility/FacilityUsers.tsx
- src/pages/Organization/OrganizationView.tsx
- src/pages/Organization/OrganizationPatients.tsx
🔇 Additional comments (1)
public/locale/en.json (1)
987-988: LGTM! Well-designed localization approach.The new generalized entity count entries provide a flexible and maintainable solution for displaying counts across different types of entities. The implementation follows i18n best practices for handling pluralization.
@coderabbitai, how to implement the nitpick comment |
|
Tip For best results, initiate chat on the files or code changes. @AdityaJ2305 The nitpick comment suggests improving the pluralization handling in
{
"entity_count_one": "{{count}} {{entity}}",
"entity_count_other": "{{count}} {{entityPlural}}"
}
t("entity_count", {
count: facilities?.count || 0,
entity: "Facility",
entityPlural: "Facilities"
})This approach will handle irregular plurals correctly (like "person"/"people") instead of just adding 's' to the singular form. Would you like me to help you implement this change across all the files? 🔗 Analysis chain🏁 Scripts executedThe following scripts were executed for the analysis: Script: Length of output: 5733 |
Hey Team, Do we really need to do that ? It just makes it too complex for simple translation |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/pages/Organization/OrganizationView.tsx (1)
57-66: Extract Badge styling to a reusable component.The Badge styling is duplicated across multiple components. Consider extracting it to a reusable component.
Create a new component
EntityCountBadge:// src/components/Common/EntityCountBadge.tsx interface EntityCountBadgeProps { count: number | undefined; entity: string; } export function EntityCountBadge({ count, entity }: EntityCountBadgeProps) { const { t } = useTranslation(); return ( <Badge className="bg-purple-50 text-purple-700 ml-2 text-sm font-medium rounded-xl px-3 m-3" variant="outline" > {t("entity_count", { count: count ?? 0, entity, defaultValue: "{{count}} {{entity}}" })} </Badge> ); }🧰 Tools
🪛 GitHub Check: cypress-run (1)
[failure] 61-61:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/Facility/FacilityUsers.tsx(1 hunks)src/pages/Organization/OrganizationFacilities.tsx(2 hunks)src/pages/Organization/OrganizationPatients.tsx(2 hunks)src/pages/Organization/OrganizationUsers.tsx(2 hunks)src/pages/Organization/OrganizationView.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Check: cypress-run (1)
src/components/Facility/FacilityUsers.tsx
[failure] 76-76:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
src/pages/Organization/OrganizationFacilities.tsx
[failure] 68-68:
Argument of type '["facility_count", { count: string | number; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
src/pages/Organization/OrganizationPatients.tsx
[failure] 107-107:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
src/pages/Organization/OrganizationUsers.tsx
[failure] 67-67:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
src/pages/Organization/OrganizationView.tsx
[failure] 61-61:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
🪛 GitHub Actions: Cypress Tests
src/components/Facility/FacilityUsers.tsx
[error] 76-76: Type error: Argument of type '['entity_count', { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'
🔇 Additional comments (2)
src/pages/Organization/OrganizationUsers.tsx (1)
67-71: Same issues as other components.This component has the same translation type error and Badge styling duplication issues as discussed in previous files.
🧰 Tools
🪛 GitHub Check: cypress-run (1)
[failure] 67-67:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.src/pages/Organization/OrganizationPatients.tsx (1)
107-111: Same issues as other components.This component has the same translation type error and Badge styling duplication issues as discussed in previous files.
🧰 Tools
🪛 GitHub Check: cypress-run (1)
[failure] 107-107:
Argument of type '["entity_count", { count: string | number; entity: string; }]' is not assignable to parameter of type '[key: string | string[], options: TOptionsBase & $Dictionary & { defaultValue: string; }] | [key: string | string[], defaultValue: string, options?: (TOptionsBase & $Dictionary) | undefined] | [key: ...]'.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
public/locale/en.json (1)
994-994: Consider handling irregular plurals.The current implementation adds 's' directly to {{entity}} which works for regular plurals but may not handle irregular plurals correctly (e.g., "person" -> "people", "child" -> "children").
Consider using a separate placeholder for plural form:
- "entity_count_other": "{{count}} {{entity}}s", + "entity_count_other": "{{count}} {{entityPlural}}",Then in the code, provide both forms:
t("entity_count", { count: total, entity: "Person", entityPlural: "People" })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/locale/en.json(2 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: CodeQL-Build
🔇 Additional comments (1)
public/locale/en.json (1)
993-994: LGTM! Well-structured pluralization implementation.The implementation follows i18n best practices by:
- Using standard plural key suffixes (_one, _other)
- Maintaining consistent placeholder naming
- Properly handling both singular and plural forms
|
LGTM |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
public/locale/en.json(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
- GitHub Check: CodeQL-Build
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/components/Questionnaire/QuestionnaireSearch.tsx (2)
Line range hint
91-91: Internationalize the search placeholder text.The search placeholder is currently hardcoded in English. Consider using a translation key for consistency with the internationalization improvements.
- placeholder="Search questionnaires..." + placeholder={t("search_questionnaires_placeholder")}
Line range hint
38-127: Enhance component accessibility.Consider the following accessibility improvements:
- Add aria-label to the search input
- Add aria-expanded state to the popover trigger
- Add role="combobox" to the search input
- Add aria-controls to link the input with the results list
<PopoverTrigger asChild> <Button data-cy="add-questionnaire-button" variant="outline" className="w-full justify-between" disabled={disabled || isLoading} + aria-expanded={isOpen} > ... <input className="flex h-11 w-full rounded-md bg-transparent py-3 text-sm outline-none placeholder:text-muted-foreground disabled:cursor-not-allowed disabled:opacity-50" placeholder={t("search_questionnaires_placeholder")} value={search} onChange={(e) => setSearch(e.target.value)} + role="combobox" + aria-expanded={isOpen} + aria-controls="questionnaire-search-results" + aria-label={t("search_questionnaires_label")} /> ... - <div className="grid gap-1 p-2"> + <div id="questionnaire-search-results" className="grid gap-1 p-2">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/Facility/FacilityUsers.tsx(3 hunks)src/components/Questionnaire/QuestionnaireSearch.tsx(1 hunks)src/pages/Organization/components/EntityBadge.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Facility/FacilityUsers.tsx
- src/pages/Organization/components/EntityBadge.tsx
🔇 Additional comments (1)
src/components/Questionnaire/QuestionnaireSearch.tsx (1)
74-74: LGTM! Good job on improving internationalization.The change to use the translation key for the loading text aligns well with the internationalization efforts.
|
@rithviknishad @Jacobjeevan , Can we merge this PR if any further changes are not required, it is already been tested and reviewed |
|
@AdityaJ2305 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
OrganizationTabs #10087@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
New Features
EntityBadgecomponent for displaying entity counts.Improvements
Localization