Add 'Update Details' Button in Community Nurse Log Update Details Page #9094
Conversation
WalkthroughThe changes involve the addition of a new button labeled "Update Log" in the Changes
Assessment against linked issues
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/LogUpdate/CriticalCarePreview.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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/components/LogUpdate/CriticalCarePreview.tsx(1 hunks)
🔇 Additional comments (2)
src/components/LogUpdate/CriticalCarePreview.tsx (2)
59-69: Button layout integration looks good
The new button is well-integrated into the existing layout with consistent spacing and styling.
62-66: Implementation successfully meets PR objectives
The added button provides the required direct navigation to the edit log update page as specified in issue #9060. The URL construction is correct and includes all necessary parameters.
| <div> | ||
| <div className="mt-2"> | ||
| <ButtonV2 | ||
| href={`/facility/${props.facilityId}/patient/${props.patientId}/consultation/${props.consultationId}/daily-rounds/${props.id}/update`} | ||
| > | ||
| Update Details | ||
| </ButtonV2> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Several improvements needed for the Update Details button implementation
- The button text should use the translation system like other texts in the component
- The nested div structure can be simplified
- Missing accessibility and testing attributes
Apply these improvements:
- <div>
- <div className="mt-2">
- <ButtonV2
- href={`/facility/${props.facilityId}/patient/${props.patientId}/consultation/${props.consultationId}/daily-rounds/${props.id}/update`}
- >
- Update Details
- </ButtonV2>
- </div>
- </div>
+ <ButtonV2
+ className="mt-2"
+ id="update-details"
+ data-testid="update-details-button"
+ aria-label={t("update_details")}
+ href={`/facility/${props.facilityId}/patient/${props.patientId}/consultation/${props.consultationId}/daily-rounds/${props.id}/update`}
+ >
+ {t("update_details")}
+ </ButtonV2>Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
@kihan2518B, thank you for sharing the screenshot. It appears that simplifying the nested divs may have affected the layout. To maintain the desired spacing while cleaning up the code, we can adjust the structure slightly:
<div className="mt-2">
<ButtonV2
id="update-details"
data-testid="update-details-button"
aria-label={t("update_details")}
href={`/facility/${props.facilityId}/patient/${props.patientId}/consultation/${props.consultationId}/daily-rounds/${props.id}/update`}
>
{t("update_details")}
</ButtonV2>
</div>This keeps the mt-2 margin on the wrapping <div> to preserve the spacing, while simplifying the nested divs and utilizing the translation function for the button text. Additionally, the accessibility and testing attributes are included for better practice.
Let me know your thoughts!
|
@rithviknishad can you review my pr |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Summary by CodeRabbit