Fix: Minor UI changes #10056
Fix: Minor UI changes #10056
Conversation
WalkthroughThe pull request introduces focused UI enhancements across multiple components. The changes primarily involve modifying focus ring behaviors in dialog and sheet components by replacing Changes
Assessment against linked issues
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. |
![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 (2)
src/pages/FacilityOrganization/components/FacilityOrganizationSelector.tsx (2)
35-42: Consider maintaining query flexibility for future requirements.While the current implementation works for single organization selection, hardcoding
parent: ""might limit future extensibility if hierarchical organization selection is needed later.Consider parameterizing the parent filter:
- parent: "", + parent: props.parentId || "",
71-89: Add loading state handling for better UX.The UI implementation looks clean, but it should handle loading states to prevent potential UI flickers or unresponsive appearances during data fetching.
Consider adding loading states:
- const { data: organizations } = useQuery<{ + const { data: organizations, isLoading } = useQuery<{ results: FacilityOrganization[]; }>({ return ( <> <Label className="mb-2"> {t("select_department")} {required && <span className="text-red-500">*</span>} </Label> <div className="space-y-4"> <Autocomplete value={selectedOrg} options={getOrganizationOptions(organizations?.results)} onChange={handleOrgChange} placeholder={t("select_department")} + disabled={isLoading} + loading={isLoading} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/ui/dialog.tsx(1 hunks)src/components/ui/sheet.tsx(1 hunks)src/pages/FacilityOrganization/components/FacilityOrganizationSelector.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (4)
src/components/ui/dialog.tsx (1)
47-47: LGTM! Enhanced keyboard navigation focus behavior.The change from
focus:ring-2tofocus-visible:ring-2improves accessibility by showing the focus ring only during keyboard navigation, reducing visual noise for mouse users while maintaining keyboard accessibility.src/components/ui/sheet.tsx (1)
67-67: LGTM! Consistent focus behavior across components.The focus behavior change matches the Dialog component, ensuring a consistent keyboard navigation experience throughout the application.
src/pages/FacilityOrganization/components/FacilityOrganizationSelector.tsx (2)
31-32: LGTM! Simplified state management.The refactor from multi-level to single organization selection simplifies the state management and improves maintainability.
57-61: LGTM! Clean selection handler implementation.The handler properly manages both the organization ID and its associated data, with appropriate null checks.
|
@rithviknishad @Jacobjeevan Any changes required ? |
|
|
||
| const handleLevelChange = (value: string, level: number) => { | ||
| const orgList = | ||
| level === 0 | ||
| ? getAllOrganizations?.results | ||
| : currentLevelOrganizations?.results; | ||
|
|
||
| const selectedOrg = orgList?.find((org) => org.id === value); | ||
| if (!selectedOrg) return; | ||
|
|
||
| const newLevels = selectedLevels.slice(0, level); | ||
| newLevels.push(selectedOrg); | ||
| setSelectedLevels(newLevels); | ||
| setSelectedOrganization(selectedOrg); | ||
|
|
||
| // Always update the selected value, regardless of children | ||
| onChange(selectedOrg.id); | ||
| }; | ||
|
|
||
| const getOrganizationOptions = ( |
There was a problem hiding this comment.
the issue was about changing the icons right?
is this required?
Proposed Changes
Edit button
v1.mp4
Closing Button
v2.mp4
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
Release Notes
UI Improvements
Bug Fixes
Refactor