Fix #14999 :Loading Skeleton for Queue Management Page#15095
Fix #14999 :Loading Skeleton for Queue Management Page#15095Jacobjeevan merged 7 commits intoohcnetwork:developfrom
Conversation
✅ Deploy Preview for care-ohc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
WalkthroughReplaces generic Loading spinners with skeleton UIs: introduces an internal QueueManagementSkeleton in the Queue Management page and dialog, and swaps Loading for Skeletons in the Service Points dropdown. No changes to business logic or public APIs. Changes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/pages/Facility/queues/ManageQueue.tsx:
- Around line 347-363: The loading branch renders DialogContent directly which
breaks dialog context; instead ensure the Dialog wrapper (and its DialogTrigger)
is always rendered and move the loading skeleton into DialogContent inside that
Dialog: in ManageQueue render the <Dialog> (with existing DialogTrigger)
regardless of allServicePoints, and when allServicePoints is falsy render the
skeleton UI inside the DialogContent component so the dialog open/close behavior
remains intact.
- Around line 292-334: In QueueManagementSkeleton, the Page component is
currently given title=" " (a single space); change this to the empty string
title="" so the intent of no title is explicit while keeping the existing
hideTitleOnPage prop; update the Page invocation that starts the component to
use title="" instead of " ".
📜 Review details
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
src/pages/Facility/queues/ManageQueue.tsx
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx}: Write concise, technical TypeScript code with accurate examples
Use TypeScript for all code; prefer interfaces over types
Avoid enums; use maps instead
Use TanStack Query for data fetching from the API along with query and mutate utilities for the queryFn and mutationFn
Use raviger for routing
**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Useinterfacefor defining object types in TypeScript
Avoid explicitanytype in TypeScript
Use proper nullability annotations in TypeScript types
Use dedicated error handlers and TypeScript strict null checks
**/*.{ts,tsx}: Use TypeScript for all new code
Prefer interfaces over types for object definitions in TypeScript
Avoid usinganytype; use proper type definitions in TypeScript
Use type inference where possible in TypeScript
Use TanStack Query for API data management
Prefer React Context for global state management
**/*.{ts,tsx}: Use TanStack Query with thequeryandmutateutilities from@/Utils/request/
Use appropriate query keys following the resource pattern for TanStack Query
Leverage TanStack Query built-in features for pagination and debouncing
Implement proper error handling using the global error handler for TanStack Query operations
UseuseQueryhook withqueryKeyandqueryFnparameters, wherequeryFnwraps the API route with thequery()utility
UseuseMutationhook withmutationFnparameter wrapping API routes with themutate()utility, and implementonSuccesscallbacks to invalidate related queries
Support path parameters in TanStack Query using thepathParamsoption in query/mutate utilities
Support query parameters in TanStack Query using thequeryParamsoption in query/mutate utilities
Use thesilent: trueoption to suppress global error notifications when custom error handling is needed
**/*.{ts,tsx}: Use TypeScript with strict mode and ES2022 target
Useinterfacefor defining object types
Avoid explicitanytypes and maint...
Files:
src/pages/Facility/queues/ManageQueue.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{ts,tsx,js,jsx}: Use functional and declarative programming patterns; avoid classes
Prefer iteration and modularization over code duplication
Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError)
Use the "function" keyword for pure functions
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements
**/*.{ts,tsx,js,jsx}: Use path aliases for imports from src (@/*)
Use double quotes for strings
Semicolons are required at end of statements
Order imports as: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names
**/*.{ts,tsx,js,jsx}: Use path aliases@/*for imports from src directory
Use double quotes for strings
Require semicolons at end of statements
Order imports: 3rd-party → library → CAREUI → UI → components → hooks → utils → relative
Use PascalCase for component and class names
Use camelCase for variable and function names
Files:
src/pages/Facility/queues/ManageQueue.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursorrules)
Use declarative JSX
Files:
src/pages/Facility/queues/ManageQueue.tsx
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{ts,tsx}: Use strict TypeScript configuration for medical data safety in all TypeScript source files
All literal strings must use i18next for multi-language support in healthcare interfaces
Use comprehensive error boundaries and user-friendly error messages for medical workflow debugging without exposing PHI
Follow ESLint configured rules for React hooks, accessibility, and code quality
Use @tanstack/react-query for server state management in API client code
Localize date and time formats for medical timestamps using locale-aware formatting functions
Use date-fns for date handling and manipulation with locale awareness for medical timestamps
Files:
src/pages/Facility/queues/ManageQueue.tsx
src/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Prettier for consistent code formatting across the healthcare application
Files:
src/pages/Facility/queues/ManageQueue.tsx
src/pages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use raviger for application routing in page components and route definitions
src/pages/**/*.{ts,tsx}: Keep page components insrc/pages/organized by feature
Use descriptive file names that reflect the page purpose
Export page components as default exports
Co-locate page-specific components and utilities with page components
Useravigerrouter for navigation (existing dependency)
Implement proper route parameters and query string handling
Handle navigation guards and authentication checks in page components
Implement proper loading states and error boundaries in page components
Use semantic HTML structure for accessibility in page components
Use@tanstack/react-queryfor API state management in page components (existing dependency)
Implement proper loading, error, and success states for data fetching
Handle pagination and infinite scroll patterns appropriately
Cache data appropriately in page components based on usage patterns
Use controlled components for form inputs in page components
Implement proper form validation withzodschemas in page components
Handle form submission states (loading, success, error) in page components
Use React hooks for local page state
Minimize unnecessary re-renders in page components
Implement proper authentication checks in page components
Handle different user roles and permissions in page components
Redirect to login when authentication is required in page components
Show appropriate UI based on user permissions in page components
Set appropriate page titles and meta descriptions in page components
Handle OpenGraph and Twitter card meta tags in page components
Implement structured data where applicable in page components
Implement code splitting for large page components
Use lazy loading for heavy components within page components
Optimize bundle size with proper imports in page components
Handle image optimization and lazy loading in page components
Implement comprehensive error boundaries in page components
Handle API e...
Files:
src/pages/Facility/queues/ManageQueue.tsx
src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
src/**/*.{tsx,ts}: Use React 19.1.1 features and patterns following React 19 Documentation for healthcare application development
Use shadcn/ui as primary component system and CAREUI for healthcare-specific components
Use framer-motion for animations in healthcare UI interfaces
Files:
src/pages/Facility/queues/ManageQueue.tsx
src/**/*.{tsx,css}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Tailwind CSS 4.1.3 utility classes with custom healthcare-specific design system for styling
Files:
src/pages/Facility/queues/ManageQueue.tsx
**/*
📄 CodeRabbit inference engine (CLAUDE.md)
Use 2-space indentation
Files:
src/pages/Facility/queues/ManageQueue.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/02-coding-standards.mdc)
**/*.tsx: Use functional components with proper type definitions in React
One component per file is preferred
Prefer default exports for React components
Follow the component naming pattern:ComponentName.tsxfor React components
Use Tailwind CSS for styling
Use Shadcn UI components when available
Use local state for component-specific data
Use PascalCase for component file names (e.g.,AuthWizard.tsx)
Files:
src/pages/Facility/queues/ManageQueue.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/04-ui-components.mdc)
**/*.{tsx,ts}: Use Shadcn UI components as the primary component library
Follow the component documentation for proper usage
Customize components using Tailwind CSS
UsenavigateanduseRedirectfrom raviger for navigation and redirects
Use Tailwind's responsive classes and follow mobile-first approach
Implement proper ARIA attributes for accessibility
Ensure keyboard navigation works in components
Define component props using TypeScript interfaces
Use translation keys fromsrc/Locale/for internationalization
Support RTL languages in components
Use proper date/time formatting for multiple language support
Files:
src/pages/Facility/queues/ManageQueue.tsx
**/*.{ts,tsx,js,jsx,json,css,scss,html}
📄 CodeRabbit inference engine (AGENTS.md)
Use 2-space indentation
Files:
src/pages/Facility/queues/ManageQueue.tsx
🧠 Learnings (16)
📚 Learning: 2025-08-10T10:20:32.921Z
Learnt from: AdityaJ2305
Repo: ohcnetwork/care_fe PR: 13316
File: src/pages/Facility/settings/productKnowledge/ProductKnowledgeView.tsx:63-65
Timestamp: 2025-08-10T10:20:32.921Z
Learning: In the ProductKnowledgeView component at src/pages/Facility/settings/productKnowledge/ProductKnowledgeView.tsx, the loading state should return only the Loading component without wrapping it in a Page shell. This is the expected behavior to provide a cleaner loading experience without maintaining the page layout during data fetching.
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2024-11-13T11:33:22.403Z
Learnt from: sainak
Repo: ohcnetwork/care_fe PR: 9079
File: src/components/Facility/ConsultationDetails/ConsultationNursingTab.tsx:93-117
Timestamp: 2024-11-13T11:33:22.403Z
Learning: In `src/components/Facility/ConsultationDetails/ConsultationNursingTab.tsx`, the parent component manages the loading state, so child components like `NursingPlot` should not implement their own loading indicators.
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:52:19.758Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/pages.instructions.md:0-0
Timestamp: 2025-11-25T13:52:19.758Z
Learning: Applies to src/pages/**/*.{ts,tsx} : Implement proper loading states and error boundaries in page components
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:52:19.758Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/pages.instructions.md:0-0
Timestamp: 2025-11-25T13:52:19.758Z
Learning: Applies to src/pages/**/*.{ts,tsx} : Implement proper loading, error, and success states for data fetching
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:52:51.914Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/react-components.instructions.md:0-0
Timestamp: 2025-11-25T13:52:51.914Z
Learning: Applies to src/components/**/*.{ts,tsx} : Use named exports from `lucide-react` for icons (e.g., `import { SettingsIcon } from "lucide-react"`)
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:52:51.914Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/react-components.instructions.md:0-0
Timestamp: 2025-11-25T13:52:51.914Z
Learning: Applies to src/components/**/*.{ts,tsx} : Implement loading, error, and success states for async operations
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:52:19.758Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/pages.instructions.md:0-0
Timestamp: 2025-11-25T13:52:19.758Z
Learning: Applies to src/pages/**/*.{ts,tsx} : Use lazy loading for heavy components within page components
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:52:19.758Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/pages.instructions.md:0-0
Timestamp: 2025-11-25T13:52:19.758Z
Learning: Applies to src/pages/**/*.{ts,tsx} : Optimize bundle size with proper imports in page components
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:52:51.914Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/react-components.instructions.md:0-0
Timestamp: 2025-11-25T13:52:51.914Z
Learning: Applies to src/components/**/*.{ts,tsx} : Use healthcare-specific CAREUI custom components (Calendar, WeekdayCheckbox, Zoom) from `@/CAREUI/`
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:54:35.875Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/04-ui-components.mdc:0-0
Timestamp: 2025-11-25T13:54:35.875Z
Learning: Applies to **/*.{tsx,ts} : Use Shadcn UI components as the primary component library
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:52:51.914Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/instructions/react-components.instructions.md:0-0
Timestamp: 2025-11-25T13:52:51.914Z
Learning: Applies to src/components/**/*.{ts,tsx} : Use shadcn/ui components as primary UI system (Button, Card, Badge, etc.) from `@/components/ui/`
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:49:43.065Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T13:49:43.065Z
Learning: Applies to components/**/*.{ts,tsx} : Use Shadcn UI, Radix, and Tailwind for components and styling
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:54:09.978Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .cursor/rules/02-coding-standards.mdc:0-0
Timestamp: 2025-11-25T13:54:09.978Z
Learning: Applies to **/*.tsx : Use Shadcn UI components when available
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-11-25T13:50:10.786Z
Learnt from: CR
Repo: ohcnetwork/care_fe PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T13:50:10.786Z
Learning: Applies to src/**/*.{tsx,ts} : Use shadcn/ui as primary component system and CAREUI for healthcare-specific components
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-12-17T19:47:58.152Z
Learnt from: AdityaJ2305
Repo: ohcnetwork/care_fe PR: 14821
File: src/components/Location/LocationNavigation.tsx:161-163
Timestamp: 2025-12-17T19:47:58.152Z
Learning: To ensure the remove-unused-i18n.js cleanup script detects i18n keys, avoid wrapping keys inside a ternary expression passed directly to t(). Instead, call t() separately in each branch of a conditional, e.g. condition ? t("key1") : t("key2"), or compute the key in a variable before passing to t(). For LocationNavigation.tsx (and similar TSX files in the repo), prefer explicit separate i18n calls per branch or a shared precomputed key to guarantee all keys are statically detectable by the script.
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
📚 Learning: 2025-12-22T10:16:36.690Z
Learnt from: Jacobjeevan
Repo: ohcnetwork/care_fe PR: 14804
File: src/pages/Facility/services/pharmacy/DispensesView.tsx:117-135
Timestamp: 2025-12-22T10:16:36.690Z
Learning: In the care_fe repository, prefer the native HTML title attribute for simple tooltips over using shadcn/ui Tooltip components. Use Tooltip components only when the tooltip requires richer content, interactivity, or accessibility enhancements that cannot be achieved with a plain title. For simple cases, add a title attribute to the element (e.g., <span title="...">). If a Tooltip is used, ensure it is accessible and necessary, and avoid duplicating content available in the title attribute.
Applied to files:
src/pages/Facility/queues/ManageQueue.tsx
🧬 Code graph analysis (1)
src/pages/Facility/queues/ManageQueue.tsx (2)
src/components/Common/Page.tsx (1)
Page(16-34)src/components/ui/dialog.tsx (2)
DialogContent(126-126)DialogHeader(129-129)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: build
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Agent
- GitHub Check: Redirect rules - care-ohc
- GitHub Check: Header rules - care-ohc
- GitHub Check: Pages changed - care-ohc
- GitHub Check: CodeQL-Build
- GitHub Check: Test
- GitHub Check: OSSAR-Scan
- GitHub Check: deploy-preview
🔇 Additional comments (2)
src/pages/Facility/queues/ManageQueue.tsx (2)
29-29: LGTM! Correct shadcn/ui Skeleton import.The import follows the project's path alias pattern and coding guidelines for using shadcn/ui components.
107-107: Successfully replaces generic Loading with skeleton.This change achieves the PR's primary objective of replacing the generic spinner with a skeleton loader that reflects the page layout. The early return pattern with the dedicated skeleton component is appropriate for the loading state.
| function QueueManagementSkeleton() { | ||
| return ( | ||
| <Page title=" " hideTitleOnPage> | ||
| <div className="flex flex-col gap-6"> | ||
| {/* Header Section */} | ||
| <div className="flex justify-between gap-3"> | ||
| <div className="flex gap-2 items-center"> | ||
| <Skeleton className="size-10 rounded-md" /> | ||
| <div className="flex flex-col gap-2"> | ||
| <Skeleton className="h-6 w-48" /> | ||
| <Skeleton className="h-4 w-32" /> | ||
| </div> | ||
| </div> | ||
| <div className="flex gap-5 items-center"> | ||
| <div className="hidden sm:flex items-center gap-2"> | ||
| <Skeleton className="h-5 w-10 rounded-full" /> | ||
| <Skeleton className="h-4 w-24" /> | ||
| </div> | ||
| <Skeleton className="size-10 rounded-md" /> | ||
| </div> | ||
| </div> | ||
|
|
||
| {/* Tabs Section */} | ||
| <div className="flex flex-col gap-4"> | ||
| <div className="flex gap-4 border-b"> | ||
| <Skeleton className="h-10 w-32 mb-[-1px]" /> | ||
| <Skeleton className="h-10 w-32 mb-[-1px]" /> | ||
| </div> | ||
|
|
||
| {/* Content Area */} | ||
| <div className="space-y-4"> | ||
| <Skeleton className="h-12 w-full rounded-lg" /> | ||
| <div className="grid grid-cols-1 md:grid-cols-3 gap-4"> | ||
| <Skeleton className="h-64 w-full rounded-lg" /> | ||
| <Skeleton className="h-64 w-full rounded-lg" /> | ||
| <Skeleton className="h-64 w-full rounded-lg" /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </Page> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using empty string for title prop.
The skeleton component structure appropriately mirrors the page layout with header, tabs, and content placeholders. However, line 294 passes title=" " (a space) to avoid displaying a title.
♻️ Cleaner alternative
- <Page title=" " hideTitleOnPage>
+ <Page title="" hideTitleOnPage>Using an empty string is semantically clearer than a space character when no title is needed.
🤖 Prompt for AI Agents
In @src/pages/Facility/queues/ManageQueue.tsx around lines 292 - 334, In
QueueManagementSkeleton, the Page component is currently given title=" " (a
single space); change this to the empty string title="" so the intent of no
title is explicit while keeping the existing hideTitleOnPage prop; update the
Page invocation that starts the component to use title="" instead of " ".
| if (!allServicePoints) { | ||
| return <Loading />; | ||
| return ( | ||
| <DialogContent className="max-w-md"> | ||
| <DialogHeader> | ||
| <Skeleton className="h-6 w-48" /> | ||
| </DialogHeader> | ||
| <div className="space-y-2"> | ||
| {[...Array(3)].map((_, i) => ( | ||
| <div key={i} className="flex items-center space-x-3 p-3"> | ||
| <Skeleton className="size-4 rounded" /> | ||
| <Skeleton className="h-4 flex-1" /> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </DialogContent> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Critical: DialogContent cannot function without Dialog wrapper.
The loading state returns DialogContent directly (lines 349-361), bypassing the Dialog and DialogTrigger components. This breaks the dialog's open/close functionality—when a user clicks the trigger during loading, nothing will happen because DialogContent requires the context provided by the parent Dialog component.
🔧 Proposed fix
if (!allServicePoints) {
- return (
+ return (
+ <Dialog {...props}>
+ <DialogTrigger asChild>{trigger}</DialogTrigger>
- <DialogContent className="max-w-md">
- <DialogHeader>
- <Skeleton className="h-6 w-48" />
- </DialogHeader>
- <div className="space-y-2">
- {[...Array(3)].map((_, i) => (
- <div key={i} className="flex items-center space-x-3 p-3">
- <Skeleton className="size-4 rounded" />
- <Skeleton className="h-4 flex-1" />
- </div>
- ))}
- </div>
- </DialogContent>
+ <DialogContent className="max-w-md">
+ <DialogHeader>
+ <Skeleton className="h-6 w-48" />
+ </DialogHeader>
+ <div className="space-y-2">
+ {[...Array(3)].map((_, i) => (
+ <div key={i} className="flex items-center space-x-3 p-3">
+ <Skeleton className="size-4 rounded" />
+ <Skeleton className="h-4 flex-1" />
+ </div>
+ ))}
+ </div>
+ </DialogContent>
+ </Dialog>
);
}This maintains the proper Dialog → DialogTrigger → DialogContent structure so the dialog can open and display the loading skeleton.
🤖 Prompt for AI Agents
In @src/pages/Facility/queues/ManageQueue.tsx around lines 347 - 363, The
loading branch renders DialogContent directly which breaks dialog context;
instead ensure the Dialog wrapper (and its DialogTrigger) is always rendered and
move the loading skeleton into DialogContent inside that Dialog: in ManageQueue
render the <Dialog> (with existing DialogTrigger) regardless of
allServicePoints, and when allServicePoints is falsy render the skeleton UI
inside the DialogContent component so the dialog open/close behavior remains
intact.
🚀 Preview Deployment Ready!🔗 Preview URL: https://pr-15095.care-preview-a7w.pages.dev 📱 Mobile Access: This preview will be automatically updated when you push new commits to this PR. |
There was a problem hiding this comment.
Pull request overview
This PR replaces the generic Loading component with custom skeleton loaders for the Queue Management page to provide better visual feedback during data loading. The implementation adds two skeleton components: QueueManagementSkeleton for the main page and an inline skeleton for the ManageServicePointsDialog.
Changes:
- Removed the generic Loading component import and replaced with Skeleton component from shadcn/ui
- Created QueueManagementSkeleton component that mirrors the page layout (header, tabs, content grid)
- Added skeleton loading state for ManageServicePointsDialog with checkboxes and labels
- Updated loading conditions to use the new skeleton components
| return ( | ||
| <Page title=" " hideTitleOnPage> |
There was a problem hiding this comment.
The Page component is being passed an empty string as the title. While hideTitleOnPage is true, this could cause issues for SEO and accessibility. Consider passing a meaningful title string or using a constant like "Loading..." that is localized using i18next, e.g., title={t("loading")}.
| return ( | |
| <Page title=" " hideTitleOnPage> | |
| const { t } = useTranslation(); | |
| return ( | |
| <Page title={t("loading")} hideTitleOnPage> |
| <div className="grid grid-cols-1 md:grid-cols-3 gap-4"> | ||
| <Skeleton className="h-64 w-full rounded-lg" /> | ||
| <Skeleton className="h-64 w-full rounded-lg" /> | ||
| <Skeleton className="h-64 w-full rounded-lg" /> |
There was a problem hiding this comment.
The skeleton structure doesn't accurately represent the actual page content. The actual page shows a two-column layout within the QueueColumn component (Waiting and Called+Now Serving), but the skeleton shows a generic three-column grid. Consider updating the skeleton to better match the actual queue management layout with labeled columns like the real content.
| <div className="grid grid-cols-1 md:grid-cols-3 gap-4"> | |
| <Skeleton className="h-64 w-full rounded-lg" /> | |
| <Skeleton className="h-64 w-full rounded-lg" /> | |
| <Skeleton className="h-64 w-full rounded-lg" /> | |
| <div className="grid grid-cols-1 md:grid-cols-2 gap-4"> | |
| <Skeleton className="h-64 w-full rounded-lg" /> | |
| <Skeleton className="h-64 w-full rounded-lg" /> |
| return ( | ||
| <DialogContent className="max-w-md"> | ||
| <DialogHeader> | ||
| <Skeleton className="h-6 w-48" /> |
There was a problem hiding this comment.
The DialogHeader wrapper is missing a DialogTitle component, which is important for accessibility. Screen readers rely on proper dialog titles. The skeleton should either include a DialogTitle wrapper around the Skeleton or use a visually hidden title for accessibility purposes.
| <Skeleton className="h-6 w-48" /> | |
| <DialogTitle> | |
| <Skeleton className="h-6 w-48" /> | |
| </DialogTitle> |
| return ( | ||
| <Page title=" " hideTitleOnPage> |
There was a problem hiding this comment.
The skeleton loading state should include an aria-label or aria-busy attribute to inform screen reader users that content is loading. This is important for accessibility compliance in healthcare applications. Consider adding aria-busy="true" and aria-label="Loading queue management" attributes to the skeleton container or Page component.
| return ( | |
| <Page title=" " hideTitleOnPage> | |
| const { t } = useTranslation(); | |
| return ( | |
| <Page | |
| title=" " | |
| hideTitleOnPage | |
| aria-busy="true" | |
| aria-label={t("queue_management.loading_label", { | |
| defaultValue: "Loading queue management", | |
| })} | |
| > |
🎭 Playwright Test ResultsStatus: ✅ Passed
📊 Detailed results are available in the playwright-final-report artifact. Run: #6818 |
|
@gauritejusa @yash-learner @Jacobjeevan Could you please review this pr? |
|
|
||
| function QueueManagementSkeleton() { | ||
| return ( | ||
| <Page title=" " hideTitleOnPage> |
There was a problem hiding this comment.
let's not wrap with Page here, not needed.
Jacobjeevan
added
Changes Required
and removed
question
| if (!allServicePoints) { | ||
| return <Loading />; | ||
| return ( | ||
| <DialogContent className="max-w-md"> | ||
| <DialogHeader> | ||
| <Skeleton className="h-6 w-48" /> | ||
| </DialogHeader> | ||
| <div className="space-y-2"> | ||
| {[...Array(3)].map((_, i) => ( | ||
| <div key={i} className="flex items-center space-x-3 p-3"> | ||
| <Skeleton className="size-4 rounded" /> | ||
| <Skeleton className="h-4 flex-1" /> | ||
| </div> | ||
| ))} | ||
| </div> | ||
| </DialogContent> |
There was a problem hiding this comment.
When !allServicePoints, this branch returns DialogContent directly, which uses the dialog content primitive outside of a Dialog root and also drops the DialogTrigger/trigger prop entirely. This misuses the dialog API and will render a standalone modal container without any open/close state or trigger, which can lead to inconsistent behavior and accessibility issues. Consider keeping the Dialog wrapper (and trigger) in place and rendering the skeleton inside a DialogContent within that wrapper, or using a non-dialog container for the loading skeleton instead.
|
@Jacobjeevan i think the requested changes have been made, is there anything else that's required to close this pr? |
|
When it will be merge @Jacobjeevan @gauritejusa |
ohc-project-manager
Bot
added
needs testing
needs review
and removed
Changes Required
labels
Jacobjeevan
left a comment
Jacobjeevan
left a comment
There was a problem hiding this comment.
Go ahead and adjust for ServicePointsDropDown as well; you can enable throttling through browser's network tab to simulate slow connection.
|
|
||
| {/* Content Area */} | ||
| <div className="space-y-4"> | ||
| <Skeleton className="h-12 w-full rounded-lg" /> |
There was a problem hiding this comment.
The skeleton for the search/filter row (line 322) shows a single full-width skeleton element, but the actual layout has two elements arranged side-by-side: a search input on the left and a service points dropdown on the right (see ManageQueueOngoingTab.tsx lines 65-87). Consider updating the skeleton to show two side-by-side skeleton elements with appropriate responsive sizing to better match the actual layout and reduce layout shift when content loads.
| <Skeleton className="h-12 w-full rounded-lg" /> | |
| <div className="flex flex-col gap-3 sm:flex-row sm:items-center"> | |
| <Skeleton className="h-12 w-full rounded-lg sm:flex-1" /> | |
| <Skeleton className="h-12 w-full rounded-lg sm:w-48 md:w-64" /> | |
| </div> |
Jacobjeevan
added
Tested
reviewed
Proposed Changes
Fixes #14999
Loadingcomponent import and replaced it with theSkeletoncomponent from@/components/ui/skeletonQueueManagementSkeletoncomponent that mirrors the actual page layout with placeholders for:ManageServicePointsDialogwith matching skeleton content (checkbox rows + header) for consistent visual feedbackLoadingcomponent inServicePointsDropDownwith a skeleton that mirrors the actual dropdown shape (wide left pill area + narrow chevron button on the right) — visible when simulating slow connections via the browser's Network throttleTagging: @ohcnetwork/care-fe-code-reviewers
Merge Checklist
Screen.Recording.2026-01-11.094715.mp4
Summary by CodeRabbit