-
Notifications
You must be signed in to change notification settings - Fork 481
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
fix: added tooltips for Discussion Notes icons (#8618) #8637
fix: added tooltips for Discussion Notes icons (#8618) #8637
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.
LGTM, but do keep the representable texts in i18n locale files and use useTranslations
as mentioned in the PR merge checklist
src/Locale/en/Consultation.json
Outdated
"open_in_full_window": "Open in full window", | ||
"minimize_screen":"Minimize", | ||
"close_screen":"Close", | ||
"send_message":"Send" |
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.
Let's keep it in Common.json
as these texts by themselves are generic although maybe used in Consultation and/or anywhere else.
…i/care_fe into fix/issue-8618
…e to incorrect previous placement
LGTM |
👋 Hi, @syedfardeenjeelani, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
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.
Co-authored-by: Bodhish Thomas <[email protected]>
src/Locale/en/Common.json
Outdated
@@ -231,6 +231,11 @@ | |||
"SORT_OPTIONS__-name": "Patient name Z-A", | |||
"SORT_OPTIONS__bed__name": "Bed No. 1-N", | |||
"SORT_OPTIONS__-bed__name": "Bed No. N-1", | |||
"open_in_full_window": "Open in full window", |
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.
I think Fullscreen
is better 🤔
src/Locale/en/Common.json
Outdated
"open_in_full_window": "Open in full window", | ||
"minimize_screen":"Minimize", | ||
"close_screen":"Close", | ||
"send_discussion_message":"Send", |
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't this be just Send
? 🤔 Same for the others as its added to the common.
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't this be just
Send
? 🤔 Same for the others as its added to the common.
please review the changes @bodhish
Sync your fork's develop branch with latest develop, and merge develop to your PR branch. And while resolving conflicts, move the newly added keys to |
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.
Ensure prettier is configured and used properly.
LGTM |
👋 Hi, @syedfardeenjeelani, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
👋 Hi, @syedfardeenjeelani, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
Proposed Changes
Fixes Add tooltip for Discussion Notes Icons #8618 (Add tooltips for Discussion Notes Icons)
Added tooltips for icons in the Discussion Notes module:
Demo Video
icons.1.mp4
@ohcnetwork/care-fe-code-reviewers
Merge Checklist