Skip to content
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

Changes 'CONSULTATION NOTES' to 'GENERAL INSTRUCTIONS (ADVICE)' #8216

Merged

Conversation

ahmedobaid23
Copy link
Contributor

Proposed Changes

@coronasafe/care-fe-code-reviewers @coronasafe/code-reviewers

Merge Checklist

  • Add specs that demonstrate bug / test a new feature.
  • Update product documentation.
  • Ensure that UI text is kept in I18n files.
  • Prep screenshot or demo video for changelog entry, and attach it to issue.
  • Request for Peer Reviews
  • Completion of QA

@ahmedobaid23 ahmedobaid23 requested a review from a team as a code owner July 25, 2024 19:01
Copy link

vercel bot commented Jul 25, 2024

@ahmedobaid23 is attempting to deploy a commit to the Open Healthcare Network Team on Vercel.

A member of the Team first needs to authorize it.

Copy link

netlify bot commented Jul 25, 2024

Deploy Preview for care-egov-staging ready!

Name Link
🔨 Latest commit 85109ac
🔍 Latest deploy log https://app.netlify.com/sites/care-egov-staging/deploys/66a7801f66205f000869e8a8
😎 Deploy Preview https://deploy-preview-8216--care-egov-staging.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice attempt!

Although this works, there would be other keys that might need an altered display text eventually.

Consider adding a map or using translation files (preferred) to reverse the key to a display text.

For example, refer:

@ahmedobaid23
Copy link
Contributor Author

@rithviknishad

Thanks for reviewing my PR.

I will create a new file named ConsultationFormTranslation.json. For now, I will only map consultation_notes to its relevant display text. In the future, if the display text of other keys needs to changed, they will be added and mapped to their corresponding display texts in this file. Sounds good?

@rithviknishad
Copy link
Member

You can keep it in consultation.json itself. We have all consultation related translation keys present there.

interface IProps {
values: Record<string, unknown>;
}

type ConsultationMessageKeys = keyof typeof consultationMessage;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't be importing the translations file directly. Use useTranslation hook to retrieve the display text instead. Ref: https://github.com/coronasafe/care_fe/blob/f76248c997c1153a9f859511a6678ada18dd0af9/src/Components/Facility/Consultations/DailyRoundsList.tsx#L24

Copy link

👋 Hi, @ahmedobaid23,
Conflicts have been detected against the base branch. Please rebase your branch against the base branch.


This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there.

@github-actions github-actions bot added the merge conflict pull requests with merge conflict label Jul 29, 2024
@nihal467
Copy link
Member

@ahmedobaid23 can you work on the changes requested

@rithviknishad rithviknishad added changes required and removed merge conflict pull requests with merge conflict labels Jul 29, 2024
@ahmedobaid23
Copy link
Contributor Author

@rithviknishad Yes, I am working on it

@nihal467
Copy link
Member

LGTM

@khavinshankar khavinshankar merged commit 2ba8471 into ohcnetwork:develop Jul 31, 2024
31 of 32 checks passed
Copy link

@ahmedobaid23 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! 🙌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Events module: Advise
5 participants