-
Notifications
You must be signed in to change notification settings - Fork 434
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
Design of List View layout in Resource Page #9096
base: develop
Are you sure you want to change the base?
Design of List View layout in Resource Page #9096
Conversation
WalkthroughThe changes introduced in this pull request involve updates to both localization and the user interface of the resource list. A new key for resource status has been added to the English locale JSON file, enhancing localization. Additionally, the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/components/Resource/ResourceList.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@typescript-eslint/parser' declared in '.eslintrc.json': Cannot find module '@typescript-eslint/parser'
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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. |
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: 3
🧹 Outside diff range and nitpick comments (5)
src/components/Resource/ResourceList.tsx (5)
195-195
: Remove Unnecessary Empty DivAt line 195, there is an empty
<div>
with styling that may not be necessary. Removing it can simplify the layout and reduce unnecessary DOM elements.Apply this diff to remove the empty div:
- <div className="md:px-4"></div>
242-242
: Simplify and Verify Translation Key for CategoryThe translation key
{t("LOG_UPDATE_FIELD_LABEL__patient_category")}
at line 242 is verbose and may not accurately reflect the resource category. Consider using a more appropriate and concise key, such as{t("resource_category")}
, for clarity.
87-87
: Simplify Complex Class NamesAt line 87, the class names applied are complex and may reduce readability. Consider simplifying the class names or using utility classes for better maintainability.
209-209
: Use Standard Spacing Classes for ConsistencyAt line 209, an arbitrary padding value
py-[11px]
is used. To maintain consistency with the design system, consider using standard spacing classes provided by Tailwind CSS, such aspy-3
.Apply this diff to use a standard spacing class:
- <ButtonV2 className="py-[11px]" onClick={onBoardViewBtnClick}> + <ButtonV2 className="py-3" onClick={onBoardViewBtnClick}>
210-210
: Use Appropriate Icon Instead of RotatingAt line 210, the icon
l-list-ul
is rotated by 90 degrees usingclassName="rotate-90"
. Instead of rotating the icon, consider using an icon that directly represents the desired orientation to improve performance and clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
public/locale/en.json
(1 hunks)src/components/Resource/ResourceList.tsx
(5 hunks)
🔇 Additional comments (2)
public/locale/en.json (1)
1094-1094
: LGTM! The new translation key follows conventions.
The addition of the resource_status
key maintains consistent naming conventions, proper capitalization, and alphabetical ordering used throughout the localization file.
src/components/Resource/ResourceList.tsx (1)
102-103
: Ensure Text Color Contrast Meets Accessibility Standards
At lines 102-103, the text color text-yellow-500
on a background bg-yellow-200
may not provide sufficient contrast for readability. Please verify that the color combination meets WCAG accessibility guidelines.
…:AnveshNalimela/care_fe into issue/ohcnetwork#9090/listview-resources
add readable titles for the PR @AnveshNalimela |
@rithviknishad Could you check on these PR a |
className="w-3/4 mt-1 h-fit flex h-5 shrink-0 items-center rounded-full px-2 py-0.5 text-xs font-medium leading-4 overflow-hidden whitespace-nowrap text-ellipsis truncate bg-gray-200" | ||
> | ||
<CareIcon icon="l-truck" className="mr-2" /> | ||
<dd className="text-sky-600 truncate">{resource.status}</dd> |
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.
<dd className="text-sky-600 truncate">{resource.status}</dd> | |
<dd className="text-sky-600 truncate">{t(`resource_status__${resource.status}`)}</dd> |
Add translations
<dt | ||
title={t("resource_status")} | ||
className={`w-fit mt-1 h-fit flex h-5 shrink-0 items-center rounded-full px-2 py-0.5 text-xs font-medium leading-4 ${ | ||
resource.status === "APPROVED" | ||
? "bg-sky-200" | ||
: "bg-yellow-200 text-yellow-500" | ||
}`} | ||
> | ||
<CareIcon icon="l-truck" className="mr-2" /> | ||
<dd className="text-sky-600">{resource.status}</dd> | ||
</dt> |
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.
Can we use the <Chip>
component here?
<ResourceBlock resource={resource} /> | ||
return data.map((resource: ResourceModel, i) => ( | ||
<div key={i} className="w-full border-b-2 border-gray-100 col-span-6"> | ||
<div className="border-3 flex grid w-full gap-1 overflow-hidden bg-white p-4 shadow sm:grid-cols-1 md:grid-cols-1 lg:grid-cols-5"> |
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.
We are moving away from shadows in our designs, lets use borders. gray-300
or gray-200
border would be fine
Proposed Changes
-Changed the List View into List format which includes imp deatils of resource card
Before:
After:
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Style