-
Notifications
You must be signed in to change notification settings - Fork 515
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Enhanced Resource Page #9768
base: develop
Are you sure you want to change the base?
Enhanced Resource Page #9768
Conversation
WalkthroughThis pull request introduces significant changes to the resource management interface by consolidating the board and list views into a single 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. |
@rithviknishad review this one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/Kanban/Board.tsx (1)
38-39
: Removed horizontal scrolling buttons and simplified layout.The new two-tier container approach (outer
+ flex boxes) is neat. Ensure no accessibility regression for users who relied on the previous scroll control.src/components/Resource/ResourceCardList.tsx (1)
26-85
: Card layout effectively conveys resource info.Using a grid and separate display for title, status, and emergency badge is clear, providing a well-organized view. Ensure each field is accessible with descriptive ARIA labels if needed.
src/components/Resource/ResourcePage.tsx (1)
94-126
: Consider removing redundant fragments.A fragment wrapping a single child is flagged by the linter. You can safely remove it to reduce JSX verbosity.
- <> <div className="flex lg:flex-row flex-col w-full justify-end gap-2"> - </>🧰 Tools
🪛 Biome (1.9.4)
[error] 94-126: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment(lint/complexity/noUselessFragments)
public/locale/en.json (1)
1616-1616
: LGTM! Consider adding a placeholder variant.The new translation key follows the naming convention and is properly placed. For consistency with other selection fields in the application, consider adding a placeholder variant:
"select_status": "Select Status", +"select_status_placeholder": "Select a status",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
public/locale/en.json
(1 hunks)src/Routers/routes/ResourceRoutes.tsx
(1 hunks)src/components/Kanban/Board.tsx
(3 hunks)src/components/Resource/ResourceBoard.tsx
(0 hunks)src/components/Resource/ResourceCardList.tsx
(1 hunks)src/components/Resource/ResourceList.tsx
(0 hunks)src/components/Resource/ResourcePage.tsx
(1 hunks)
💤 Files with no reviewable changes (2)
- src/components/Resource/ResourceList.tsx
- src/components/Resource/ResourceBoard.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/components/Resource/ResourcePage.tsx
[error] 94-126: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (16)
src/Routers/routes/ResourceRoutes.tsx (2)
3-3
: Successfully imported the newResourcePage
component.The import statement is clear and aligns with the consolidated view approach.
8-8
: Route consolidation looks good.Replacing the old
BoardView
/ListView
routes withResourcePage
is consistent with the PR objective of unifying the resource views.src/components/Kanban/Board.tsx (2)
137-137
: Height change fromcalc(100vh-200px)
tocalc(100vh-340px)
.Increasing the offset reduces visible space, which might cause more scrolling. Verify on smaller devices to ensure no usability issues.
169-169
: Placeholder usage in Droppable context.Including
provided.placeholder
ensures proper rendering of dragged items. This is currently a best practice for React DnD.src/components/Resource/ResourceCardList.tsx (5)
1-3
: Imports are well-structured.Imports for
Link
anduseTranslation
are minimal and aligned with usage below.
11-13
: Interface definition is clear.
ResourceCardListProps
is concise, making the component’s expected data structure explicit.
18-24
: Graceful handling of empty data.Rendering "no_results_found" message improves user feedback when there are no resources.
131-135
: “View all details” link for each resource.This link offers a direct route to resource details, improving discoverability.
142-142
: Export statement confirmed.Exporting
ResourceCardList
as default matches the new consolidated design approach.src/components/Resource/ResourcePage.tsx (7)
42-45
: Lazy loading the KanbanBoard component.This reduces initial bundle size and is a strong performance optimization for routes that may not require the board immediately.
51-67
: ResourcePage initialization logic.Storing filter states (
boardFilter
,viewMode
) is straightforward. The fallback to default active statuses plus an option for completed items is well-structured.
68-77
: Fetch logic usinguseTanStackQueryInstead
.Looks correct. The query configuration matches the advanced filter structure. Confirm that all relevant data fields are included in the final payload for the Resource list.
79-84
: Export function for CSV.Nice use of
request
withcsv: true
. This helps quickly integrate a CSV export feature.
129-227
: Board view integration.The board filter and drag-and-drop updates to resource status align well with the consolidated view. Watch out for edge cases when switching between active and completed boards.
228-297
: List view integration with advanced filters.Combining the
ResourceCardList
, refresh actions, and pagination is well-organized. The consistent approach to filtering across both views helps unify UX.
302-302
: ExportingResourcePage
.This final export aligns with the route changes in
ResourceRoutes.tsx
, ensuring navigational consistency.
|
I am unable to reopen that PR as I've reforked the repo 😅 |
solve the changes requested in previous PR here atleast then? |
I think I've done that🤔Let me list the changes.
|
let's keep the implementation idential to the one in appointments, and delete that kanban board |
got it |
@rithviknishad Do we need DragDropContext in resource page board view? |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Summary by CodeRabbit
New Features
ResourceCardList
component for displaying resource detailsResourcePage
component for comprehensive resource managementChanges
BoardView
andListView
componentsResourceRoutes
to directly renderResourcePage
Improvements