-
Notifications
You must be signed in to change notification settings - Fork 532
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
Modified the UI of events log and switch tab #8825
Conversation
✅ 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.
Please follow the PR template and checklist.
I have added proposed changed in the pr template. can you check if everything is ok or if anything needs to changed @rithviknishad ? |
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.
lgtm, just a minor change
@JOSHIK27 the
|
Attached demo video above. And the pr above does not introduce any text to the ui (just layout fixes). Please let me if anything is needed. Thanks. |
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.
a self review of the code changes would help identify missing translations
{ | ||
text: ( | ||
<div className="flex items-center justify-center gap-1 text-sm"> | ||
Events |
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.
missing i18n
<div className="flex items-center justify-center gap-1 text-sm"> | ||
Events | ||
<span className="rounded-lg bg-warning-400 p-px px-1 text-xs text-white"> | ||
beta |
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.
missing i18n
/> | ||
<TextFormField | ||
{...field("taken_at_after")} | ||
label="Measured after" |
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.
missing i18n
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.
LGTM
Please let me know if anything needs to be changed. Screen.Recording.2024-10-23.at.08.29.11.mov |
👋 Hi, @JOSHIK27, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
LGTM |
@JOSHIK27 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
Screen.Recording.2024-10-21.at.06.32.44.mov
@ohcnetwork/care-fe-code-reviewers
Merge Checklist