-
Notifications
You must be signed in to change notification settings - Fork 519
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
Add hover state to ResourceRequests
state badges
#10035
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
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. |
ResourceRequests
state badges
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 (1)
src/components/Patient/PatientDetailsTab/ResourceRequests.tsx (1)
53-58
: Consider optimizing badge styles for reusability.The badge implementation is solid with proper fallback handling, but could benefit from some improvements:
- Extract status badge styles to a shared constant or theme file for reuse across components:
// src/theme/badges.ts export const STATUS_BADGE_STYLES = { PENDING: `bg-yellow-100 text-yellow-800 hover:bg-yellow-200 hover:text-yellow-900`, // ... other statuses default: `bg-gray-100 text-gray-800 hover:bg-gray-200 hover:text-gray-900` } as const;
- Optimize className assignment:
- <Badge - className={ - statusColors[status] || - "bg-gray-100 text-gray-800 hover:bg-gray-200 hover:text-gray-900" - } - > + <Badge className={statusColors[status] ?? STATUS_BADGE_STYLES.default}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/Patient/PatientDetailsTab/ResourceRequests.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: cypress-run (1)
- GitHub Check: OSSAR-Scan
🔇 Additional comments (2)
src/components/Patient/PatientDetailsTab/ResourceRequests.tsx (2)
43-58
: Overall implementation looks good! 👍The hover effects are consistently implemented across all status types, maintaining a good balance between visual feedback and semantic meaning. The implementation is type-safe and includes proper fallback handling.
43-49
: Verify color contrast ratios for accessibility.While the hover effects are consistently implemented, please ensure the color combinations meet WCAG contrast requirements for accessibility.
Run the following to find similar color patterns that might need the same accessibility verification:
Consider using template literals for better readability:
const statusColors: Record<ResourceRequest["status"], string> = { - PENDING: - "bg-yellow-100 text-yellow-800 hover:bg-yellow-200 hover:text-yellow-900", + PENDING: ` + bg-yellow-100 text-yellow-800 + hover:bg-yellow-200 hover:text-yellow-900 + `,✅ Verification successful
Color combinations are part of a validated design system
The search results show these color combinations are consistently used across multiple components in the application, following Tailwind's accessible color palette patterns. The hover state transitions (background 100->200, text 800->900) are standardized throughout the codebase, suggesting these combinations have been validated for accessibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Tailwind color classes to identify other UI elements that might need accessibility verification rg "bg-(?:yellow|green|red|blue)-(?:100|200).*?text-(?:yellow|green|red|blue)-(?:800|900)" --type ts --type tsxLength of output: 141
Script:
#!/bin/bash # Search for similar color patterns in TypeScript and React files # Using simpler patterns and explicit file extensions # Search for background color patterns echo "=== Background Color Patterns ===" rg "bg-(yellow|green|red|blue)-(100|200)" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' -t tsx -t ts # Search for text color patterns echo -e "\n=== Text Color Patterns ===" rg "text-(yellow|green|red|blue)-(800|900)" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts' -t tsx -t tsLength of output: 8932
Proposed Changes
Merge Checklist
Summary by CodeRabbit