fix: Service point drop down responsive issue#16258
fix: Service point drop down responsive issue#16258NikhilA8606 wants to merge 5 commits intodevelopfrom
Conversation
WalkthroughUpdated ServicePointsDropDown responsive behavior: changed visible-chip breakpoints to Changes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
ohc-project-manager
bot
added
Work in Progress
needs testing
needs review
and removed
Work in Progress
labels
There was a problem hiding this comment.
Pull request overview
Fixes a responsive layout issue in the Queue “Service Points” selector by adjusting how many assigned service points are shown at different breakpoints and changing the selected-service-point “pill” layout to better fit smaller viewports.
Changes:
- Update breakpoint-based
defaultServicePointscount (show fewer onsm, restore onlg). - Make the assigned service point pills stack vertically on smaller screens and switch back to a row on medium+ screens.
- Adjust pill width to be full-width on very small screens and narrower on
sm+.
Comments suppressed due to low confidence (1)
src/pages/Facility/queues/ServicePointsDropDown.tsx:58
- The service point “pill” uses a flex layout with a fixed width at
sm(sm:w-20), but the inner text span only hastruncateand nomin-w-0/ flex sizing. In flexbox, this often prevents the text from shrinking, so ellipsis won’t apply and the name can overflow the pill (reintroducing horizontal overflow on small/medium widths). Consider addingmin-w-0(and typicallyflex-1) to the text element (or an appropriate wrapper) so truncation reliably works within the constrained pill width.
className="flex sm:w-20 w-full items-center justify-center gap-1 border border-gray-300 py-0.5 px-1.5 rounded-sm bg-gray-50 whitespace-nowrap"
>
<div className="bg-primary-200 border border-primary-500 w-2 h-2 rounded-full" />
<span className="text-sm text-gray-950 font-medium truncate">
{subQueue.name}
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Facility/queues/ServicePointsDropDown.tsx (1)
38-54:⚠️ Potential issue | 🟡 MinorAdd
min-w-0in the chip row to make truncation reliable.With the new narrow chip widths, long names can still overflow in flex contexts. Add
min-w-0to the flex wrappers (Line 44 / Line 54) sotruncatecan actually shrink text.Suggested fix
- <div className="flex flex-col md:flex-row gap-1 items-center justify-center w-full"> + <div className="flex min-w-0 w-full flex-col gap-1 items-center justify-center md:flex-row"> ... - className="flex sm:w-20 w-full items-center justify-center gap-1 border border-gray-300 py-0.5 px-1.5 rounded-sm bg-gray-50 whitespace-nowrap" + className="flex min-w-0 w-full items-center justify-center gap-1 border border-gray-300 py-0.5 px-1.5 rounded-sm bg-gray-50 whitespace-nowrap sm:w-20"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Facility/queues/ServicePointsDropDown.tsx` around lines 38 - 54, The chip row flex containers in ServicePointsDropDown (the JSX that renders when assignedServicePointIds.length > 0) need the CSS utility min-w-0 added to their className so text with truncate can actually shrink; update the outer wrapper (the div with className "flex flex-col md:flex-row gap-1 items-center justify-center w-full") and the individual chip container (the div with className "flex sm:w-20 w-full items-center justify-center gap-1 border ...") to include min-w-0 to prevent overflow of long service point names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/Facility/queues/ServicePointsDropDown.tsx`:
- Line 21: The breakpoint map passed to useBreakpoints in ServicePointsDropDown
(variable defaultServicePoints) is non-monotonic ({ default: 4, sm: 2, lg: 4})
and causes small-screen overflow; update the map to follow mobile-first
progression (ensure default ≤ sm ≤ lg) — for example set default to 2 (e.g., {
default: 2, sm: 2, lg: 4 }) so chip sizing/gap behaves monotonically across
viewports.
---
Outside diff comments:
In `@src/pages/Facility/queues/ServicePointsDropDown.tsx`:
- Around line 38-54: The chip row flex containers in ServicePointsDropDown (the
JSX that renders when assignedServicePointIds.length > 0) need the CSS utility
min-w-0 added to their className so text with truncate can actually shrink;
update the outer wrapper (the div with className "flex flex-col md:flex-row
gap-1 items-center justify-center w-full") and the individual chip container
(the div with className "flex sm:w-20 w-full items-center justify-center gap-1
border ...") to include min-w-0 to prevent overflow of long service point names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1fddf09b-0dd1-4428-a36c-72cd8237ad85
📒 Files selected for processing (1)
src/pages/Facility/queues/ServicePointsDropDown.tsx
| const { assignedServicePointIds, allServicePoints, toggleServicePoint } = | ||
| useQueueServicePoints(); | ||
| const defaultServicePoints = useBreakpoints({ default: 4, sm: 6 }); | ||
| const defaultServicePoints = useBreakpoints({ default: 4, sm: 2, lg: 4 }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Locate useBreakpoints implementation:"
fd -i 'useBreakpoints.ts' src
fd -i 'useBreakpoints.tsx' src
echo
echo "Print hook implementation for semantic check:"
fd -i 'useBreakpoints.ts' src --exec sed -n '1,220p' {}
fd -i 'useBreakpoints.tsx' src --exec sed -n '1,220p' {}
echo
echo "Find breakpoint maps used across repo for consistency:"
rg -n --type=ts --type=tsx -C2 'useBreakpoints\(\s*\{'Repository: ohcnetwork/care_fe
Length of output: 1315
🏁 Script executed:
cat -n src/pages/Facility/queues/ServicePointsDropDown.tsx | head -50Repository: ohcnetwork/care_fe
Length of output: 2322
🏁 Script executed:
cat -n src/pages/Facility/queues/ServicePointsDropDown.tsx | tail -n +50Repository: ohcnetwork/care_fe
Length of output: 3856
Breakpoint map violates mobile-first responsive design principle.
The pattern { default: 4, sm: 2, lg: 4 } is non-monotonic: smallest screens show 4 chips (each w-full) while medium screens show 2 chips (each w-20). This causes layout tension on small viewports where 4 full-width chips with gap-1 will wrap or overflow. Follow the mobile-first approach by ensuring the progression is monotonic: default should be ≤ sm ≤ lg. Consider changing default to 2 to prevent small-screen overflow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Facility/queues/ServicePointsDropDown.tsx` at line 21, The
breakpoint map passed to useBreakpoints in ServicePointsDropDown (variable
defaultServicePoints) is non-monotonic ({ default: 4, sm: 2, lg: 4}) and causes
small-screen overflow; update the map to follow mobile-first progression (ensure
default ≤ sm ≤ lg) — for example set default to 2 (e.g., { default: 2, sm: 2,
lg: 4 }) so chip sizing/gap behaves monotonically across viewports.
🎭 Playwright Test ResultsStatus: ✅ Passed
📊 Detailed results are available in the playwright-final-report artifact. Run: #8207 |
|
@nihal467 this looks fine |
Deploying care-preview with
|
| Latest commit: |
cf94454
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8e8fb98a.care-preview-a7w.pages.dev |
| Branch Preview URL: | https://feat-service-point-broken.care-preview-a7w.pages.dev |
nihal467
left a comment
nihal467
left a comment
There was a problem hiding this comment.
- IN SMALLER SCREEN ITS BREAKING
ohc-project-manager
bot
added
Changes Required
and removed
needs review
needs testing
labels
|
[ENG-11] |
| className="flex sm:w-auto w-full items-center justify-center gap-1 border border-gray-300 py-0.5 px-1.5 rounded-sm bg-gray-50 whitespace-nowrap" | ||
| > | ||
| <div className="bg-primary-200 border border-primary-500 w-2 h-2 rounded-full" /> | ||
| <span className="text-sm text-gray-950 font-medium truncate"> |
There was a problem hiding this comment.
With sm:w-auto on the chip container, long service point names can expand the chip to its full content width, and the truncate on the inner <span> won’t reliably take effect in a flex row without min-w-0/a max-width constraint. This can reintroduce horizontal overflow on sm+ viewports. Consider adding a max width (responsive if needed) and min-w-0/overflow-hidden so truncation actually works for long names.
| className="flex sm:w-auto w-full items-center justify-center gap-1 border border-gray-300 py-0.5 px-1.5 rounded-sm bg-gray-50 whitespace-nowrap" | |
| > | |
| <div className="bg-primary-200 border border-primary-500 w-2 h-2 rounded-full" /> | |
| <span className="text-sm text-gray-950 font-medium truncate"> | |
| className="flex w-full max-w-full items-center justify-center gap-1 overflow-hidden whitespace-nowrap rounded-sm border border-gray-300 bg-gray-50 px-1.5 py-0.5 sm:w-auto sm:max-w-xs" | |
| > | |
| <div className="bg-primary-200 border border-primary-500 w-2 h-2 rounded-full shrink-0" /> | |
| <span className="min-w-0 truncate text-sm font-medium text-gray-950"> |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/pages/Facility/queues/ServicePointsDropDown.tsx (1)
21-21:⚠️ Potential issue | 🟠 MajorUse a monotonic mobile-first breakpoint map.
At Line 21,
{ default: 4, sm: 2, lg: 4 }is non-monotonic and breaks mobile-first behavior (smallest screens render more chips thansm). This can reintroduce cramped/overflow-prone layouts on narrow devices.💡 Proposed fix
- const defaultServicePoints = useBreakpoints({ default: 4, sm: 2, lg: 4 }); + const defaultServicePoints = useBreakpoints({ default: 2, sm: 2, lg: 4 });As per coding guidelines "Use Tailwind's responsive classes and follow mobile-first approach."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/Facility/queues/ServicePointsDropDown.tsx` at line 21, The breakpoint map passed to useBreakpoints (const defaultServicePoints) is non‑monotonic ({ default: 4, sm: 2, lg: 4}) which violates mobile‑first behavior; change it to a monotonic, mobile‑first map (e.g., start with the smallest count at default and increase or stay same at larger breakpoints) so narrow screens render fewer chips—update the map used by defaultServicePoints in ServicePointsDropDown.tsx accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pages/Facility/queues/ServicePointsDropDown.tsx`:
- Line 57: In the ServicePointsDropDown component, the span with className
"min-w-0 truncate text-sm font-medium text-gray-950" hides the full service
point name — add a title attribute that contains the full label so users can see
the complete name on hover; e.g., set title={servicePoint.label ||
servicePoint.name || option.label} (or the exact prop used where the list is
rendered) and consider mirroring it to aria-label for accessibility.
---
Duplicate comments:
In `@src/pages/Facility/queues/ServicePointsDropDown.tsx`:
- Line 21: The breakpoint map passed to useBreakpoints (const
defaultServicePoints) is non‑monotonic ({ default: 4, sm: 2, lg: 4}) which
violates mobile‑first behavior; change it to a monotonic, mobile‑first map
(e.g., start with the smallest count at default and increase or stay same at
larger breakpoints) so narrow screens render fewer chips—update the map used by
defaultServicePoints in ServicePointsDropDown.tsx accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fbef5e89-621d-4a62-a405-5b191e81d2c7
📒 Files selected for processing (1)
src/pages/Facility/queues/ServicePointsDropDown.tsx
| <div className="bg-primary-200 border border-primary-500 w-2 h-2 rounded-full" /> | ||
| <span className="text-sm text-gray-950 font-medium truncate"> | ||
| <div className="bg-primary-200 border border-primary-500 w-2 h-2 rounded-full shrink-0" /> | ||
| <span className="min-w-0 truncate text-sm font-medium text-gray-950"> |
There was a problem hiding this comment.
Add a tooltip for truncated service point names.
At Line 57, truncate hides full names without a fallback. Add a title so users can read the complete service point label.
💡 Proposed fix
- <span className="min-w-0 truncate text-sm font-medium text-gray-950">
+ <span
+ title={subQueue.name}
+ className="min-w-0 truncate text-sm font-medium text-gray-950"
+ >
{subQueue.name}
</span>Based on learnings "When using text truncation (truncate class or similar) in the UI, always add a tooltip to show the full text."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/Facility/queues/ServicePointsDropDown.tsx` at line 57, In the
ServicePointsDropDown component, the span with className "min-w-0 truncate
text-sm font-medium text-gray-950" hides the full service point name — add a
title attribute that contains the full label so users can see the complete name
on hover; e.g., set title={servicePoint.label || servicePoint.name ||
option.label} (or the exact prop used where the list is rendered) and consider
mirroring it to aria-label for accessibility.
Proposed Changes
Fixes #16257
Tagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit