Adds support for searching for OrganizationSelector and fix scroll for dropdown in sheet
#9620
Conversation
WalkthroughThe pull request introduces enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Deploying care-fe with
|
| Latest commit: |
2c2c87f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a10e773c.care-fe.pages.dev |
| Branch Preview URL: | https://rithviknishad-enhancements.care-fe.pages.dev |
rithviknishad
changed the title
OrganizationSelector and fix scroll for dropdown in sheet
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/components/ui/autocomplete.tsx (2)
30-30: Check for optionalonSearchbefore invocation
Currently,onSearchis defined as optional, but the code does not guard against the case where it may be undefined. Consider adding a safety check (e.g.,onSearch?.(value)) in any place where the function is called to avoid possible runtime errors.
75-75: Use safe invocation foronValueChange
IfonValueChangeis bound directly toonSearch, ensure thatonSearchis valid (e.g., viaonSearch?.(val)) to prevent errors whenonSearchis not provided.src/pages/Organization/components/OrganizationSelector.tsx (1)
37-37: Adjust the 500ms debounce if needed
A 500ms debounce might be slightly high for fast typists, but it’s generally acceptable. If user feedback suggests decreased responsiveness, consider lowering it to around 300ms.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/ui/autocomplete.tsx(3 hunks)src/pages/Organization/components/OrganizationSelector.tsx(5 hunks)
🔇 Additional comments (6)
src/components/ui/autocomplete.tsx (2)
40-40: Prop grouping looks good
Passing onSearch and other props together follows a clear pattern. No issues here.
48-48: Verify modal={true} usage for Popover
Using modal mode can significantly change focus and keyboard interaction. Ensure that this behavior is expected. If not, consider removing or setting it to false.
src/pages/Organization/components/OrganizationSelector.tsx (4)
10-11: New import for useDebouncedState
The introduction of useDebouncedState is a neat approach to controlling the frequency of queries. Consider ensuring the hook is well-tested for different edge cases (e.g., empty string, rapidly changing input).
48-53: Good practice including searchQuery in queryKey
Including searchQuery in queryKey ensures the query result properly reflects the user input. This approach is essential for correct React Query caching and refetch behavior.
65-71: Consistent application of searchQuery for nested organizations
Mirroring the same logic for the child-level organizations keeps the UI consistent. Continue verifying that the derived organizations truly reflect all search-related updates.
162-162: Directly passing setSearchQuery to onSearch
This direct pass works well, especially with the debounce approach. Just be mindful that no additional formatting of the search input is done before updating the state.
…re_fe into rithviknishad/enhancements
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/hooks/useSlug.ts (1)
20-20: Consider consistency in the deprecation message.The
@deprecatedtag foruseSlugsis slightly different than foruseSlug. Verify that this minor formatting difference in documentation is intentional. Ideally, both deprecation tags should consistently reference the new function.Proposed minimal fix:
- * * @deprecated use `usePathParams` instead + * @deprecated use `usePathParams` instead
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locale/en.json(4 hunks)public/locale/hi.json(2 hunks)public/locale/kn.json(2 hunks)public/locale/ml.json(2 hunks)public/locale/ta.json(2 hunks)src/components/ui/sidebar/app-sidebar.tsx(3 hunks)src/hooks/useSlug.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- public/locale/en.json
🔇 Additional comments (11)
src/hooks/useSlug.ts (1)
5-5: Documentation updates look good!
The deprecation notice is helpful in guiding developers toward usePathParams. Ensure that the transition plan (including migration guides) is clearly communicated in the project's documentation so that future maintainers know these functions will eventually be removed or replaced.
src/components/ui/sidebar/app-sidebar.tsx (6)
44-46: Acknowledgment of the TODO and optional user parameter
Including the optional user parameter is a good approach to keep the function flexible until the serializer update. The inline TODO reminds fellow developers of the pending change, which helps maintain overall clarity and direction in the code.
50-50: Ensure link ordering and naming consistency
When defining navigation links, consider how link ordering and naming (singular/plural) are structured across the application. Consistency aids user familiarity. Currently, this looks consistent with app-wide conventions.
58-58: Pluralization alignment with i18n updates
Using t("search_patients") aligns well with the pluralized key introduced in the locale files. This helps maintain consistency across the interface.
69-69: Naming alignment for “organization”
Renaming the link to organization is consistent with the i18n changes. Ensure that any references to capitalized variants are also harmonized across the codebase.
75-81: Conditional sidebar link for “schedules”
Adding this link only if user is defined is sensible. In the future, consider verifying that user.username is not empty or undefined to prevent potential URL or logic errors. Overall, logic is solid here.
186-186: Propagating the user parameter to generateFacilityLinks
Passing the user object ensures that the newly added conditional “schedules” link is constructed accurately. This maintains the single-responsibility of generateFacilityLinks.
public/locale/hi.json (1)
676-676: Pluralization fix for search label
Changing "search_patient" to "search_patients" aligns the Hindi locale with the updated keys in the code. This maintains consistency across different languages and improves clarity for users.
public/locale/kn.json (1)
678-678: Pluralization fix for Kannada locale
Renaming "search_patient" to "search_patients" maintains the pluralized naming convention. This ensures consistency with other languages and the code references.
public/locale/ta.json (1)
676-676: LGTM! Consistent pluralization update.
The change from "search_patient" to "search_patients" aligns with the pluralization updates across localization files.
public/locale/ml.json (1)
677-677: LGTM! Consistent pluralization update.
The change from "search_patient" to "search_patients" maintains consistency with other localization files.
CARE
|
||||||||||||||||||||||||||||
| Project |
CARE
|
| Branch Review |
rithviknishad/enhancements
|
| Run status |
|
| Run duration | 01m 09s |
| Commit |
|
| Committer | Rithvik Nishad |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
3
|
| View all changes introduced in this branch ↗︎ | |
|
LGTM |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/ui/calendar.tsx (1)
65-66: Add an aria-label for improved accessibilityWhen icons are used as interactive elements (in this case, buttons), it's good practice to provide a label for screen readers. This helps visually impaired users navigate the calendar more easily.
Here is an example fix:
components={{ - PreviousMonthButton: () => <ChevronLeftIcon className="h-4 w-4" />, - NextMonthButton: () => <ChevronRightIcon className="h-4 w-4" />, + PreviousMonthButton: () => ( + <button type="button" aria-label="Previous Month"> + <ChevronLeftIcon className="h-4 w-4" /> + </button> + ), + NextMonthButton: () => ( + <button type="button" aria-label="Next Month"> + <ChevronRightIcon className="h-4 w-4" /> + </button> + ), }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/components/ui/calendar.tsx(1 hunks)
|
@rithviknishad 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
OrganizationSelector@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Improvements
Deprecations
useSluganduseSlugsfunctions in favor ofusePathParams