-
Notifications
You must be signed in to change notification settings - Fork 449
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
Added No records found in ABDM and button placement adjusted in Investigation Reports Page #8999
Added No records found in ABDM and button placement adjusted in Investigation Reports Page #8999
Conversation
WalkthroughThe changes in this pull request introduce conditional rendering in the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 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 failed.
|
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
🧹 Outside diff range and nitpick comments (2)
src/components/ABDM/ABDMFacilityRecords.tsx (2)
187-193
: Consider improving the empty state UX and accessibility.While the implementation addresses the basic requirement, there are a few suggestions to enhance the user experience:
- The
h-screen
class might cause unnecessary vertical scrolling as it forces the container to be 100vh tall. Consider using a more appropriate height.- The empty state message should be wrapped in a semantic HTML element with proper ARIA attributes for better accessibility.
Consider applying this improvement:
{consentsResult?.results.length === 0 && ( - <div className="flex h-screen items-center justify-center p-4"> - <p className="text-2xl font-bold text-secondary-600"> + <div className="flex min-h-[200px] items-center justify-center p-4"> + <p + className="text-2xl font-bold text-secondary-600" + role="status" + aria-live="polite" + > {t("no_records_found")} </p> </div> )}
187-193
: Consider adding a refresh button in the empty state.Since the component already has a refresh functionality (via the
refetch
function), consider adding a refresh button in the empty state to allow users to easily retry loading the data.{consentsResult?.results.length === 0 && ( <div className="flex h-screen items-center justify-center p-4"> - <p className="text-2xl font-bold text-secondary-600"> - {t("no_records_found")} - </p> + <div className="flex flex-col items-center gap-4"> + <p className="text-2xl font-bold text-secondary-600"> + {t("no_records_found")} + </p> + <ButtonV2 + onClick={() => refetch()} + ghost + className="text-sm text-secondary-700 hover:text-secondary-900" + > + <CareIcon icon="l-refresh" /> {t("refresh")} + </ButtonV2> + </div> </div> )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/Locale/en.json
(1 hunks)src/components/ABDM/ABDMFacilityRecords.tsx
(1 hunks)src/components/Facility/Investigations/Reports/index.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/Locale/en.json
- src/components/Facility/Investigations/Reports/index.tsx
🔇 Additional comments (1)
src/components/ABDM/ABDMFacilityRecords.tsx (1)
187-193
: Verify translation key existence.
The code uses the translation key "no_records_found". Let's verify that this key exists in the localization files to prevent missing translation issues.
✅ Verification successful
Translation key 'no_records_found' is properly defined in the English locale file
The key exists in src/Locale/en.json
with the translation "No Records Found". Let's verify if it's also defined in other locale files to ensure complete translation coverage.
Translation key 'no_records_found' needs to be added to non-English locale files
The translation key is only defined in the English locale file (src/Locale/en.json
). Let's check if there are other locale files that need this translation added.
Translation key 'no_records_found' is properly defined
The key exists in the English locale file (src/Locale/en.json
) and since no other locale files were found in the repository, there are no missing translations to be concerned about. The translation system appears to be correctly set up for this key.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the translation key in localization files
rg -g '*.json' -g '*.ts' '"no_records_found"'
Length of output: 108
Script:
#!/bin/bash
# Find all locale files and check for the translation key
fd -e json -p 'Locale/' --exec rg -l '"no_records_found"' {}
Length of output: 83
Script:
#!/bin/bash
# List all locale files to identify missing translations
fd -e json 'Locale/'
Length of output: 20
LGTM |
👋 Hi, @nithish1018, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
👋 Hi, @nithish1018, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
@nithish1018 Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
Before Changes:
After Changes:
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit